- 🇩🇪Germany Daniel Kulbe Berlin
Please also note, I fixed in this Version :
- Missing context vs. context value, a cache issue, as stated in the comment in code.
- The conditions count was created prior the and-or plugin was removed.
- The form
- I applied the same logic in the radio negate switch form to all considerable elements in the form.
- I choose to stick to jQuery in block.js
One further opinion on this ... the check regarding the empty request_path plugin - shouldn't it be happen in the plugin form validation method instead. It is called a few lines below where the check was added.
- 🇩🇪Germany Daniel Kulbe Berlin
Sorry, still found a tiny JavaScript selector issue. I will not start the failed tests in 78 again, as nothing has changed.
Can please sb. take care of the functional test, who has some strong suites there. - 🇩🇪Germany Daniel Kulbe Berlin
One final thought on this - the original code is right in one further thing: The Condition should be added to the conditions array in every case, even if there is an exception, so when collecting cachability, every condition is applied to forbidden for missing value, not just only the valid ones. Adding another version here, which fixes this.
- 🇮🇹Italy trickfun
I think that having one condition logic for all visibility types is wrong.
"user roles" condition should be always in AND mode.user roles is different from "content type" or "path".
i suggest to separate "user role" from other conditions.
I think the better solution is to haveVisibility (AND or OR for all conditions)
AND
User Visibility (AND or OR for all conditions relative to user)
Thank you
- 🇮🇹Italy kopeboy Milan
@ #82: This is a restriction imho: why?
I could say we should leave it more general, as you can't exclude in advance an OR condition on user role.Here is an example usecase for a block showing diff/debug/editorial information:
- Show this block on taxonomy term pages only
AND - Show this block on admin or edit pages only
OR - Show this block if user has taxonomy editor role
- Show this block on taxonomy term pages only
- 🇮🇹Italy trickfun
Is it possible to have this use case?
1. show A block on taxonomy term
OR
2. show A block on /my-custom-url3. show in the previous page only if user is admin
thank you
- last update
11 months ago Patch Failed to Apply - last update
11 months ago Patch Failed to Apply - Status changed to Needs review
11 months ago 4:04am 4 January 2024 - last update
11 months ago Custom Commands Failed - Status changed to Needs work
11 months ago 5:32am 4 January 2024 - Status changed to Needs review
11 months ago 4:04am 5 January 2024 - 🇮🇳India gauravvvv Delhi, India
Tried fixing CCF, attached interdiff for same.
- last update
11 months ago 25,956 pass, 1,830 fail - Status changed to Needs work
11 months ago 2:48pm 5 January 2024 - 🇩🇪Germany onfire84
Tryed Patch #86 on a 10.2.x Drupal Page. Patch does apply, but when i try to configure a block on the blocklayout page, it fails with "Error: Undefined constant "Drupal\block\form_state" in Drupal\block\BlockForm->buildVisibilityInterface() (line 242 of core/modules/block/src/BlockForm.php)."
- Status changed to Needs review
8 months ago 7:28pm 1 April 2024 - 🇺🇸United States smustgrave
Hiding patches for clarity
Cleaned up issue summary so removing that tag.
Ran test-only feature and got https://git.drupalcode.org/issue/drupal-923934/-/jobs/1210671 but that shows the schema update
This shows the block placement test. https://git.drupalcode.org/issue/drupal-923934/-/jobs/1211130
So removing that tag as well
Turned patch #93 into an MR with a few changes
Removed some changes to the tests that seemed out of scope.
Turned the new condition into an attribute vs annotation usage.
Added new test coverage.Suggestions appreciated
- Status changed to Needs work
8 months ago 7:49pm 1 April 2024 - 🇺🇸United States smustgrave
Posted in slack and @berdir made a recommendation to take a deeper look at #59
- 🇺🇸United States smustgrave
Updated issue summary with new solution.
I've pushed changes to the new MR but there is a test failure I can't figure out why, with the default config.
- Status changed to Needs review
8 months ago 2:08am 2 April 2024 - 🇺🇸United States smustgrave
All tests green, think ready for review of new approach.
- Status changed to Needs work
8 months ago 3:22am 2 April 2024 - 🇦🇺Australia acbramley
Left some comments on the MR, IMO the summary could use a rewording - it took me a few goes to understand what "hit" meant in this context.
- Status changed to Needs review
8 months ago 11:07pm 2 April 2024 - 🇺🇸United States smustgrave
Updated and cleaned out more to just include a single example.
Addressed feedback
- Status changed to Needs work
8 months ago 11:21pm 2 April 2024