Skip to content

Fix: 'Set opt-in notification as seen' returns 400 on repeat visits#23056

Open
JorPV wants to merge 1 commit intotrunkfrom
seen-opt-in-notification_locale_user-returns-400
Open

Fix: 'Set opt-in notification as seen' returns 400 on repeat visits#23056
JorPV wants to merge 1 commit intotrunkfrom
seen-opt-in-notification_locale_user-returns-400

Conversation

@JorPV
Copy link
Contributor

@JorPV JorPV commented Mar 6, 2026

Context

When a user visits the General page after the task list opt-in notification has already been seen, the browser console logs: Error setting opt-in notification as seen: Object { success: false, status: 400 }. This happens on every page load after the first time.

Root cause

This was introduced by #22960, which replaced the Toast with a ModalNotification (HeadlessUI Dialog). As part of that change, OptInContainer was refactored to always render TaskListOptInNotification and control visibility via the isOpen prop, instead of conditionally mounting/unmounting it.

Previously, TaskListOptInNotification only mounted when the notification needed to be shown. Its useEffect (with [] deps) called setOptInNotificationSeen("task_list") on mount — which was correct, since mounting only happened once when the notification was actually visible.

After #22960, the component always mounts (even when isOpen=false), so two things go wrong:

  1. JS: The useEffect fires on every page load regardless of whether the notification is visible, sending a POST to /yoast/v1/seen-opt-in-notification every time.
  2. PHP: On subsequent loads the user meta is already '1', so update_user_meta() returns false (no change was made). The endpoint interprets false as failure and returns a 400 status.

Summary

This PR can be summarized in the following changelog entry:

  • Fixes an unreleased bug where a 400 error was logged in the console on every General page load after the task list opt-in notification had already been seen.

Relevant technical choices:

  • JS — Conditional rendering restored (opt-in-container.js): OptInContainer now returns null when isOpen is false, so TaskListOptInNotification only mounts when the notification is actually visible. This prevents the useEffect from firing unnecessary API calls. The ModalNotification component works fine being mounted/unmounted — there's no need to keep it always mounted.
  • PHP — Idempotent endpoint (opt-in-route.php): Before calling update_user_meta, the endpoint checks if the meta is already set to true. If so, it returns { success: true, status: 200 } immediately. This is a safety net against the well-known update_user_meta footgun where unchanged values return false.
  • Test — Idempotency test (Set_Opt_In_Seen_Test.php): Added a test that calls the endpoint twice and verifies both calls return 200.

Test instructions

Test instructions for the acceptance test before the PR gets merged

This PR can be acceptance tested by following these steps:

  1. With Yoast Free installed and active, go to the database and remove _yoast_wpseo_task_list_opt_in_notification_seen from wp_usermeta for your user.
  2. Open the browser console.
  3. Navigate to the Yoast General page.
  4. Confirm the opt-in notification appears and no errors are in the console.
  5. Reload the page.
  6. Confirm the notification does not appear and there are no 400 errors in the console.
  7. Navigate to the post editor (e.g. edit a post).
  8. Confirm there are no seen-opt-in-notification errors in the console.

Relevant test scenarios

  • Changes should be tested with the browser console open
  • Changes should be tested on different posts/pages/taxonomies/custom post types/custom taxonomies
  • Changes should be tested on different editors (Default Block/Gutenberg/Classic/Elementor/other)
  • Changes should be tested on different browsers
  • Changes should be tested on multisite

Check the console for any 400 errors related to seen-opt-in-notification.

Test instructions for QA when the code is in the RC

  • QA should use the same steps as above.

Impact check

This PR affects the following parts of the plugin, which may require extra testing:

  • Task list opt-in notification on the General page — verify it still shows correctly for new users and dismisses properly.

Other environments

  • This PR also affects Shopify. I have added a changelog entry starting with [shopify-seo], added test instructions for Shopify and attached the Shopify label to this PR.
  • This PR also affects Yoast SEO for Google Docs. I have added a changelog entry starting with [yoast-doc-extension], added test instructions for Yoast SEO for Google Docs and attached the Google Docs Add-on label to this PR.

Documentation

  • I have written documentation for this change. For example, comments in the Relevant technical choices, comments in the code, documentation on Confluence / shared Google Drive / Yoast developer portal, or other.

Quality assurance

  • I have tested this code to the best of my abilities.
  • During testing, I had activated all plugins that Yoast SEO provides integrations for.
  • I have added unit tests to verify the code works as intended.
  • If any part of the code is behind a feature flag, my test instructions also cover cases where the feature flag is switched off.
  • I have written this PR in accordance with my team's definition of done.
  • I have checked that the base branch is correctly set.
  • I have run grunt build:images and commited the results, if my PR introduces new images or SVGs.

Innovation

  • No innovation project is applicable for this PR.
  • This PR falls under an innovation project. I have attached the innovation label.
  • I have added my hours to the WBSO document.

Fixes 2896

@JorPV JorPV added the changelog: bugfix Needs to be included in the 'Bugfixes' category in the changelog label Mar 6, 2026
@JorPV JorPV requested a review from Copilot March 6, 2026 15:01
@JorPV JorPV added changelog: non-user-facing Needs to be included in the 'Non-userfacing' category in the changelog and removed changelog: bugfix Needs to be included in the 'Bugfixes' category in the changelog labels Mar 6, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes repeated 400 errors caused by the “set opt-in notification as seen” request being triggered on every General page load after the notification had already been seen, and makes the backend endpoint safely idempotent.

Changes:

  • Restore conditional rendering of the opt-in notification so it only mounts (and triggers its “seen” side-effect) when actually shown.
  • Make the REST endpoint return success when the “seen” user meta is already set, avoiding update_user_meta()’s “unchanged value returns false” behavior.
  • Add a WP test that calls the endpoint twice and asserts both responses are 200/success.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
packages/js/src/general/components/opt-in-container.js Prevents unnecessary mounting/API calls by returning null when isOpen is false.
src/general/user-interface/opt-in-route.php Adds an early-success path when the meta is already set; reuses a computed $meta_key.
tests/WP/General/User_Interface/Set_Opt_In_Seen_Test.php Adds coverage to ensure the endpoint remains idempotent on repeated calls.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@JorPV JorPV added this to the 27.2 milestone Mar 6, 2026
@coveralls
Copy link

Pull Request Test Coverage Report for Build 685e10595c107c93c4c7f33daa345ed2a716fc28

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 10 of 12 (83.33%) changed or added relevant lines in 2 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.6%) to 53.915%

Changes Missing Coverage Covered Lines Changed/Added Lines %
packages/js/src/general/components/opt-in-container.js 0 2 0.0%
Files with Coverage Reduction New Missed Lines %
packages/js/src/general/components/opt-in-container.js 1 5.88%
Totals Coverage Status
Change from base Build ede936a6e1168eb354239ed07e66df72bb332b6d: 0.6%
Covered Lines: 34247
Relevant Lines: 63656

💛 - Coveralls

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog: non-user-facing Needs to be included in the 'Non-userfacing' category in the changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants