Thank you all, I've merged
I've reverted adding the patch as it does not seem to help, and instead added caching the account per user. Lets see the result of this.
Even if I set ->setCacheMaxAge(0); the behaviour persists, it seems like Drupal is not checking the observers access before its rendering the page, or before its caching the entity or view mode
I've pushed some commits to check if the issue is solved by this patch, but it seems to bring up new issues, I'll look more into it
andras_szilagyi → made their first commit to this issue’s fork.
I confirm the issue reported, I reviewed the related issue and it looks good, so I will add a line in this modules readme about it so users know to use it.
The code here also looks good.
andras_szilagyi → made their first commit to this issue’s fork.
Indeed if I check "custom permissions" without this fix then there is nothing happening. With the fix a table of permissions pop up.
I confirm the issue and fix.
Looks good
Looks good
Works well, looks good
andras_szilagyi → created an issue.
Looks good
Yes, looks good
Looks good, works fine also in my tests
Looks good
It's ok @damienmckenna, we are moving forward.
andras_szilagyi → made their first commit to this issue’s fork.
andras_szilagyi → made their first commit to this issue’s fork.
andras_szilagyi → made their first commit to this issue’s fork.
Merged, thank you all. I'm leaving the status open in case the bot wants to post more in the future.
andras_szilagyi → made their first commit to this issue’s fork.
Seems the issue was fixed in this commit https://git.drupalcode.org/project/decoupled_auth/-/commit/b581a0a7e08fa... but did not know as it lacked an issue and no release was made since.
I'm attaching a path from this commit so anyone can have the fix till the release is out.
andras_szilagyi → created an issue.
Made some changes, for me its ok now
andras_szilagyi → made their first commit to this issue’s fork.
Thank you all
andras_szilagyi → made their first commit to this issue’s fork.
Thank you all
andras_szilagyi → made their first commit to this issue’s fork.
I've added some commits to re-enable testing pipelines, and fixed some issues the pipeline reported. Thank you for your contribution.
andras_szilagyi → made their first commit to this issue’s fork.
For some reason I cant create an MR
andras_szilagyi → created an issue.
a contribution is welcome
Uploading a stable patch
and now the test passes, ready for review
tests fail with the issue reported https://git.drupalcode.org/issue/message_digest-3477172/-/jobs/2875429
andras_szilagyi → created an issue.
andras_szilagyi → created an issue.
Thank you @raveen-thakur for your fixes, I've taken care of the cspell
Andras_Szilagyi → made their first commit to this issue’s fork.
Andras_Szilagyi → created an issue.
Andras_Szilagyi → created an issue.
Think best would be to merge this as its green in 9.5, and then fix the tests in the groups upgrade ticket
Now that the d10 compatibility was merged (not ready) we will have to update and merge https://www.drupal.org/project/gcontent_moderation/issues/3309542 💬 Is group content moderation module compatible with Groups 2.0.0-beta3? Needs work without tests, or merge this and make the d10 tests pass in that issue
this was 2 early, first https://www.drupal.org/project/gcontent_moderation/issues/3348700 🐛 Fix failing tests Fixed should have been merged, then the group 2.x, then this
The 1 test is failing because this branch is not d10 compatible yet
MR created, there I will have the fix and test together, but I'm uploading also as patch to have a static fix until the issue is merged.
The parent issue should add the test, fail, and then this one would fix it.
Andras_Szilagyi → created an issue.
Actually reopeneing to permit the bot to post further patches
Nobody noticed the failing test in D10?
Andras_Szilagyi → created an issue. See original summary → .
Andras_Szilagyi → made their first commit to this issue’s fork.
My time is limited, but if you provide a MR I will review it and would welcome the contribution
I'm leaving the ticket status as is to receive any future patches.
I ran rector and tested manually in d10, works well, no errors in the logs
Re-opening to receive further patches from the bot.
I'm marking this as child of the D10 upgrade issue as it was necessary and fixed there, and closing.
Ready for you @ieguskiza
@iguskiza please enable test to run on fork
Andras_Szilagyi → made their first commit to this issue’s fork.
I've created a patch without the changes from #14, I see some d10 related stuff, doesn't make any sense to me.
Andras_Szilagyi → made their first commit to this issue’s fork.
Can one of the maintainers please enable testing on git push in forks, I need to add testing so everyone can see this pass in D9 and D10, see https://www.drupal.org/node/3297903/qa/merge_request/3297903-automated-d... → "There are no tests for this merge request yet. The first test must be triggered with a git push."
Andras_Szilagyi → made their first commit to this issue’s fork.
Shouldn't we reopen this in case the bot wants to post future patches?
Also I don't see one passing test, and I'm not surprised as the defaultTheme is set to classy which is gone in d10.
Please set back to active status, this needs more work. (I wont do it as I think its the maintainers decision)
Tested this with group 2.0.0-rc2 and the patch works as expected.
Andras_Szilagyi → made their first commit to this issue’s fork.
I confirm the issue is still present and that the patch solved it. I got the error using gcontent_moderation, same errors when visiting the group permissions page, the patch fixed it.
Andras_Szilagyi → created an issue.
I wanted my suggestion to be reviewed
I'm closing this issue as the child issues fixed it
The MR is ok and passes, and we can see the same test fail for the right reasons in the patch.