Repository navigation
[SEC-2660] Add blurb about block visibility config to readme - #50
Conversation
📝 WalkthroughWalkthroughThe README gains a new subsection documenting the Embargo Notifications Block: its placement, visibility rules tied to an associated embargo entity, and the "Node is Embargoed" condition variants for ignoring expired or bypassed embargoes. ChangesEmbargo Notifications Block Documentation
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
README.md (1)
67-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winClarify the condition selection guidance.
The current phrasing makes it difficult to determine which option corresponds to which preference for bypassed users. Consider explicitly mapping each condition to its behavior:
-If you would like the Embargo Notifications Block to stop displaying when a timed -Embargo expires, you would want to set the "Node is Embargoed" block visibility -condition to `Current embargoes on node (ignore expired)` or -`Active embargoes on node (ignore bypassed)`, depending on whether you want the block -displayed for users who have access to bypass the given Embargo. +To hide the Embargo Notifications Block when a timed embargo has expired, use: +• `Current embargoes on node (ignore expired)` — hides the block when expired, but + still shows it to users who can bypass the embargo. +• `Active embargoes on node (ignore bypassed)` — hides the block when expired OR + for users who can bypass the embargo.Also consider noting that the block's own logic already suppresses output when no embargoes apply, so this condition is primarily useful to control whether the block appears for bypassed or expired embargoes rather than letting the block render an empty container.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 67 - 72, Clarify the README guidance for the “Node is Embargoed” visibility condition by explicitly mapping each option to its behavior for bypassed users: use “Current embargoes on node (ignore expired)” when the block should still show for users who can bypass the embargo, and “Active embargoes on node (ignore bypassed)” when it should not. Also note that the block logic already hides output when there are no applicable embargoes, so this setting is mainly about controlling visibility for bypassed or expired embargo cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@README.md`:
- Around line 67-72: Clarify the README guidance for the “Node is Embargoed”
visibility condition by explicitly mapping each option to its behavior for
bypassed users: use “Current embargoes on node (ignore expired)” when the block
should still show for users who can bypass the embargo, and “Active embargoes on
node (ignore bypassed)” when it should not. Also note that the block logic
already hides output when there are no applicable embargoes, so this setting is
mainly about controlling visibility for bypassed or expired embargo cases.
|
Just poking the |
|
Tag generated by PR: v1.7.4 |
Addressing need for documentation around block visibility condition sparked by a thread from the islandora slack.
Summary by CodeRabbit