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.

Sign up for GitHub

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

Jump to bottom

Desktop: Fixes #10382: Fix default values for plugin settings stored in settings.json not applied #10383

Merged
merged 2 commits into from
May 2, 2024
Merged

Desktop: Fixes #10382: Fix default values for plugin settings stored in settings.json not applied #10383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e682d5c
Desktop: Fixes #10382: Fix default values for settings.json plugin se…
personalizedrefrigerator Apr 28, 2024
8bede60
Revert unnecessary test change
personalizedrefrigerator Apr 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 7 additions & 2 deletions packages/lib/models/Setting.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,20 @@ describe('models/Setting', () => {
await expectThrow(async () => Setting.value('itsgone'), 'unknown_key');
}));

it('should allow registering new settings dynamically', (async () => {
it.each([
SettingStorage.Database, SettingStorage.File,
])('should allow registering new settings dynamically (storage: %d)', (async (storage) => {
await expectThrow(async () => Setting.setValue('myCustom', '123'), 'unknown_key');

await Setting.registerSetting('myCustom', {
public: true,
value: 'default',
type: Setting.TYPE_STRING,
storage,
});

expect(Setting.value('myCustom')).toBe('default');

await expectNotThrow(async () => Setting.setValue('myCustom', '123'));

expect(Setting.value('myCustom')).toBe('123');
Expand Down Expand Up @@ -328,7 +333,7 @@ describe('models/Setting', () => {

expect((await Setting.loadOne('locale')).value).toBe('fr_FR');
expect((await Setting.loadOne('theme')).value).toBe(Setting.THEME_DARK);
expect((await Setting.loadOne('sync.target')).value).toBe(undefined);
expect((await Setting.loadOne('sync.target'))).toBe(null);
});

it('should save sub-profile settings', async () => {
Expand Down
13 changes: 9 additions & 4 deletions packages/lib/models/Setting.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2103,6 +2103,7 @@ class Setting extends BaseModel {
}

// Low-level method to load a setting directly from the database. Should not be used in most cases.
// Does not apply setting default values.
public static async loadOne(key: string): Promise<CacheItem | null> {
if (this.keyStorage(key) === SettingStorage.File) {
let fileSettings = await this.fileHandler.load();
Expand All @@ -2113,10 +2114,14 @@ class Setting extends BaseModel {
fileSettings = mergeGlobalAndLocalSettings(rootFileSettings, fileSettings);
}

return {
key,
value: fileSettings[key],
};
if (key in fileSettings) {
return {
key,
value: fileSettings[key],
};
} else {
return null;
}
}

// Always check in the database first, including for secure settings,
Expand Down