Skip to content

Commit

Permalink
Settings service localize default value, fix unit tests
Browse files Browse the repository at this point in the history
  • Loading branch information
tjcouch-sil committed Jun 7, 2024
1 parent 8b645fe commit fe22e67
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 4 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { testingProjectSettingsService } from '@extension-host/services/project-settings.service-host';
import { LocalizationSelectors } from '@shared/services/localization.service-model';
import { ProjectSettingValidator } from '@shared/services/project-settings.service-model';
import { slice } from 'platform-bible-utils';

jest.mock('@shared/services/network.service', () => ({
...jest.requireActual('@shared/services/network.service'),
Expand Down Expand Up @@ -30,6 +32,16 @@ jest.mock('@extension-host/data/core-project-settings-info.data', () => ({
},
},
}));
jest.mock('@shared/services/localization.service', () => ({
__esModule: true,
default: {
async getLocalizedStrings({ localizeKeys: keys }: LocalizationSelectors): Promise<{
[localizeKey: string]: string;
}> {
return Object.fromEntries(keys.map((key) => [key, slice(key, 1, -1)]));
},
},
}));

describe('isValid', () => {
it('should return true', async () => {
Expand All @@ -56,7 +68,7 @@ describe('getDefault', () => {
it('should get default value', async () => {
const projectSettingKey = 'platform.fullName';
const defaultValue = await testingProjectSettingsService.getDefault(projectSettingKey);
expect(defaultValue).toBe('%test_project_full_name_missing%');
expect(defaultValue).toBe('test_project_full_name_missing');
});

it('should throw if a default is not present', async () => {
Expand Down
24 changes: 24 additions & 0 deletions src/extension-host/services/settings.service-host.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import { testingSettingService } from '@extension-host/services/settings.service-host';
import { LocalizationSelectors } from '@shared/services/localization.service-model';
import { SettingNames } from 'papi-shared-types';
import { slice } from 'platform-bible-utils';

const MOCK_SETTINGS_DATA = {
'platform.interfaceLanguage': ['fre'],
Expand Down Expand Up @@ -32,6 +35,10 @@ jest.mock('@extension-host/data/core-settings-info.data', () => ({
label: '%platform_group1%',
description: '%platform_group1_description%',
properties: {
'platform.name': {
label: '%settings_platform_name_label%',
default: '%missing%',
},
'platform.verseRef': {
label: '%settings_platform_verseRef_label%',
default: { bookNum: 1, chapterNum: 1, verseNum: 1 },
Expand All @@ -53,6 +60,16 @@ jest.mock('@extension-host/data/core-settings-info.data', () => ({
},
},
}));
jest.mock('@shared/services/localization.service', () => ({
__esModule: true,
default: {
async getLocalizedStrings({ localizeKeys: keys }: LocalizationSelectors): Promise<{
[localizeKey: string]: string;
}> {
return Object.fromEntries(keys.map((key) => [key, slice(key, 1, -1)]));
},
},
}));

test('Get verseRef returns default value', async () => {
const result = await settingsProviderEngine.get('platform.verseRef');
Expand All @@ -64,6 +81,13 @@ test('Get interfaceLanguage returns stored value', async () => {
expect(result).toEqual(MOCK_SETTINGS_DATA['platform.interfaceLanguage']);
});

test('Get default localizeKey returns localized string', async () => {
// This is a fake setting
// eslint-disable-next-line no-type-assertion/no-type-assertion
const result = await settingsProviderEngine.get('platform.name' as SettingNames);
expect(result).toEqual('missing');
});

test('No setting exists for key', async () => {
// settingsTest.noSettingExists does not exist on SettingNames
// @ts-expect-error ts(2345)
Expand Down
35 changes: 32 additions & 3 deletions src/extension-host/services/settings.service-host.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import {
debounce,
deserialize,
includes,
isLocalizeKey,
isString,
serialize,
} from 'platform-bible-utils';
import { joinUriPaths } from '@node/utils/util';
Expand Down Expand Up @@ -59,9 +61,9 @@ async function writeSettingsDataToFile(settingsData: Partial<AllSettingsData>) {
await nodeFS.writeFile(SETTINGS_FILE_URI, JSON.stringify(settingsData));
}

function getDefaultValueForKey<SettingName extends SettingNames>(
async function getDefaultValueForKey<SettingName extends SettingNames>(
key: SettingName,
): SettingTypes[SettingName] {
): Promise<SettingTypes[SettingName]> {
const settingInfo = settingsDocumentCombiner.getSettingsContributionInfo()?.settings[key];
if (!settingInfo) {
throw new Error(`No setting exists for key ${key}`);
Expand All @@ -73,7 +75,34 @@ function getDefaultValueForKey<SettingName extends SettingNames>(
throw new Error(`No default value specified for key ${key}`);
}

return settingInfo.default;
// If this key is the interface language, return it since we need to use that in the localization
// service. Or if default is not a localized string key, return it. Otherwise we need to get the
// localized version instead
if (
key === 'platform.interfaceLanguage' ||
!isString(settingInfo.default) ||
!isLocalizeKey(settingInfo.default)
)
return settingInfo.default;

const localizedSettingInfo = (
await settingsDocumentCombiner.getLocalizedSettingsContributionInfo()
)?.settings[key];

if (!localizedSettingInfo) {
throw new Error(`Could not find localized setting ${key}.`);
}

// We shouldn't be able to hit this anymore since the settings document combiner should
// throw if this ever happened. But this is still here just in case because this would be a
// serious error
if (!('default' in localizedSettingInfo)) {
throw new Error(`Could not find localized default value for setting ${key}.`);
}

// This type is correct. Looks like `ReplaceType` breaks mapped types and just unions the types
// eslint-disable-next-line no-type-assertion/no-type-assertion
return localizedSettingInfo.default as SettingTypes[SettingName];
}

async function validateSetting<SettingName extends SettingNames>(
Expand Down

0 comments on commit fe22e67

Please sign in to comment.