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.
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
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.
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.