3070 bulk editor during ai generation user can edit manually#23452
Conversation
|
A merge conflict has been detected for the proposed code changes in this PR. Please resolve the conflict by either rebasing the PR or merging in changes from the base branch. |
…n-user-can-edit-manually
Coverage Report for CI Build 15Coverage increased (+0.006%) to 55.185%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats💛 - Coveralls |
| describe( "BulkEditorTable", () => { | ||
| beforeAll( () => { | ||
| // The rows read the external-generation flag from the store, so it must be registered. | ||
| registerStore(); |
There was a problem hiding this comment.
The new hasExternalGeneration path isn't covered by a test. registerStore() in beforeAll keeps the existing tests green now that the row reads the store, but nothing asserts the actual new behaviour — that Edit disables table-wide when the flag is set. There's a dedicated test for the sibling flag (disables every row's Edit button while Premium AI suggestions are pending review), and this deserves the mirror:
it( "disables every row's Edit button while an external AI generation is in flight", () => {
dispatch( STORE_NAME ).setHasExternalGeneration( true );
render( <BulkEditorTable items={ items } fieldSet={ searchFieldSet } /> );
expect( screen.getByRole( "button", { name: "Edit What Is SEO" } ) ).toBeDisabled();
expect( screen.getByRole( "button", { name: "Edit On-Page SEO Checklist" } ) ).toBeDisabled();
dispatch( STORE_NAME ).setHasExternalGeneration( false ); // reset — the store is a shared singleton across this file
} );Note the reset: because the store is registered once in beforeAll, a true left set here leaks into every later test in the file. Worth also adding a small reducer test under store/external-generation.test.js to match external-pending-changes.test.js.
| const fillsMetaDescription = useSlotFills( `${ TABLE_CELL_FIELD_SLOT }/metaDescription/${item.id}` ); | ||
| const fillsSocialTitle = useSlotFills( `${ TABLE_CELL_FIELD_SLOT }/socialTitle/${item.id}` ); | ||
| const fillsSocialDescription = useSlotFills( `${ TABLE_CELL_FIELD_SLOT }/socialDescription/${item.id}` ); | ||
| const hasExternalGeneration = useSelect( ( select ) => select( STORE_NAME ).selectHasExternalGeneration(), [] ); |
There was a problem hiding this comment.
Consistency nit (non-blocking): the sibling flag hasExternalPendingChanges is selected once in bulk-editor-content.js and drilled down as a prop (bulk-editor-table → table-body → table-row), whereas hasExternalGeneration is read here with useSelect inside every row. Two conceptually identical global flags now follow two different patterns, and this variant adds a per-row store subscription (N per page) instead of one at the top. Unless there's a reason to diverge, I'd source it the same way as hasExternalPendingChanges and pass it through as a prop — it keeps the row presentational and the two flags symmetrical.
There was a problem hiding this comment.
Damn, I thought about avoiding prop drilling and didn't think about the multiplication of the store subscription...
…es flag already is, for uniformity
…70-bulk-editor-during-ai-generation-user-can-edit-manually
|
CR & Acceptance: ✅ |
Context
Note: to be tested together with https://github.com/Yoast/wordpress-seo-premium/pull/5021
Summary
This PR can be summarized in the following changelog entry:
Relevant technical choices:
Test instructions
Test instructions for the acceptance test before the PR gets merged
This PR can be acceptance tested by following these steps:
Prerequisites
Test the fix
Yoast->Tools->Bulk editorEditbutton stays deactivated while the AI generation is runningRelevant test scenarios
Test instructions for QA when the code is in the RC
Impact check
This PR affects the following parts of the plugin, which may require extra testing:
Other environments
[shopify-seo], added test instructions for Shopify and attached theShopifylabel to this PR.[yoast-doc-extension], added test instructions for Yoast SEO for Google Docs and attached theGoogle Docs Add-onlabel to this PR.Documentation
Quality assurance
grunt build:imagesand committed the results, if my PR introduces or edits images or SVGs.Innovation
innovationlabel.Fixes https://github.com/Yoast/plugins-automated-testing/issues/3070