feat(access-control): advanced settings for RSS content restriction#4613
feat(access-control): advanced settings for RSS content restriction#4613
Conversation
There was a problem hiding this comment.
Pull request overview
Adds “Advanced settings” support to the Audience → Access Control wizard, introducing backend-stored toggles to (1) truncate restricted posts in RSS feeds and (2) optionally hide Everlit embeds in restricted content.
Changes:
- Adds an Advanced Settings modal to the Access Control wizard UI and wires it to a new wizard settings REST endpoint.
- Introduces a new
Content_Gate_Settingsbackend class to enforce feed truncation and Everlit block stripping based on saved options. - Adds unit tests covering feed truncation behavior and Everlit stripping behavior.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
tests/unit-tests/content-gate/test-advanced-settings.php |
Adds unit tests for feed truncation and Everlit stripping behavior. |
src/wizards/audience/views/content-gates/types/index.d.ts |
Adds TS types for advanced_settings returned in wizard config. |
src/wizards/audience/views/content-gates/content-gates.tsx |
Enables “Advanced settings” menu item and mounts the new modal component. |
src/wizards/audience/views/content-gates/advanced-settings.tsx |
Implements the Advanced Settings modal UI and saving/undo via REST. |
includes/wizards/audience/class-audience-content-gates.php |
Adds REST endpoint to update advanced settings and exposes settings in wizard config. |
includes/content-gate/class-content-gate.php |
Ensures the new settings class is loaded during Content Gate initialization. |
includes/content-gate/class-content-gate-settings.php |
New settings class: option storage + feed truncation + Everlit block stripping hooks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@dkoo — Thomas has reviewed these changes and will be marking this PR as design approved. Changes pushed in the latest commit:
|
miguelpeixe
left a comment
There was a problem hiding this comment.
Tested well! Left a couple of non-blocking suggestions and a small issue.
| /** | ||
| * Main class. | ||
| */ | ||
| class Content_Gate_Settings { |
There was a problem hiding this comment.
Not to be confused with the component at src/wizards/audience/views/content-gates/content-gate-settings.tsx
Non-blocking suggestion: we could make this class just about the RSS feed handling. It is already by implementing the feed restriction on top of the settings, just need to make it official with the docblocks and class name 😄
I can see a year from now, this class having 1000+ lines of small features and patches mixed together...
There was a problem hiding this comment.
I see your point, but this class is kind of where I envision we'll add additional "global" settings beyond just this RSS option, hence why it originally also owned the Everlit option. What if we renamed it to Content_Gate_Advanced_Settings or Content_Gate_Global_Settings?
There was a problem hiding this comment.
I think Content_Gate_Advanced_Settings is better.
There was a problem hiding this comment.
00d01c4 renames the class to Content_Gate_Global_Settings (and the test class too) to hopefully make the intention a bit clearer.
| /** | ||
| * Tests for RSS feed content restriction. | ||
| */ | ||
| class Test_Advanced_Settings extends \WP_UnitTestCase { |
There was a problem hiding this comment.
Also non-blocking, but I think we can have this either for RSS Feed restriction control alone or just additional tests within tests/unit-tests/content-gate/content-gates.php. "Advanced settings" is just a UI layer.
There was a problem hiding this comment.
00d01c4 renames this to Test_Global_Settings to match the renamed class it tests.
| const wizardData = useWizardData( AUDIENCE_CONTENT_GATES_WIZARD_SLUG ) as WizardData; | ||
| const { wizardApiFetch, isFetching, resetError, setError } = useWizardApiFetch( AUDIENCE_CONTENT_GATES_WIZARD_SLUG ); | ||
| const { addNotice, resetNotices, updateWizardSettings } = useDispatch( WIZARD_STORE_NAMESPACE ); | ||
| const [ config, setConfig ] = useState< AdvancedSettingsConfig >( ( wizardData?.config?.advanced_settings as AdvancedSettingsConfig ) || {} ); |
There was a problem hiding this comment.
The AdvancedSettings component is always mounted, so changing config, closing, and re-opening the modal will show stale data. I don't think we need to introduce a dirty state confirmation modal, but might be a good idea to reset on close here.
There was a problem hiding this comment.
The component always being mounted is intentional, as this is what allows the Undo button in the Snackbar component that appears after saving to function (otherwise the prior state in the ref is lost when the component unmounts). The internal config state is updated after a successful save in tandem with the wizard store, using the same data, so it's not likely to get out of sync. If you're worried about this being possibly too fragile we could do away with the "Undo" button so the component can be unmounted when the modal isn't visible. But so far I haven't run into any trouble with the component state and store data getting out of sync. 🤷
There was a problem hiding this comment.
I see... I had a different UX perception for this. When I open something in a modal, make changes, and close without clicking the Save button, I'd not expect that state to be preserved. I'd also not expect that closed modal state to be persisted if I click save somewhere else.
There was a problem hiding this comment.
That's a valid point. b748566 fixes that issue, so that the local state is reset upon closing the modal. Also applies the same fix to the Premium Newsletters advanced settings modal, which uses the same component pattern (maybe we could combine these into one reusable component in the future).
There was a problem hiding this comment.
If I save the new value, opening the modal again will render the state from page load.
|
@miguelpeixe thanks for the feedback! Happy to discuss further if you feel the changes aren't enough |
All Submissions:
Changes proposed in this Pull Request:
Adds an Advanced Settings modal in the Access Control wizard page to contain options that don't fit into any other UI views or components. Implements the following options:
Hide Everlit in restricted content - Prevents Everlit audio embed blocks from rendering in restricted content. Enabled by default.Note: we've removed this option as this should be handled by the Everlit plugin, not here.Closes NPPD-1035.
How to test the changes in this Pull Request:
https://sitedomain/feed) and confirm that all restricted articles show their full content.Other information: