upcoming: [DI-19328] - Added capability to save & retrieve CloudPulse user preferences

[nikhagra-akamai]

Description 📝

Added a feature to save the users global filter selection to preferences & load it back when user revisit the page.

Changes 🔄

Added UserPreference utility file to handle all the preferences related operations
Modified all the global filter components to by default get selected with preferences value, if any present.

Target release date 🗓️

Please specify a release date to guarantee timely review of this PR. If exact date is not known, please approximate and update it as needed.

Preview 📷

Include a screenshot or screen recording of the change

💡 Use <video src="" /> tag when including recordings in table.

Before	After

How to test 🧪

Note: Since Dashboards api is not ready to use, therefore have to enable mock services for testing. But with mock services preferences will not be saved properly, that's why it can be verified from the network tab that preferences api call is going on initial loading of the CloudPulse page to retrieve the saved preferences as well as on changing of every global filter value to update it.

1. Select values from global filter drop-downs.
2. Refresh the page or open in new tab or new window
3. You will see that your previous selections are retrieved from the preferences ( no saved preferences will be retrieved in case of mock user).

As an Author I have considered 🤔

Check all that apply

• 👀 Doing a self review
• ❔ Our contribution guidelines
• 🤏 Splitting feature into small PRs
• ➕ Adding a changeset
• 🧪 Providing/Improving test coverage
• 🔐 Removing all sensitive information from the code and PR description
• 🚩 Using a feature flag to protect the release
• 👣 Providing comprehensive reproduction steps
• 📑 Providing or updating our documentation
• 🕛 Scheduling a pair reviewing session
• 📱 Providing mobile support
• ♿ Providing accessibility support

[mjac0bs]

@nikhagra Before we review this PR, can we get a complete PR description? Unless there are no visual changes, the Preview section is helpful to see the UI differences at a glance. Including steps in How to test allows us to verify we can reproduce your changes as expected and edge cases have been accounted for.

[nikhagra-akamai]

@nikhagra Before we review this PR, can we get a complete PR description? Unless there are no visual changes, the Preview section is helpful to see the UI differences at a glance. Including steps in How to test allows us to verify we can reproduce your changes as expected and edge cases have been accounted for.

@mjac0bs Yes working on it

[venkymano-akamai]

LGTM

[bnussman-akamai]

I worry that /v4/profile/preferences won't be a good long term solution for storing this stuff.

[bnussman-akamai]

We should be able to use our React Query data layer for anything related to user preferences. (packages/manager/src/queries/profile/preferences.ts). We try to use that instead of interfacing with @linode/api-v4 functions directly

[mjac0bs]

To add to Banks, here's reference point in our docs: https://linode.github.io/manager/development-guide/05-fetching-data.html#react-query

[kmuddapo]

Thanks!! for the suggestion. We will take this up as part of enhancements later to use React Query Data layer via https://track.akamai.com/jira/browse/DI-19503 as in Q3 we have a commitment for beta with minimum functionalities. Hope this is fine for now.

[mjac0bs]

@kmuddapo Use of React Query is something we'd need to see in this PR to get it merged, rather than later. Using direct api-v4 functions is rare in our codebase, and it may be acceptable on a one-off basis, but our best practice is to use RQ almost all the time -- this offers caching benefits, helps us manage states (loading, errors), as well as provides overall consistency in code. (See here for our section of documentation on this.) In this PR, even with debouncing, we'd be making several requests to /profile/preferences and we should be leveraging the RQ cache.

@nikhagra Please update to use React Query here in place of the api-v4 functions, and let us know if you run into any questions on implementing that switch.

[github-actions]

Coverage Report: ✅
Base Coverage: 82.31%
Current Coverage: 82.31%

[mjac0bs]

Thanks for the PR, @nikhagra. Left some comments throughout, and I do agree with Banks in thinking that a user's profile preferences doesn't feel like the best place to store CloudPulse prefs... were there other options discussed?

