🇷🇴Romania @claudiu.cristea

Given 📌 AccessDeniedSubscriber should extend HttpExceptionSubscriberBase RTBC is RTBC and most likely will me merged, I think we should do it also here.

i think #11 makes sense

Thanks

claudiu.cristea → made their first commit to this issue’s fork.

Done

claudiu.cristea → created an issue.

Thank you for review.

This is ready for a new review. Please take a look to issue summary where I've prepared the release notes for 3.0.0, because there are some tiny BC violations, but this is accepted between major releases according to semver.

OK, I found it.

In my case I want to deprecate a list of event constants because i want to switch to class fully qualified names. What I did:]

Registered a new compile pass acting after RegisterEventSubscribersPass

<?php

declare(strict_types=1);

namespace Drupal\cas\DependencyInjection;

use Drupal\cas\Event\CasPostLoginEvent;
use Drupal\cas\Event\CasPostValidateEvent;
use Drupal\cas\Event\CasPreLoginEvent;
use Drupal\cas\Event\CasPreRedirectEvent;
use Drupal\cas\Event\CasPreRegisterEvent;
use Drupal\cas\Event\CasPreUserLoadEvent;
use Drupal\cas\Event\CasPreUserLoadRedirectEvent;
use Drupal\cas\Event\CasPreValidateEvent;
use Drupal\cas\Event\CasPreValidateServerConfigEvent;
use Drupal\cas\Service\CasHelper;
use Drupal\Component\Utility\DeprecationHelper;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;

@trigger_error('The ' . __CLASS__ . ' is deprecated in cas:3.0.0 and is removed from cas:3.1.0. No replacement is provided. See https://www.drupal.org/node/3462792', E_USER_DEPRECATED);

/**
 * Compiler pass to detect deprecated event constants.
 *
 * @deprecated in cas:3.0.0 and is removed from cas:3.1.0. No replacement is
 *   provided.
 *
 * @see https://www.drupal.org/node/3462792
 */
class DeprecatedEventConstants implements CompilerPassInterface {

  /**
   * List of deprecated constant names and their replacement.
   *
   * @var string[]
   */
  private const DEPRECATE_EVENT_IDS = [
    CasHelper::class . '::EVENT_PRE_USER_LOAD' => CasPreUserLoadEvent::class,
    CasHelper::class . '::EVENT_PRE_USER_LOAD_REDIRECT' => CasPreUserLoadRedirectEvent::class,
    CasHelper::class . '::EVENT_PRE_REGISTER' => CasPreRegisterEvent::class,
    CasHelper::class . '::EVENT_PRE_LOGIN' => CasPreLoginEvent::class,
    CasHelper::class . '::EVENT_PRE_REDIRECT' => CasPreRedirectEvent::class,
    CasHelper::class . '::EVENT_PRE_VALIDATE_SERVER_CONFIG' => CasPreValidateServerConfigEvent::class,
    CasHelper::class . '::EVENT_PRE_VALIDATE' => CasPreValidateEvent::class,
    CasHelper::class . '::EVENT_POST_VALIDATE' => CasPostValidateEvent::class,
    CasHelper::class . '::EVENT_POST_LOGIN' => CasPostLoginEvent::class,
  ];

  /**
   * {@inheritdoc}
   */
  public function process(ContainerBuilder $container): void {
    $definition = $container->getDefinition('event_dispatcher');
    $arguments = DeprecationHelper::backwardsCompatibleCall(
      \Drupal::VERSION,
      '10.3',
      fn(): array => array_map(fn(array $call): string => $call[1][0], $definition->getMethodCalls()),
      fn(): array => array_keys($definition->getArgument(1)),
    );

    $deprecated = [];
    foreach (self::DEPRECATE_EVENT_IDS as $constantName => $eventClass) {
      if (in_array(constant($constantName), $arguments, TRUE)) {
        $deprecated[] = $constantName;
      }
    }

    if ($deprecated) {
      $message = sprintf(
        'The following event constants are deprecated: %s. Use the corresponding event class fully qualified names instead: %s. See https://www.drupal.org/node/3462792',
        implode(', ', $deprecated),
        implode(', ', array_map(
          fn(string $constantName): string => $constantName . ' -> ' . self::DEPRECATE_EVENT_IDS[$constantName],
          $deprecated,
        )),
      );
      \Drupal::getContainer()->get('logger.factory')->get('cas')->warning($message);
      @trigger_error($message, E_USER_DEPRECATED);
    }
  }

}

Previously, the subscribers were added as argument of event_dispatcher service. From where can I get a list?

📌 Drupal 11 compatibility Fixed is merged.

Thanks

Ready for a 2nd round of review. Fixed some and answered the other

claudiu.cristea → created an issue.

This is ready for review.

Contains:

• Drupal 11 compatibility
• Testing extended to cover:
  • Drupal stable version (currently 10.3.x) which runs for lowest supported PHP (currently PHP 8.1) and highest supported PHP (currently PHP 8.3)
  • Drupal next major release (currently 11) which runs with highest supported PHP (currently PHP 8.3)
• Converted ServiceControllerTest unit test to kernel test
• Converted CasProxyHelperTest::testProxyAuthenticate() unit test to kernel test
• Removed the backwards compatibility deprecated layer
• Started modernization of code (this process should continue in the alpha stage):
  • Removed constructor documentation because is useless and only adds noise to the code. There are some exceptions where documenting the parameters is important.
  • Switched to constructor property promotion. Some exceptions were kept because of compatibility with Drupal 10.3.x.

Great!

Changing the branch

Working on this

Fix branch

claudiu.cristea → created an issue.

claudiu.cristea → created an issue.

Thank you for adding tests. Apart from a leftover (see the MR), I think we need:

• A change record
• Prepare a section in https://www.drupal.org/docs/upgrading-drupal/upgrading-from-drupal-8-or-... → , to explain how to use the lenient Composer plugin together with the new setting. Let's add this addition here and we'll update docs page after this is merged.

claudiu.cristea → made their first commit to this issue’s fork.

Maybe we can extend UpdateScriptTest::testExtensionCompatibilityChange() or ExtensionListTest::testCheckIncompatibility()?

I've tested the MR on our project, with dozens of modules, from which >90% don't have an updated core_version_requirement. Works as expected by setting the kill-switch in settings.php. I wonder if this needs tests. Tentatively, marking as Needs tests

claudiu.cristea → created an issue.

@japerry, latest release is from December 2022. Any chance for a new one?

OMG, I cannot believe we've dropped this. Thank you

claudiu.cristea → made their first commit to this issue’s fork.

It makes sense

Probably will need a new major version :(

claudiu.cristea → made their first commit to this issue’s fork.

Manual testing has been performed in #100 ✨ Add a setting to make description a required field for file items Needs work

claudiu.cristea → made their first commit to this issue’s fork.

claudiu.cristea → created an issue.

Drupal 10.3.x version patch

Sorry, the last patch was wrong. Here's a 10.3 version

Drupal 10.3.x patch

Patch for 11.x

Somehow @smustgrave comments in the MR were lost. At least , I cannot see them. Tentatively setting back to NR

Note that this MR has been already RTBCed in #233 ✨ Actions reordering on views bulk forms RTBC . Since then no change has been applied except:

• Reroll
• Update of core/tests/fixtures/config_install/multilingual.tar.gz (see #237 ✨ Actions reordering on views bulk forms RTBC )
• Update of fixture file from Drupal 9.4.0 to 10.3.0 (see #239 ✨ Actions reordering on views bulk forms RTBC )
• PHPCS fixed because of new rules in core
• CSS lint fixes because of new rules in core

Ready for a final review

Here's a patch for 10.3.x

Tests are green. Ready for a final review.

Great!. Thank you!

claudiu.cristea → made their first commit to this issue’s fork.

Can we get an answer on #93 from the subsystem maintainer before doing any development here?

Ready for review

claudiu.cristea → created an issue.

Thank you for clarifying

claudiu.cristea → created an issue.

I'm setting to NW because of "Needs followup" tag.

Just a proof of concept. Should we go this way?

claudiu.cristea → created an issue.

I support this

Thank you for the patch. However, creating PHP files on the fly and run them from temp dir it's a terrible idea. Don't do that, it's a security issue.

I am thinking to an approach where the list of entity types is not stored in config but in code, e.g., by exposing a hook whose invocation returns a list of entity types. Then 3rd-party code implements that hook tell which entity types they like to receive the field. Publication date will implement that hook and only return ['node']. An update hook, will iterate over all entity types and check if the field is installed. If not, will do the proper storage definition install.

claudiu.cristea → created an issue.

Ready for review

claudiu.cristea → created an issue.

claudiu.cristea → created an issue.

claudiu.cristea → made their first commit to this issue’s fork.

Thank you all

@catch, I think this part from standards is the problem

Be consistent, do not mix camelCase and snake_case variables inside a method or function.

A constructor extending the parent (which have snake_case params) cannot add a new param with constructor property promotion because that is camelCase. So, it will mix snake_case with camelCase, violating the quoted text

Thank you

Tests added

Linking the initial issue

Thank you for reporting. Indeed, CasHelper::EVENT_PRE_REGISTER events are pretending that you can alter the user properties but the recent issue just overrides any subscriber changes

 * Subscribers to this event can:
 *  - ...
 *  - Change the username that will be assigned to the Drupal account. By
 *    default it is the same as the CAS username.
 *  - Set properties on the user account that will be created, like user roles
 *    or a custom first name field (for example by populating it with data from
 *    the CAS attributes available in $casPropertyBag).

I've proposed something in the MR but we still need to weight on that and we need tests.

claudiu.cristea → made their first commit to this issue’s fork.

@jrglasgow, fixed conflicts and changed to 2.x (spell, phpstan, eslint are failing same as in current 2.x)

Moving to 2.x but I have no permission to change the base branch in MR

Moving to new 2.x branch

Rerolled after 1.5.0

claudiu.cristea → made their first commit to this issue’s fork.

greggles → credited claudiu.cristea → .

Just needed something that makes the URI unique

There's no plan to support Drupal 10.1. But if someone will provide a MR, I'm more than happy to review it and merge.

Thank you for explanations. Is it possible to add a test that shows the failure?

When you create a new node, the URI is auto generated using this plugin: \Drupal\rdf_sync\Plugin\rdf_sync\RdfUriGenerator\DefaultRdfUriGenerator. You can create your custom plugin in a custom module, then select it in "URI generator plugin" dropdown

Right now, there's no way to set the URI manually, in the node form. I think it can be done but needs some work. Contribution is welcomed.

This module → would benefit from suc a new property, see 💬 How to map image field? Active

Also note that there's ✨ Add a computed 'file_url' property to FileItem (for exposing file URL in file field normalization) Needs work aiming to fix this in core. With this you can map directly the computed property.

+1 for this. It should be similar with "processed" computer property from rich text fields.

I think you should create a computed field that computes the absolute file/image URL. Then map that field.