Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fetch sensitive feature flag stays active even if sensitive content feature flag is disabled #2994

Closed
sarayourfriend opened this issue Sep 6, 2023 · 1 comment · Fixed by #2995
Assignees
Labels
🕹 aspect: interface Concerns end-users' experience with the software 🛠 goal: fix Bug fix 🟨 priority: medium Not blocking but should be addressed soon 🧱 stack: frontend Related to the Nuxt frontend

Comments

@sarayourfriend
Copy link
Contributor

Description

If the sensitive content feature flag is toggled off, the fetch sensitive feature flag stays on, but without the UI being present to toggle it back off. This means the browser gets stuck in a mode where it always fetches sensitive content without any way of turning it off.

Found while testing #2955

@dhruvkb I'm not sure whether this is a blocker. I think it would be because it would prevent us from fully rolling back the sensitive content feature, even if we turn the sensitive content feature flag off, the session's flag to make requests for sensitive content would be stuck on. This would be bad if we discovered we needed to roll the feature back because of some critical issue (however unlikely that seems at the moment).

Reproduction

  1. Visit https://staging.openverse.org/preferences and turn on the sensitive content feature flag
  2. Visit search and turn on the sensitive content fetching
  3. Go back to https://staging.openverse.org/preferences and turn off the sensitive content feature flag
  4. Visit search and confirm you can still get sensitive results (or, more easily, that the fetch sensitive feature flag is still on in the preferences). I've confirmed locally that this does still fetch the sensitive results.

To emulate the rollback scenario, test it locally by running the dev server and then:

  1. Turn on the sensitive content feature flag and fetch sensitive feature flags
  2. Leaving your browser tab open, update feature-flags.json to set sensitive_content to disabled at the staging level. This will also disable it locally.
  3. Refresh your browser tab and note that the fetch sensitive feature flag is still on and that your requests will still include sensitive content. This is true on main and Separate features into persistent and session scoped cookies #2955, so it isn't an issue with that PR, but the PR also won't fix this.

@dhruvkb Is the best way to fix this by making checks against fetch_sensitive also check the general feature flag? Basically making fetch_sensitive dependent on sensitive_content also being true? Also, do you agree with my assessment of this being an issue due to rollbacks? Even if we don't block on it, I think we could fix this before #2550 is unblocked anyway (:confused:).

@sarayourfriend sarayourfriend added 🟨 priority: medium Not blocking but should be addressed soon 🛠 goal: fix Bug fix 🕹 aspect: interface Concerns end-users' experience with the software 🧱 stack: frontend Related to the Nuxt frontend labels Sep 6, 2023
@dhruvkb
Copy link
Member

dhruvkb commented Sep 6, 2023

The /preferences page is meant to be internal, with those toggles only being affected by people who know what they are doing. So that page accepts accepts all sorts of discrepancies in the flags, e.g. screenshot below where fetching is enabled but sidebar UI hidden.

Screenshot 2023-09-06 at 11 02 55 AM

I do see how the current state is incompatible with rollbacks. The sidebar UI is shown only if sensitive_content flag is true. The results include sensitive content only if fetch_sensitive is true, which can be changed by the general user only if the sidebar is visible.

Rolling back the sidebar to be hidden (by locking the sensitive_content flag to false) would take away the ability of a user to toggle the appearance of sensitive content, thus fixing the fetch_sensitive flag to whatever value is set for the session.

This imo is not a very severe problem, as

  • the value of the fetch_sensitive flag is what was set by the user during the session
  • restarting the browser resets the fetch_sensitive flag back to the default i.e. not fetched

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🕹 aspect: interface Concerns end-users' experience with the software 🛠 goal: fix Bug fix 🟨 priority: medium Not blocking but should be addressed soon 🧱 stack: frontend Related to the Nuxt frontend
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants