Adapted the issue description and added steps to reproduce.
Merged 2.0.x into the branch and changed the target of the MR.
I've converted the patch from #71 to a MR to comply with current Drupal processes. Tests are successful and test-only changes show that the test will fail without the fix.
nico.b → made their first commit to this issue’s fork.
I think this is the same issue as https://www.drupal.org/project/drupal/issues/3456964 🐛 Unhandled exception when trying to register a duplicate username with equality mismatch between php and database layer Active
avpaderno → credited nico.b → .
Based on @sanduhrs merge request, I've created a new MR that takes the recommendations from @boulaffasae into account.
nico.b → made their first commit to this issue’s fork.
I think this might have caused another (very similar) regression: https://www.drupal.org/project/drupal/issues/3456964 🐛 Unhandled exception when trying to register a duplicate username with equality mismatch between php and database layer Active
nico.b → created an issue.
Thanks a lot, @smustgrave and @larowlan! The suggestions in the MR seem perfectly valid and I've applied them.
Pipeline runs are still successful (and test-only changes still fail) and all threads are resolved, so I hope the MR is good to go now :)
@aasarava: Based on what I've also only learned just a few days ago (from here → ), patches usually go to the main dev branch (currently 11.x). The patch is then cherry-picked back to stable branches, so e.g. 10.2. So as @larowlan also said, the change of the target is rather a "technical detail"/"process adherence"-thing instead of a decision to not bring it to 10.2.
Thanks for the confirmation @aasarava. We've also already applied the patch to our production system due to the same reasons. For large pages with a high number of user registrations per day, this issue is really critical.
@smustgrave I've updated the MR target branch. For the open thread, there's not much I can do except for waiting for a response. I've also added a response directly in the MR again (not only here).
Ok, seems like there was indeed an issue in 11.x making the test fail. I've merged a fix from 11.x and now all tests are passing again :)
Also the test-only changes show the failing test case, so this issue should now be ready for review.
Thanks for the info, @smustgrave. Then the test documentation → is probably not up-to-date yet ;)
I've added a test case in the MR and changed the target to 11.x.
The fix might be easy. Before #981870 → , the code part mentioned above did only run if default_argument_skip_url was set to TRUE. Since that setting has been entirely removed, I believe that we can completely delete that code section (instead of always running it, as it is now).
Created an MR.
nico.b → created an issue.
I've updated the MR so that it fixes the structure that was established on 3.x.
I think the changes from the MR were not applied correctly. On the 3.x branch, all changes made/files added in the MR ended up in the root directory of the project.
By the way, why did you not just merge the PR instead of manually applying the changes? IMHO, applying the changes manually is more error-prone and credit to the original change authors is lost.
I don't think we can assume all instances of this constraint want a case insensitive comparison.
In general, I agree with this statement.
However, there are still some things not clear to me: If I understand the code of the UniqueFieldValueValidator
and the
changes made →
correctly, wasn't the behavior exactly the same in the past since the case-sensitivity was solely depend on the DB layer (since there were no additional PHP checks)? Do the changes I suggested really change this behavior since if the DB layer is case-sensitive, the code I suggested doesn't change that because the PHP code doesn't even see the potential case-insensitive duplicate because it's not even returned by the DB layer as a potential candidate (=> the results are still case-sensitive)? On the other hand, if the DB layer is case-insensitive, we now preserve this behavior and don't do an additional check on the PHP-side to filter out case-insensitive duplicates.
In contrast, in the code changes you suggested in
https://www.drupal.org/project/drupal/issues/3419816
🐛
Able to create user accounts with the same email address
Active
, I believe the behavior is now quite confusing:
- If shouldIgnoreCase == FALSE, case-sensitivity is enforced, so this is fine.
- However, if shouldIgnoreCase == TRUE, case-insensitivity is not enforced if the DB layer performs a case-sensitive comparison, right? Therefore, even if shouldIgnoreCase == TRUE, it can happen that the uniqueness is still checked in a case-sensitive manner.
This behavior doesn't align with the expectations I would have towards a shouldIgnoreCase toggle. There might be an oversight on my end, so obviously the above only applies if my understandings are correct.
I've also added a potential fix. As stated before, not sure about potential side effects though.
I added a test case that outlines the issue. This is my first ever test contribution (or Core contribution in general), so I hope everything is fine.
@caesius I did some debugging on the code and, actually, it seems like the condition you mentioned leads to correct results. Only this statement seems to get rid of duplicates. For example, when already having an account "example" and then trying to create an account "Example", $other_entity_values
contains "example", which is what we want, right? Only #L85 then gets rid of "example" and thus, the code ends up in the conclusion that there are no duplicate values.
A really straightforward fix that worked for me was converting all values to lowercase before making the intersection
$duplicate_values = array_intersect(array_map('strtolower', $item_values), array_map('strtolower', $other_entity_values));
Not sure if this might have any unintended side effects, though.
I believe this is heavily related to the changes introduced by https://www.drupal.org/node/3372085 → . It seems like the UniqueFieldValueValidator is no longer case-insensitive.
Furthermore, this also introduces another problem: You can now create multiple user accounts with the same e-mail address if you use a different case since the validator does no longer detect that there is an account with the same e-mail address, for example you can create one account with user@example.com and one with User@example.com.
Hi @Amir Simantov,
did you also hit the "Import menu links" button at admin/config/development/menu_export/import and selected the menu previously in the list of menus to be imported/exported (/admin/config/development/menu_export)?
Sorry if the question sounds dumb, but when I started using this module, it was also not clear to me that this is necessary (in addition to importing/exporting the config via Drupal's config sync mechanism).
nico.b → created an issue.
@salvis @therobyouknow I can confirm that the forum_access module works fine (so far) with your patch on Drupal 10.1.5 and ACL 2.0.0-beta1.
IF it works for you, let us know by adding a note to THIS issue here.
Please mention which client module(s) you're using that depend(s) on ACL.
I just wanted to confirm that we have been using ACL 2.0.0-beta1 on Drupal 10.1.5 for a few weeks now without any problems.
The client module depending on ACL is the forum_access module (including the patch from https://www.drupal.org/project/forum_access/issues/3333087 🐛 Forum access table template breaks on Drupal 10 Closed: duplicate ).
Now that I've looked at the code, I think I've also identified the issue. In the query you (@galvis) have already shown above
DELETE FROM "node_access" WHERE ("gid" = 'rol_test2') AND ("realm" = 'forum_access');
we pass the role ID (which is a string since Drupal 8/9) to "gid". However, the "gid" column in the database is actually an integer column. We need to map the role ID to the "gid" first (similar to as it's also done at other places in the module).
I have created a merge request that implements this and I was not able to reproduce the issue anymore.
I am able to reliably reproduce the issue with the current dev version on Drupal 9.5.11 with ACL 8.x-1.1.
Here are the steps to reproduce the issue:
- Set up a fresh Drupal installation ("standard" installation profile) - I used simplytest.me for this
- Install forum_access
- Create a role with name "Test"
- Edit the "General discussion" forum and allow the new role to "View this forum" and "Post in this forum"
- Save (and rebuild permissions)
- Create a forum post in the "General discussion" forum
- Delete the role with name "Test"
-> Issue occurs
Drupal\Core\Database\DatabaseExceptionWrapper: SQLSTATE[22007]: Invalid datetime format: 1292 Truncated incorrect DOUBLE value: 'test': DELETE FROM "node_access" WHERE ("gid" = :db_condition_placeholder_0) AND ("realm" = :db_condition_placeholder_1); Array ( [:db_condition_placeholder_0] => test [:db_condition_placeholder_1] => forum_access ) in forum_access_user_role_delete() (line 503 of /var/lib/tugboat/stm/web/modules/contrib/forum_access/forum_access.module).
I noticed that
$menuLinkEntity = \Drupal::entityQuery('menu_link_content')
->accessCheck(FALSE)
->condition('uuid', $menu['uuid'])
->execute();
always returned an empty array. The condition does no longer to be fulfilled and it seems like this is related to the value of "uuid" being placed in the array key "value" instead of directly in "uuid", although I am not sure when and where this changed - but maybe somebody else can shed some light onto this?
I created a merge request that seems to fix the issue while maintaining backwards compatibility. However, I am not sure whether the fix could have any side effects that I'm unaware of - so feedback welcome :)
nico.b → created an issue.
nico.b → created an issue.
Added a MR implementing the deletion.
nico.b → created an issue.
I had the exact same problem. I agree that the module itself should be able to do this, but since that's not the case I just built a custom module with a transactions table that implements hook_ENTITY_TYPE_update() for the userpoints entity type and updates the custom table accordingly by taking the difference of the original and the new entity. Maybe that approach would also work for you.
Created a MR that implements a migration routine.
nico.b → created an issue.
Opened a merge request adding a first simple version of a migration routine that migrates all points into a default points type. It only respects the total userpoints (i.e., the userpoints_total table). That means that the log is currently not migrated. Feedback welcome!
nico.b → created an issue.
Opened a MR that adds migration routines - open for feedback!
nico.b → created an issue.
You're right, I somehow didn't find the CKEditor issue - sorry!
At first, I also had the same message as you. However, by removing everything from the toolbar except the "Bold" (B) icon and using the htmLawed config given above, I was able to get rid of that message (which is still very confusing though, but I think that's not a problem of htmLawed). However, then I ran into the other issue that I described above.
nico.b → created an issue.
Can confirm that it fixed the issue - thanks a lot!
Thanks for following up on the issue.
I tried with most recent version of Coder (8.3.18) and PHPCS (3.7.2) and the error still occurs.
I attached a (minimal) setup (including config) that causes the issue for me:
- Unzip the directory
- Run "composer install" in the directory
- Run "./vendor/bin/phpcbf example.php"
Note that it only affects phpcbf, phpcs works fine.
If you need any additional information, please let me know!
I opened a merge request addressing the issue. I propose to just update the revision time every time a new revision is set since I cannot think of a scenario where you would not want to have this executed automatically.
nico.b → created an issue.
From my understanding, Userpoints 2.0.0-beta2 does not make use of the Transaction module anymore.
Points can be added via the UserpointsService (by using the addPoints() method). If you want to react to certain actions, you can call the service in hooks and/or EventSubscribers.
nico.b → created an issue. See original summary → .
nico.b → created an issue.
nico.b → created an issue.
There actually seem to be two errors with the image upload and path.
First, as described above,
Error: Call to a member function get() on null in Drupal\achievements\Form\AchievementEntityForm->submitForm() (line 228 of modules/contrib/achievements/src/Form/AchievementEntityForm.php).
appears when uploading an image. If setting the path manually, another error occurs due to the usage of deprecated/removes APIs:
Error: Call to undefined method Drupal\Core\File\FileSystem::uriScheme() in Drupal\achievements\Form\AchievementEntityForm->validatePath() (line 209 of modules/contrib/achievements/src/Form/AchievementEntityForm.php)
I submitted a patch (as a merge request) that should fix both issues.
I'd appreciate a review.
nico.b → made their first commit to this issue’s fork.
nico.b → created an issue.