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

Update triggerSurvey to be no-op for non-authenticated users #4806

Closed
aaemnnosttv opened this issue Feb 8, 2022 · 2 comments
Closed

Update triggerSurvey to be no-op for non-authenticated users #4806

aaemnnosttv opened this issue Feb 8, 2022 · 2 comments
Labels
P0 High priority QA: Eng Requires specialized QA by an engineer Rollover Issues which role over to the next sprint Type: Enhancement Improvement of an existing feature

Comments

@aaemnnosttv
Copy link
Collaborator

aaemnnosttv commented Feb 8, 2022

Feature Description

In the context of dashboard sharing, non-authenticated users will be able to use Site Kit dashboards where we currently trigger a number of surveys. This action however requires an access token which non-authenticated users don't have.

Accordingly, this action should be updated to be a no-op in the event the current user isn't authenticated.


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

  • The triggerSurvey action on the core/user store should be updated to bypass its fetch request (similar to the logic for a an existing current survey) if the current user is not authenticated
  • Test coverage should be updated to cover this change
    • Existing test coverage should be updated to cover the case for users that are authenticated

Implementation Brief

  • In assets/js/googlesitekit/datastore/user/surveys.js, within the triggerSurvey action creator (generator) function:
    • Call the isAuthenticated() selector and if it returns false, return an empty object (bail early).

Test Coverage

  • In assets/js/googlesitekit/datastore/user/surveys.test.js:
    • Add a test to the triggerSurvey group that checks if the fetch is bypassed if the current user is not authenticated.

QA Brief

QA Eng

  • Make sure the added code meets the AC and the tests are sufficient and passing.

Changelog entry

  • Prevent surveys from triggering for non-authenticated users on a shared dashboard.
@aaemnnosttv aaemnnosttv added P0 High priority Type: Enhancement Improvement of an existing feature labels Feb 8, 2022
@jimmymadon jimmymadon assigned jimmymadon and unassigned jimmymadon Feb 20, 2022
@tofumatt tofumatt self-assigned this Feb 21, 2022
@tofumatt
Copy link
Collaborator

IB ✅

@tofumatt tofumatt removed their assignment Feb 21, 2022
@jimmymadon jimmymadon self-assigned this Feb 27, 2022
@jimmymadon jimmymadon removed their assignment Mar 11, 2022
@FlicHollis FlicHollis added the Rollover Issues which role over to the next sprint label Mar 14, 2022
@tofumatt tofumatt assigned tofumatt and unassigned tofumatt Mar 14, 2022
@wpdarren wpdarren added the QA: Eng Requires specialized QA by an engineer label Mar 15, 2022
@techanvil techanvil self-assigned this Mar 17, 2022
@techanvil
Copy link
Collaborator

techanvil commented Mar 17, 2022

QA ✅

  • Confirmed the added code meets the AC.
  • Verified the tests pass and provide good coverage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P0 High priority QA: Eng Requires specialized QA by an engineer Rollover Issues which role over to the next sprint Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

7 participants