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

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

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Desktop: Fixes #10382: Fix default values for settings.json plugin se…
…ttings not applied
  • Loading branch information
personalizedrefrigerator committed Apr 28, 2024
commit e682d5c7a46816faec8b6d41951776c681723c9c
15 changes: 12 additions & 3 deletions packages/lib/models/Setting.test.ts
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 @@ -121,11 +126,14 @@ describe('models/Setting', () => {
expect(Setting.value('myCustom')).toBe('123');
}));

it('should return values with correct type for custom settings', (async () => {
it.each([
SettingStorage.Database, SettingStorage.File,
])('should return values with correct type for custom settings (case %#)', (async (storage) => {
await Setting.registerSetting('myCustom', {
public: true,
value: 123,
type: Setting.TYPE_INT,
storage,
});

Setting.setValue('myCustom', 456);
Expand All @@ -138,6 +146,7 @@ describe('models/Setting', () => {
public: true,
value: 123,
type: Setting.TYPE_INT,
storage,
});

expect(typeof Setting.value('myCustom')).toBe('number');
Expand Down Expand Up @@ -328,7 +337,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
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
Loading