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

feat: Cleanup orphan extensions' settings #646

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

Conversation

yassine-sallemi
Copy link
Contributor

Motivation

Unnecessary settings of orphan extensions are now deleted to avoid errors in frontend

Type of change:

  • New feature (non-breaking change which adds functionality)

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas

Copy link
Contributor

@marrouchi marrouchi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest we make the service part of the setting module, let's call it CleanUpService.

Let's for now handle only channels and helpers.

@yassine-sallemi
Copy link
Contributor Author

I think that making the new CleanupService part of the SettingModule will cause a problem, as it is loaded before the extensions are loaded, so it may delete some saved settings of some extensions, and therefore, you'll need to modify some settings upon each restart of the api server

@marrouchi
Copy link
Contributor

@yassine-sallemi Then I suggest we add onModuleInit for each module (HelperService, ChannelService), so each one cleans up his own settings.

@yassine-sallemi yassine-sallemi force-pushed the feat/orphan-extensions-settings-cleanup branch from 9a2b80f to a372529 Compare January 30, 2025 18:25
@yassine-sallemi yassine-sallemi force-pushed the feat/orphan-extensions-settings-cleanup branch from a372529 to eacd1a1 Compare January 30, 2025 19:13
api/src/plugins/plugins.service.ts Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants