Allow `recover-module` endpoint to recover multiple modules at once.

[kuasha420]

Feature Description

In #4823 a few issues have come up that encourages multiple or batched module recovery operation. This is also a common case that should be handled in the API level so we do not need to make multiple requests to the endpoint to recover more than one module.

#5297 introduces a barbaric way to recover multiple modules. It still makes multiple API calls, obviously, but it updates the recoverable module lists after all module recovery requests are settled. Once the API supports multiple module module recovery, we can refactor the recoverModules action and potentially get rid of recoverModule action altogether.

This, along with #5287 can reduce number of API calls and make the experience of module recovery more consistent.

---

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

Acceptance criteria

• The recover-module REST endpoint should be renamed to recover-modules, and updated to allow multiple modules to be recovered in a single request.
  • The error handling should be updated to reflect the batched nature of the request, so a single module recovery failure doesn't result in the failure of the entire request, and individual module error statuses are returned to be handled by the client.
  • The HTTP status code should generally be a 200 even if some modules fail to recover, unless there is an unexpected error.
  • The response should be updated to contain a map of module slugs to error statuses.
• Assuming it a) still exists and b) is not being used the recoverModule action should be removed.
• The recoverModules action should be updated to make use of the renamed & updated recover-modules endpoint, making a single request to recover all specified modules.

Implementation Brief

• In includes/Core/Modules/Modules.php:
  • In the get_rest_routes() method, locate the route definition for core/modules/data/recover-module:
    • Re-define the route to be recover-modules (plural).
    • Re-structure the callback function:
      • It should accept slugs instead of slug in $request['data']. $request['data']['slugs'] will be an array of module slugs that need to be recovered.
      • Create an array and assign it to a variable named $response. This array should have two nested empty arrays with the keys: success and error.
      • Loop through the $request['data']['slugs'] array and for each of the slug, do the following:
        • Execute all the checks and the module settings update that we currently perform for recovering a single module.
        • If everything was successful, add an element to the $response['success'] array with the current slug as the key, and true (boolean) as the value.
        • If an error was encountered:
          • Add an element to the $response['success'] array with the current slug as the key, and false (boolean) as the value.
          • Add an element to the $response['error'] array with the current slug as the key, and the error (WP_Error) as the value.
      • Return a WP_REST_Response with the $response array passed to it.
• In assets/js/googlesitekit/modules/datastore/modules.js:
  • Update the fetchRecoverModuleStore function:
    • Rename it to fetchRecoverModulesStore (plural).
    • Change baseName to recoverModules.
    • Update the controlCallback property function:
      • Replace recover-module with recover-modules (plural).
      • Replace all instances of slug with slugs (plural).
    • Update the argsToParams and validateParams property functions to replace all instances of slug with slugs.
  • Update the recoverModules action:
    • Re-structure the second parameter function of the createValidatedAction function:
      • Using the yield keyword, call the fetchRecoverModulesStore.actions.fetchRecoverModules() action with slugs passed to it and access the response property from the object returned by it (by destructuring it).
      • Destructure the response object and access success and error from it.
      • For each module slug (key) in the success object which has the value of true (i.e. successful recoveries), reload its module settings by dispatching fetchGetSettings from the respective module store. The module store name can be obtained using the getModuleStoreName selector from the core/modules store.
      • If the success object is not an empty object:
        • Using the yield keyword, call the fetchGetModulesStore.actions.fetchGetModules() action.
        • Using the yield keyword, refresh the list of recoverable modules by dispatching the invalidateResolution action from the core/modules store, passing the string getRecoverableModules and and an empty array as parameters.
        • Using the yield keyword, refresh user capabilities from the server by dispatching the refreshCapabilities action from the core/user store.
      • At last, return an object containing response.
  • In the store variable assignment, inside the Data.combineStores() call, reflect this change of fetchRecoverModuleStore to fetchRecoverModulesStore (plural).
• In assets/js/components/dashboard-sharing/ModuleRecoveryAlert/index.js:
  • Update the getRecoveryError variable assignment to call the getErrorForAction selector with recoverModules (plural) passed as the parameter, instead of recoverModule.
  • Update the clearErrors dispatch to use recoverModules (plural) as the parameter, instead of recoverModule.
• In assets/js/components/dashboard-sharing/ModuleRecoveryAlert/index.stories.js:
  • Replace all instances of recover-module with recover-modules.
  • Ensure that the stories work as expected. If not, make necessary changes.

Test Coverage

• In tests/phpunit/integration/Core/Modules/ModulesTest.php:
  • Replace all instances of recover-module with recover-modules.
  • Update all WP_REST_Request() calls to the refactored endpoint to reflect the new request and response structure.
  • Fix any other failing test.
• In assets/js/googlesitekit/modules/datastore/modules.test.js:
  • Replace all instances of recover-module with recover-modules.
  • Update all fetchMock calls and relevant expect calls to reflect the new request and response structure.
  • Fix any other failing test.

QA Brief

• This doesn't include any visual change.
• Ensure that the dashboardSharing feature flag is enabled in the tester plugin.
• Set up Site Kit with a few/all modules.
• Using the Dashboard Sharing settings modal, share access to those modules with other administrators.
• Sign up into Site Kit with a different administrator user.
• Disconnect Site Kit from the first administrator user so that the modules become recoverable.
• As the second administrator user, try to recover the modules.
• Verify that the module recovery occurs smoothly.
• In the Network tab of the browser developer tools, verify that only one request is made to the /google-site-kit/v1/core/modules/data/recover-modules endpoint to recover all modules, instead of multiple requests to /google-site-kit/v1/core/modules/data/recover-module, one for each module.
• If an error is encountered, verify that it is displayed correctly.

Changelog entry

[techanvil]

Sounds like a plan @kuasha420, I've added ACs, assigning to @aaemnnosttv to take a look.

[aaemnnosttv]

@techanvil the ACs here look like a good start. However, since this is somewhat of a batch request, there are some additional considerations that should probably be included here as well.

For example, error handling should be per-module and the response code should probably generally be a successful response unless the request itself was made incorrectly (i.e. did not provide a required parameter, etc). If one module was recovered successfully, and another failed to be recovered, we wouldn't want to fail the entire request but the client should be aware of this. We would then need to make sure we handle any errors from the individual modules attempted to be recovered on the client as we wouldn't be able to rely on the request failing to indicate this as we can with an individual request.

[techanvil]

Thanks @aaemnnosttv, that is a good point and I have added a bit of detail about that to the AC. Please let me know if you want any further detail or changes made.

[aaemnnosttv]

Thanks @techanvil, this looks good and I think we can move forward with it. One thing I would flag for the IB (and its review) is the ModuleRecoveryAlert which reads the individual errors from the error store for each, so this should be maintained or otherwise considered in the changes here. cc: @tofumatt @eugene-manuilov

[techanvil]

Nice work @nfmohit. Maybe it could be written a bit less verbosely, but it reads very well as it stands. It can be hard to know where to draw the line!

IB :white_check_mark:

[nfmohit]

Thank you very much for the feedback, @techanvil!

[wpdarren]

QA Update: ✅

Verified:

• The module recovery occurs with no errors.
  • In the Network tab of the browser developer tools, only one request is made to the /google-site-kit/v1/core/modules/data/recover-modules endpoint to recover all modules.
  • I ran the same test on latest version of Site Kit and can see in the network table that /google-site-kit/v1/core/modules/data/recover-module appears in multiple requests. This no longer occurs on the develop branch.