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

Add storeName argument to createErrorStore #5235

Closed
tofumatt opened this issue May 19, 2022 · 2 comments
Closed

Add storeName argument to createErrorStore #5235

tofumatt opened this issue May 19, 2022 · 2 comments
Labels
P0 High priority Type: Enhancement Improvement of an existing feature

Comments

@tofumatt
Copy link
Collaborator

tofumatt commented May 19, 2022

Feature Description

In order for #5234 to be implemented, each error store needs awareness of its storeName. See the design doc for more info: https://docs.google.com/document/d/1QIruz3fJSDQJ2cILtBHlU6ow8esyaVS3mcuaqt5QQR8/edit?resourcekey=0-nM2tn9DOjjHDWTtspbERkA#heading=h.xleqo0mvx6mv

We need to add a storeName argument to createErrorStore, and also add this argument to every current call to createErrorStore we make. This way, all errors will be able to return the storeName used.


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

Acceptance criteria

  • createErrorStore should take a new argument: storeName. It should be required argument unless there's a clear reason why it can't be for some code paths.
  • All existing calls to createErrorStore should be updated to use this argument, supplying the storeName they're associated with.
  • The store name of the error store should be saved in state so selectors for this error store can use it. (It will be made available to all errors returned by getErrorForSelector, see: Add new selectorData object in getErrorForSelector #5234, for now it just needs to accept the argument as a required argument.)

Implementation Brief

  • In assets/js/googlesitekit/data/create-error-store.js:
    • Add storeName (required) param.
    • Use invariant() method to validate if the storeName is passed.

Update the existing createErrorStore( storeName ) calls to pass the storename argument:

  • In assets/js/googlesitekit/datastore/forms/index.js, pass CORE_FORMS to createErrorStore( CORE_FORMS ).
  • In assets/js/googlesitekit/datastore/site/index.js, pass CORE_SITE to createErrorStore( CORE_SITE ).
  • In assets/js/googlesitekit/datastore/ui/index.js, pass CORE_UI to createErrorStore( CORE_UI ).
  • In assets/js/googlesitekit/datastore/user/index.js, pass CORE_USER to createErrorStore( CORE_USER ).
  • In assets/js/googlesitekit/modules/datastore/index.js, pass CORE_MODULES to createErrorStore( CORE_MODULES ).
  • In assets/js/googlesitekit/widgets/datastore/index.js, pass CORE_WIDGETS to createErrorStore( CORE_WIDGETS ).
  • In assets/js/googlesitekit/modules/create-module-store.js, pass the storeName variable to all the instances of createErrorStore( storeName ).
  • In assets/js/googlesitekit/modules/create-submit-changes-store.test.js, pass the storeName variable to all the instances of createErrorStore( storeName ).

Test Coverage

  • Add test coverage for receiving the storeName argument

QA Brief

  • This only adds store names to errors, so the best way to QA this is to run through scenarios in many modules/parts of the plugin that would generate errors, like auth errors, network errors, permissions, etc. Errors should continue to appear as normal before this change.

Changelog entry

  • Require storeName parameter for createErrorStore function, to provide it as context for errors.
@tofumatt tofumatt added P0 High priority Type: Enhancement Improvement of an existing feature labels May 19, 2022
@tofumatt tofumatt assigned tofumatt and unassigned tofumatt May 19, 2022
@hussain-t hussain-t self-assigned this May 23, 2022
@hussain-t hussain-t removed their assignment May 31, 2022
@tofumatt tofumatt self-assigned this May 31, 2022
@tofumatt
Copy link
Collaborator Author

Awesome, glad to see we can add this everywhere and make it a required param 🙂

IB ✅

@tofumatt tofumatt assigned tofumatt and unassigned tofumatt May 31, 2022
@tofumatt tofumatt removed their assignment Jun 8, 2022
@hussain-t hussain-t assigned hussain-t and unassigned hussain-t Jun 8, 2022
@felixarntz felixarntz assigned felixarntz and unassigned felixarntz Jun 9, 2022
@wpdarren wpdarren self-assigned this Jun 14, 2022
@wpdarren
Copy link
Collaborator

QA Brief: ✅

Verified:

  • Ran through scenarios in many modules/parts of the plugin that would generate errors, like auth errors, network errors, permissions, etc. Errors should continue to appear as normal before this change.

image

@wpdarren wpdarren removed their assignment Jun 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P0 High priority Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

4 participants