As far as the UI: I am seeing disabled fields for the dashboard, region, and resources. Should I be testing this with additional configurations on my account? I can interact with the fields if I turn on the Mock Service Worker via our dev tools, but that won't save the user's values.

[mjac0bs]

Let's rename this file utils.ts to remain consistent with convention in the codebase.

[nikhagra-akamai]

Changed the case of file name to camel case because in future more util files will be added for different functionalities

[nikhagra-akamai]

Thanks for the PR, @nikhagra. Left some comments throughout, and I do agree with Banks in thinking that a user's profile preferences doesn't feel like the best place to store CloudPulse prefs... were there other options discussed?

As far as the UI: I am seeing disabled fields for the dashboard, region, and resources. Should I be testing this with additional configurations on my account? I can interact with the fields if I turn on the Mock Service Worker via our dev tools, but that won't save the user's values.

Currently the endpoint for Dashboard component is not yet ready to use, that's why we have to use mock data for now.

[jcallahan-akamai]

Currently the endpoint for Dashboard component is not yet ready to use, that's why we have to use mock data for now.

@nikhagra I have the same question as @mjac0bs. The first testing instruction you have provided is: "Select values from global filter drop-downs."

Yet all I see is disabled fields:

In your comment above you acknowledge that we must use mock data. If that is the case, please include steps to configure mock data appropriately in your testing instructions.

[nikhagra-akamai]

Currently the endpoint for Dashboard component is not yet ready to use, that's why we have to use mock data for now.

@nikhagra I have the same question as @mjac0bs. The first testing instruction you have provided is: "Select values from global filter drop-downs."

Yet all I see is disabled fields:

In your comment above you acknowledge that we must use mock data. If that is the case, please include steps to configure mock data appropriately in your testing instructions.

@jcallahan-akamai I've updated the testing instructions in the description. Since we can only test now as mock user, so preferences won't be saved for mock user but we can verify the same from network tab for preferences API call on value change.

[mjac0bs]

Thanks for addressing some initial feedback @nikhagra. Testing instructions are looking good now - appreciate the update. I confirmed the network requests and responses upon updating /profile/preferences.

Two more bits of feedback:

1. Please see my comment about React Query.

2. I am still somewhat concerned about storing these preferences in a user's profile preferences, rather than that information being returned from an aclp endpoint. Do we anticipate there being additional data stored in user preferences for CloudPulse?

As is, it feels out of place. For context, /profile/preferences is generally used to store things like what theme (light/dark/system) colors to use, or whether banners have been dismissed.

[nikhagra-akamai]

Thanks for addressing some initial feedback @nikhagra. Testing instructions are looking good now - appreciate the update. I confirmed the network requests and responses upon updating /profile/preferences.

Two more bits of feedback:

1. Please see my comment about React Query.
2. I am still somewhat concerned about storing these preferences in a user's profile preferences, rather than that information being returned from an aclp endpoint. Do we anticipate there being additional data stored in user preferences for CloudPulse?

As is, it feels out of place. For context, /profile/preferences is generally used to store things like what theme (light/dark/system) colors to use, or whether banners have been dismissed.

@mjac0bs

1. Sure will check.
2. More additional data will be stored in user preference for CloudPulse as we are going to add more features moving forward.

[carrillo-erik]

@nikhagra Thanks for keeping up with all the PR feedback, I left some comments myself. It's almost there!

[mjac0bs]

Thanks Nikhil, approving pending a couple smaller bits of feedback are addressed, like naming the hooks as such and cleaning up another any.

Also left a few non-blocking nits about JSDocs comments, linter warnings, etc.

[mjac0bs]

Nice to have: JSDocs comments are useful to include for custom hooks and utils. useRestrictedGlobalGrantCheck and usePagination are a couple examples of where we do this.

[nikhagra-akamai]

sure

[mjac0bs]

Confirmed the only test failure is on linodes/clone-linode.spec.ts, which has been flaky and unrelated to this PR. Going to merge.