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

frontend: PluginSettings: Refactor local storage and plugin data #2671

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

vyncent-t
Copy link
Contributor

@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.
  • 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. Reduced Storage Data: Only stores essential plugin info (name and isEnabled) in local storage.
  2. Backward Compatibility: Converts old-format data into the new structure on first run.
  3. 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
Contributor Author

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

@vyncent-t
Copy link
Contributor 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
Contributor

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants