Implement new actions for sharing settings

[aaemnnosttv]

Feature Description

---

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

Acceptance criteria

The core/modules store should be extended with the following actions:

• setSharingManagement(moduleSlug: string, management: enum)
  Sets the value for a module's sharing management where management can be one of all_admins or owner
• setSharedRoles(moduleSlug: string, roles: string[])
  Sets the list of roles (role.id[]) the module is shared with
• saveSharingSettings()
  Saves all sharing settings via POST:core/modules/data/sharing-settings using createFetchStore
  • See Create REST controller for dashboard sharing #4481 for the response shape
  • The current and saved settings should be received/refreshed in the state from the response
  • Any newOwnerIDs included in the response should update the modules in state
• The state for settings should work similar to createSettingsStore which stores state for saved settings separately from the current working/draft values that selectors and actions touch
  • Accordingly, an action for receiveGetSharingSettings should also be created which should update both
• Test coverage should be added for all new actions

Implementation Brief

• Create assets/js/googlesitekit/modules/datastore/sharing-settings.js which will contain our actions and reducers.
• The baseInitialState should contain the sharingSettings and savedSharingSettings property, both initialized to an empty object.
• Create fetchSaveSharingSettingsStore using the createFetchStore function with the following details:
  • baseName should be set to saveSharingSettings
  • controlCallback should:
    • require one parameter, an object having the same shape as state.savedSharingSettings.
    • make a POST request to core/modules/data/sharing-settings using API.set, and passing the state.savedSharingSettings as the body.
    • the argsToParams and validateParams should be set as well. Refer to how it's done in other data partials.
    • For the reducerCallback,
      • state.sharingSettings should be updated to the response from the API, i.e both state.sharingSettings and state.saveSharingSettings should be equal.
      • check if newOwnerIDs from the API response is not empty. Refer to Create REST controller for dashboard sharing #4481 for the structure. In this case, loop through the array and update each module owner ID using the setOwnerID action.
• Similar to other data store partials in the folder, add the following actions within the baseAction object:
  • setSharingManagement
    • The constant SET_SHARING_MANAGEMENT having the same value should be added at the beginning of the file.
    • It accepts the moduleSlug and management as mandatory parameters.
    • The corresponding reducer should be added to the reducer function where the management property of the given moduleSlug within the state savedSharingSettings property.
  • setSharedRoles
    • The constant SET_SHARED_ROLES having the same value should be added at the beginning of the file.
    • It accepts the moduleSlug and roles as mandatory parameters.
    • The corresponding reducer should be added to the baseReducers object where the sharedRoles property of the given moduleSlug is set within the state savedSharingSettings property.
  • saveSharingSettings
    • uses the createValidatedAction function which checks whether state.savedSharingSettings is not empty.
    • The function should call the fetchSaveSharingSettingsStore.actions.fetchSaveSharingSettings function, passing the state.savedSharingSettings.
  • receiveGetSharingSettings
    • It should accept a parameter, similar to settings and it should set state.sharingSettings and state.savedSharingSettings with the provided parameter.
• For the newly added actions,
  • use invariant to check for the parameters similar to how it’s done in other data store partials.
  • Refer to the AC for the parameter types.
• Update assets/js/googlesitekit/modules/datastore/index.js to include the new data store partial.

Test Coverage

• Test coverage should be added for all new actions, selectors .

QA Brief

QA

QA:Eng

• Ensure the dashboardSharing feature flag is enabled.
• In the console, run the receiveGetSharingSettings action like so:

googlesitekit.data.dispatch('core/modules').receiveGetSharingSettings({
	'search-console': {
		sharedRoles: [ 'editor', 'subscriber' ],
		management: 'all_admins',
	},
	analytics: {
		sharedRoles: [ 'editor' ],
		management: 'owner',
	},
	'pagespeed-insights': {
		sharedRoles: [ 'editor' ],
		management: 'all_admins',
	},
})

• Note that the receiveGetSharingSettings action has to be dispatched before saveSharingSettings since it sets the sharingSettings in the state. Otherwise, it will throw an error.

  • This is only necessary until Implement new selectors for sharing settings #4795 is merged

• In the console, test invoking the added actions as defined in the ACs

  await googlesitekit.data.dispatch('core/modules')...

• Test saveSharingSettings makes an API call to the sharing-settings endpoint successfully

Changelog entry

• Implement new actions for sharing settings.

[hussain-t]

@asvinb @aaemnnosttv The getSharingSettings() selector is being implemented in #4795.

Add the getSharingSettings selector which returns state.settings

[eugene-manuilov]

• Create assets/js/googlesitekit/modules/create-sharing-store.js which contains the same logic as assets/js/googlesitekit/data/create-settings-store.js in the sense that saved sharing settings and updated settings are saved separately in the state, with the following notes:

@asvinb we don't need a factory for the sharing settings datastore. It should be a simple datastore in the assets/js/googlesitekit/modules/datastore/ folder. The createSettingsStore factory was mentioned as an example of how we can handle the original and changed settings within a datastore.

Could you please update your instructions to explain how to create a simple datastore with the requested actions? Also, please, mention the approach of how to work with changed settings and what should happen when we save them.

[tofumatt]

IB ✅

[aaemnnosttv]

@kuasha420 @hussain-t This issue should not depend on #4481. The two are interconnected but should be able to be implemented separately. I'm going to remove the dependency for now but let me know if you feel it is important to keep for some reason.

[hussain-t]

@aaemnnosttv, there are a couple of reasons I consider #4481 is blocking this ticket though I have done the base work with unit tests and created a draft PR.

1. I wanted to know the exact shape of the sharing settings
2. I want to test the saveSharingSettings action.

[hussain-t]

@aaemnnosttv, we need to move the getSharingSettings selector and resolver from #4795 to this ticket since we need to get the sharingSettings from the getSharingSettings selector to use it in the saveSharingSettings action to pass it to the fetchSaveSharingSettings action like this:

[aaemnnosttv]

there are a couple of reasons I consider #4481 is blocking this ticket though I have done the base work with unit tests and created a draft PR.

1. I wanted to know the exact shape of the sharing settings
2. I want to test the saveSharingSettings action.

1. This is defined in the ACs, so the implementation should be built according to this, however if that is unclear for you please let me know!
2. The Jest tests will be sufficient for this as far as this issue is concerned – I realize this is a bit limited but that's all this issue should be concerned with

we need to move the getSharingSettings selector and resolver from #4795 to this ticket since we need to get the sharingSettings from the getSharingSettings selector to use it in the saveSharingSettings action to pass it to the fetchSaveSharingSettings action like this:

@hussain-t I would prefer to keep these in their respective issues as defined and use a temporary workaround here instead.

E.g. we can manually "select" the state for the action above instead, and refactor it to use the selector in that issue as a simple one line change.

// TODO: Refactor to use `getSharingSettings` selector.
const { sharingSettings } = registry.stores[ CORE_MODULES ].store.getState()

That should unblock this issue and then we can address the above TODO in #4795. If that works for you, then let's move forward with that approach and we can simply update the IB for 4795 with a reminder to address this TODO (I don't think any changes to ACs are necessary here).

[hussain-t]

Thanks, @aaemnnosttv; however, currently, when we call this action, it throws an Invariant Violation param is required error because the state is being set from the getSharingSettings resolver. Currently, we don't have the state; hence it is undefined.

To keep these in their respective issues as defined, I think we can stub the shape of the sharingSettings object with the TODO comment to make it work.

Otherwise, we will have to call the receiveGetSharingSettings action to use the store.getState():

const settings = {
	'search-console': {
		sharedRoles: [ 'editor', 'subscriber' ],
		management: 'all_admins',
	},
	analytics: {
		sharedRoles: [ 'editor' ],
		management: 'owner',
	},
	'pagespeed-insights': {
		sharedRoles: [ 'editor' ],
		management: 'all_admins',
	},
};

yield actions.receiveGetSharingSettings( settings );
const { sharingSettings } = registry.stores[ CORE_MODULES ].store.getState();

IMO, the first would be better, WDYT?

[hussain-t]

Thanks, @aaemnnosttv, as per our discussion I have updated the tests to dispatch the receiveGetSharingSettings action then we should be able to get the sharingSettings from the *.store.getState(). I have also updated the QAB respectively.

[wpdarren]

QA Update: ✅

Verified:

The QAB is highlighted as QA:Eng, but I was intrigued about the Redux dev tool, so I completed part of this.

• The RECEIVE_SAVE_SHARING_SETTINGS action is not available.
• When the receiveGetSharingSettings action and code is run in the console:
  • In the Network tab an API call to the sharing-settings endpoint is made successfully.
  • The RECEIVE_SAVE_SHARING_SETTINGS and RECEIVE_GET_SHARING_SETTINGS actions are available.

Screenshots

Labelled this QA:Eng for the other tasks in the QAB.

• Ensure the code changes met the AC and IB (this can be done in CR. However, just ensure the actions work as expected).
• Ensure the unit tests for the actions are good enough and are passing.

[aaemnnosttv]

⚠️ QA:Eng

Somewhat of a secondary CR here but a few minor things should be fixed.

@hussain-t please see my comments here: #5127 (review)

[hussain-t]

Thanks, @aaemnnosttv. I have addressed your comments in a follow-up PR and assigned this ticket for review.

[jimmymadon]

QA: Eng ✅

• Verified the new actions within the ACs using the recently edited QA brief.

Screenshots:

setSharingManagement

setSharedRoles

saveSharingSettings