Skip to content

[OLD] frontend: PluginSettings: Refactor local storage and plugin data #2671

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

Closed
wants to merge 2 commits into from

Conversation

vyncent-t
Copy link
Collaborator

@vyncent-t vyncent-t commented Dec 12, 2024

Fixes Issue #2595

Description

  • This PR shrinks the saved JSON data from the plugins information and saves only the name and isEnabled status in local storage. This ensures plugin settings persist when closing and reopening the app.
  • This PR introduces the usePlugins hook into lib/k8s/api/v2
  • It also handles backward compatibility, allowing previously saved settings to be used in this new format.
  • Additionally, it no longer takes old information from the JSON in local storage. Instead, it retrieves the necessary plugin data from the backend when the main Plugin component is called.

Changes

  1. Introduces usePlugins hook: Hook used to fetch the plugins from the backend and return the plugins with their settings
  2. Reduced Storage Data: Only stores essential plugin info (name and isEnabled) in local storage.
  3. Backward Compatibility: Converts old-format data into the new structure on first run.
  4. Data Source Shift: Eliminates use of outdated local storage JSON data, and instead fetches plugin details from the backend when the Plugin component mounts.

How to Test

  1. Setup:
    • Pull this branch and run the application.
  2. Verify Plugin Settings:
    • Open headlamp in app mode
    • Navigate to the settings page then to plugins settings tab
    • Check local storage item headlampPluginSettings
  3. Check Backward Compatibility:
    • Start with an older version of the app or start from main.
    • Navigate to the settings page then to plugins settings tab
    • Check local storage item headlampPluginSettings
    • Switch to this branch and run the app.
    • Confirm that previously stored plugin settings are recognized and converted correctly.
  4. Backend Data:
    • Verify that after loading, the plugin details (other than name and isEnabled) are fetched from the backend and displayed correctly in the UI.
    • Switch plugins off and on and observe the changes in local storage and in app

Notes

  • If you have preexisting local storage data, this PR will convert it automatically. You can reset the local storage if you return to main and go to the same plugin settings paage.
  • Double-check the logs to ensure no errors appear during the migration from old data to new data.

@vyncent-t vyncent-t self-assigned this Dec 12, 2024
@vyncent-t vyncent-t force-pushed the reduce-plugin-settings-storage branch from 005cabd to d2f8668 Compare December 12, 2024 01:01
@vyncent-t vyncent-t marked this pull request as ready for review December 12, 2024 01:03
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Dec 12, 2024
@vyncent-t
Copy link
Collaborator Author

Last push adds backwards comp so that all settings wont be set to off or on

@vyncent-t
Copy link
Collaborator Author

I have tried to keep the changes as small as possible to accomplish everything it needs to, I am not using the map object to save information anymore, although to hit these targets I still had to use most of what I had reworked previously

@joaquimrocha @sniok

for this PR the targets it needs to hit were:

  • not have the plugins version/data coming from a client-side stored version of it

The plugin data comes from the backend, reused the same method of reaching it from the previous rework

  • avoid having to store all that data when what we are interested in is checking whether the plugins are enabled on the user's side

The data is now trimmed down to just being the name and isEnabled, it is now being used the same way as the original settings where this local saved item is how new app start ups save plugin settings

  • need to be backwards compatible with the previous settings method

The local storage handling logic checks for the old format and changes it to the new format

@vyncent-t vyncent-t marked this pull request as draft December 12, 2024 01:13
@vyncent-t vyncent-t changed the title frontend: PluginSettings: Refactor local storage and plugin data WIP - frontend: PluginSettings: Refactor local storage and plugin data Dec 12, 2024
@vyncent-t vyncent-t marked this pull request as ready for review December 12, 2024 01:16
@vyncent-t vyncent-t changed the title WIP - frontend: PluginSettings: Refactor local storage and plugin data frontend: PluginSettings: Refactor local storage and plugin data Dec 12, 2024
@vyncent-t vyncent-t marked this pull request as draft December 12, 2024 15:03
@vyncent-t vyncent-t force-pushed the reduce-plugin-settings-storage branch from d2f8668 to 56e912f Compare December 12, 2024 18:37
@vyncent-t vyncent-t changed the title frontend: PluginSettings: Refactor local storage and plugin data WIP - frontend: PluginSettings: Refactor local storage and plugin data Dec 12, 2024
@vyncent-t vyncent-t force-pushed the reduce-plugin-settings-storage branch from 56e912f to c150408 Compare December 19, 2024 16:10
@sniok
Copy link
Collaborator

sniok commented Dec 19, 2024

Some things to do to avoid duplicating of logic:

  • refactor fetching logic into a single place that both PluginSettings and fetchAndExecutePlugins use
  • use react-query for fetching to have caching and avoid refetching the same stuff

@vyncent-t vyncent-t force-pushed the reduce-plugin-settings-storage branch from c150408 to 83f279e Compare December 19, 2024 18:45
@vyncent-t vyncent-t force-pushed the reduce-plugin-settings-storage branch from 83f279e to 98c1b58 Compare January 2, 2025 15:24
@vyncent-t vyncent-t force-pushed the reduce-plugin-settings-storage branch from 98c1b58 to 6cea967 Compare January 23, 2025 16:00
@vyncent-t vyncent-t changed the title WIP - frontend: PluginSettings: Refactor local storage and plugin data frontend: PluginSettings: Refactor local storage and plugin data Jan 23, 2025
@vyncent-t vyncent-t marked this pull request as ready for review January 23, 2025 16:01
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Jan 23, 2025
@vyncent-t vyncent-t force-pushed the reduce-plugin-settings-storage branch 3 times, most recently from b39d2a4 to 93ed4b0 Compare January 27, 2025 20:12
@vyncent-t vyncent-t force-pushed the reduce-plugin-settings-storage branch from 93ed4b0 to 47dfb30 Compare February 6, 2025 15:13
@vyncent-t
Copy link
Collaborator Author

pushing rebase of current main

@vyncent-t
Copy link
Collaborator Author

note: that last disabled e2e needs to be redone in a later PR as it is out dated with the changes made to two different branches, this branch and the plugins name change branch, this is in my stack for playwright changes I will be working on later

@vyncent-t vyncent-t force-pushed the reduce-plugin-settings-storage branch from f7991dc to f7d5e24 Compare February 25, 2025 14:55
@illume
Copy link
Collaborator

illume commented Feb 28, 2025

@vyncent-t It seems there’s conflicts now. Can you please fix them?

Also, can you please check if the PR description is up to date? I see a lot of changes since the last PR description edit, so I guess if needs updating.

@vyncent-t vyncent-t force-pushed the reduce-plugin-settings-storage branch from f7d5e24 to 77e1d63 Compare February 28, 2025 15:30
@vyncent-t vyncent-t force-pushed the reduce-plugin-settings-storage branch 5 times, most recently from 944d4bc to e4e4515 Compare March 10, 2025 16:55
@vyncent-t vyncent-t force-pushed the reduce-plugin-settings-storage branch 5 times, most recently from 2b038b1 to 2b8da16 Compare March 20, 2025 15:14
This PR reduces the size of plugin information saved in local storage,
it also introduces the getPlugins hook for fetching plugin information.

Signed-off-by: Vincent T <[email protected]>
@vyncent-t vyncent-t force-pushed the reduce-plugin-settings-storage branch 6 times, most recently from 18c9206 to 3dc54a8 Compare March 25, 2025 19:19
since we now use a multi cluster structure to run our tests,
the cluster navigation step must be its own full nav step, different from navigateToPage, if it is tied to an auth step.

Signed-off-by: Vincent T <[email protected]>
@vyncent-t vyncent-t force-pushed the reduce-plugin-settings-storage branch from 3dc54a8 to a210f61 Compare March 25, 2025 20:03
@k8s-triage-robot
Copy link

Unknown CLA label state. Rechecking for CLA labels.

Send feedback to sig-contributor-experience at kubernetes/community.

/check-cla
/easycla

Copy link

CLA Not Signed

@k8s-ci-robot k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Apr 1, 2025
@vyncent-t vyncent-t changed the title frontend: PluginSettings: Refactor local storage and plugin data [OLD] frontend: PluginSettings: Refactor local storage and plugin data Apr 15, 2025
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 15, 2025
@illume
Copy link
Collaborator

illume commented Apr 16, 2025

Closing because new PR is here: #3123

@illume illume closed this Apr 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. size:L This PR changes 100-499 lines, ignoring generated files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants