I've added a test to cover this scenario: testUpdateFieldStorageDefinitionThrowsException()
. This test ensures that attempting to change the cardinality of a field with existing data correctly triggers the FieldStorageDefinitionUpdateForbiddenException
. This approach verifies that the fix is properly enforced and prevents unintended schema changes.
@smustgrave Okay, I will write soon.
I believe we could simplify this by adding a check for the presence of $table before array_intersect_key
.
However, in my opinion, the current solution is better because it provides a clearer understanding of the situation.
Regarding the pull request for the branch "3502900-inappropriate-error-when" – the issue with the failing test is related to Catch potential exception when calling Request::create() in PathBasedBreadcrumbBuilder 🐛 Catch potential exception when calling Request::create() in PathBasedBreadcrumbBuilder Needs review
For 11.x everything is fine.
@oily
Looks like test coverage goes in SqlContentEntityStorageSchemaTest.php
Okay than, I will try to investigate in more detail.
A current failed test is not related to my issue.
You are able to see error in raw log pipeline .
An appropriate screenshot with the error is attached.
usingsession → created an issue.
@natefollmer Thank you for the update. I’ve tested the patch in my environment, and I’m not encountering the mentioned JS error when clicking 'Yes' to keep the session active. Let me know if there's anything specific (role, autologout settings et.)
Added the classes:
- ui-dialog-state-transition-form
- ENTITY_TYPE_ID-state-transition-form
usingsession → made their first commit to this issue’s fork.
While developing this functionality, my primary goal was to avoid creating something new with plugins and implement it as simply as possible. Therefore, a significant portion was described within a hook.
In my opinion, the label configuration could be moved to a plugin, but this would require creating an instance, which might impact performance (though it might not).
I also updated the GitLab CI configuration to enable testing on Drupal 10, as the extra_field module does not yet have a version compatible with Drupal 11 - see If what you want is to run linting and tests under Drupal 10 💬 Disabling OPT_IN_TEST_CURRENT not working Active
usingsession → made their first commit to this issue’s fork.
@natefollmer I fixed the patch
@natefollmer I will take a look over the next few days. It is because the patch has some conflicts.
@dudeweb It seems that this issue does not belong to this module. Please specify which version of Drupal you are using, as well as Commerce. Have you updated Drupal to version 11?
- drupal/commerce_store[2.32.0, ..., 2.40.0] require drupal/core ^9.3 || ^10 -> found drupal/core[9.3.0, ..., 9.5.11, 10.0.0, ..., 10.4.0] but the package is fixed to 11.1.0 (lock file version) by a partial update and that version does not match. Make sure you list it as an argument for the update command.
This message indicates a conflict between the required version of drupal/core and the version fixed in your composer.lock file. The range [2.32.0, ..., 2.40.0] of drupal/commerce_store supports drupal/core versions ^9.3 or ^10. However, your composer.lock file specifies drupal/core version 11.1.0, which is outside this range.
@arantxio
I'm not quite sure if I understood, '...i've noticed if you are browsing other sites it won't give you any warning at all...'.
What exactly did you mean (or expect) in this case (alert, Drupal message ...) ? Perhaps I misunderstood you.
Made a few small changes (described above), can you use the patch or fork branch.
@arantxio
There is truth in your words, indeed. After more detailed testing, I concluded that the previous implementation of destroying the dialog if the page is inactive is a bad idea.
Reason for this modification:
Steps to Reproduce (STR):
- Set Timeout padding to, for example, 45 seconds.
- Set Timeout value in seconds to, for example, 120 seconds.
- Open two tabs and remain inactive in both simultaneously.
- Wait until the notification appears in both tabs.
- In one of the tabs, click "Yes" to extend the session.
- Monitor the other tab where the popup is still present. After 45 seconds, the user will be logged out, even though they were active in the other tab.
Cause:
The logout method is triggered if the user does not interact with the dialog. The logout, in turn, sends a request to the /autologout_ajax_logout or /autologout_alt_logout
route, which ignores the last activity and logs the user out.
Solution:
- Remove
autologout_page_activity
from localStorage according to the Timeout padding instead of after 30 seconds. I implemented this variant (issue fork). - Add a check in the
altLogout/ajaxLogout
route method, referencing theautologout_last
value from the session. - This variant more complex
I fixed the test, but I made a mistake with the branch name (Sorry about that).
usingsession → made their first commit to this issue’s fork.
@kdmdrupal I'm glad I could help
usingsession → changed the visibility of the branch 3494257-custom-tokens-tests to hidden.
usingsession → made their first commit to this issue’s fork.
@kdmdrupal This issue likely arises from applying a patch related to enabling JS Cookie dependencies 🐛 Multiple warnings after upgrading to 8.x-1.5 & 2.x with js_cookie Active , which may have introduced conflicting code. So, please just delete patch from "patches" section in composer.json. After that, reinstall the module
The patch in #3395581-13 works but has two main issues:
Multiple Tabs: If the user has several tabs/windows open, the dialog may appear in an inactive tab while the last active tab isn't focused. This can cause confusion.
Switching Tabs: When switching tabs, the confirmLogout timer is triggered in the inactive tab, while the dialog appears in the active one. This creates inconsistent behavior.
Fixes:
document.hasFocus() is used to ensure dialogs only appear in focused tabs.
blur event clears the paddingTimer and destroys dialogs for inactive tabs.
Patch below may improve multi-tab handling.
@anybody, okay, I got it :)
Thank you, Ahmed Eid (3eidoz), for creating the issue.
This was an important point, and I'm glad to inform you that a solution has already been found and included in the latest release.
Thanks again for your contribution!
UsingSession → created an issue.
@ady1503 Oh, okay, sorry. We will monitor the situation, I also ask you to remain active in this issue, as soon as I find a solution I will provide a patch.
Thanks for using my module :-)
@ady1503 Please try my "
3458283-izi-messag-stopped-working-with-drupal-10.3.patch →
" patch only.
It is not necessary to use the patch from the
10.3 upgrade now missing status-message theme suggestions
🐛
10.3 upgrade now missing status-message theme sugestions
Active
issue
@ady1503 Just in case, did not use these two patches together?
@ady1503 Did you clear your cache?
About which cookies are created when popup is showing ?
Please provide more information
Added for review about the page update.php (or MAINTENANCE_MODE)
@freelylw It has been tested and no problems have been observed. Please review the settings:
Show Better Messages on specific pages
should be:
Show on only the listed pages.
if you want to see popup message on this pages
As said @rleela
this is not a Better Messages issue, this happens because Bootstrap theme is flushing all messages after it gets them
also Bootstrap render message with custom template
@grougy I need more information about step reproduce
@Leeteq Are the settings in the screenshot below set?
@10thstreetlabel Specify the version in which this problem is observed. It is not clear from the description which version it is
Is this problem still observed?
The module has undergone changes, so the patch needs some changes. Thank you for your contribution to this module
Does this issue still exist in the new version of the module?
Papa Ours, when you submit a patch, please set the status to 'Needs review'.
@bcobin, please provide more information about this issue. Are you using this module on Drupal 9 or Drupal 7? In the description, you mention Drupal 7.
@petere (Peter Eloff), thank you for your report, but this issue is no longer relevant.
Thank you to all the participants in the Make the module Drupal 10 compatible issue. Special thanks to plopesc (Pablo López (plopesc)) for providing the patch. Your work and contributions to the module are highly appreciated!
Temporary fix. It is not known whether it will be added to the release. I am waiting for the 10.3 upgrade now missing status-message theme suggestions 🐛 10.3 upgrade now missing status-message theme sugestions Active issue to be resolved.
@siriussolutions Thanks for the report. I will follow this issue, as soon as it is clear on 10.3 upgrade now missing status-message theme suggestions 🐛 10.3 upgrade now missing status-message theme sugestions Active I will make changes
Albert Volkman Thank you for your work
UsingSession → made their first commit to this issue’s fork.
UsingSession → made their first commit to this issue’s fork.
The branch 8.x no longer supported, use 2.x.
The 2.x branch contains all current fixes
Maithri shetty (Maithri Shetty) - Good catch. It has been fixed. Also, some were fixed in previous issues
Eric Smith (ericgsmith) Thank you for your work!
UsingSession → changed the visibility of the branch 3444521-do-not-use-hook-access to active.
UsingSession → changed the visibility of the branch 3444521-do-not-use-hook-access to hidden.
Chris Burgess (xurizaemon), amanp - Thank you for your work, but unfortunately the module has undergone some changes, so we need to modify the patch.
@pobster Thanks for the reply, but this PR is no longer relevant, as changes have already been made regarding Dependency Injection
Thank you all for your attention. The problem is solved.
Thank you all for your attention. The problem was solved earlier
@plach Thanks for your work
Thank you all for your work
War is still going.
Thanks @Yoa