Update dictionary will preserve its settings (#1553)

* Fix updating dictionaries resets their positioning * preserve settings when update dictionary * cleanup * cleanup pr * lint * Add profile id * Revert "Add profile id" This reverts commit b33bc94. * Reapply "Add profile id" This reverts commit 4d984d0. * lint * don't need to handle id collision * update for loop syntax
yomidevs · Nov 4, 2024 · d09cd38 · d09cd38
1 parent 92bac4e
commit d09cd38
Show file tree

Hide file tree

Showing 7 changed files with 73 additions and 15 deletions.
diff --git a/ext/js/data/options-util.js b/ext/js/data/options-util.js
@@ -564,6 +564,7 @@ export class OptionsUtil {
             this._updateVersion50,
             this._updateVersion51,
             this._updateVersion52,
+            this._updateVersion53,
         ];
         /* eslint-enable @typescript-eslint/unbound-method */
         if (typeof targetVersion === 'number' && targetVersion < result.length) {
@@ -1498,6 +1499,16 @@ export class OptionsUtil {
         }
     }
 
+    /**
+     * - Added profile id
+     * @type {import('options-util').UpdateFunction}
+     */
+    async _updateVersion53(options) {
+        for (let i = 0; i < options.profiles.length; i++) {
+            options.profiles[i].id = `profile-${i}`;
+        }
+    }
+
     /**
      * @param {string} url
      * @returns {Promise<chrome.tabs.Tab>}

diff --git a/ext/js/pages/settings/dictionary-controller.js b/ext/js/pages/settings/dictionary-controller.js
@@ -1139,9 +1139,24 @@ export class DictionaryController {
         downloadUrl = downloadUrl ?? dictionaryInfo.downloadUrl;
         if (typeof downloadUrl !== 'string') { throw new Error('Attempted to update dictionary without download URL'); }
 
-        await this._deleteDictionary(dictionaryTitle);
+        const options = await this._settingsController.getOptionsFull();
+        const {profiles} = options;
+
+        /** @type {import('settings-controller.js').ProfilesDictionarySettings} */
+        const profilesDictionarySettings = {};
+
+        for (const profile of profiles) {
+            const dictionaries = profile.options.dictionaries;
+            for (let i = 0; i < dictionaries.length; ++i) {
+                if (dictionaries[i].name === dictionaryTitle) {
+                    profilesDictionarySettings[profile.id] = {...dictionaries[i], index: i};
+                    break;
+                }
+            }
+        }
 
-        this._settingsController.trigger('importDictionaryFromUrl', {url: downloadUrl});
+        await this._deleteDictionary(dictionaryTitle);
+        this._settingsController.trigger('importDictionaryFromUrl', {url: downloadUrl, profilesDictionarySettings});
     }
 
     /**

diff --git a/ext/js/pages/settings/dictionary-import-controller.js b/ext/js/pages/settings/dictionary-import-controller.js
@@ -127,6 +127,7 @@ export class DictionaryImportController {
                 const onProgress = importProgressTracker.onProgress.bind(importProgressTracker);
                 await this._importDictionaries(
                     this._generateFilesFromUrls([url], onProgress),
+                    null,
                     importProgressTracker,
                 );
                 void this._recommendedDictionaryQueue.shift();
@@ -251,8 +252,8 @@ export class DictionaryImportController {
     /**
      * @param {import('settings-controller').EventArgument<'importDictionaryFromUrl'>} details
      */
-    _onEventImportDictionaryFromUrl({url}) {
-        void this.importFilesFromURLs(url);
+    _onEventImportDictionaryFromUrl({url, profilesDictionarySettings}) {
+        void this.importFilesFromURLs(url, profilesDictionarySettings);
     }
 
     /** */
@@ -311,6 +312,7 @@ export class DictionaryImportController {
         const importProgressTracker = new ImportProgressTracker(this._getFileImportSteps(), fileArray.length);
         void this._importDictionaries(
             this._arrayToAsyncGenerator(fileArray),
+            null,
             importProgressTracker,
         );
     }
@@ -416,6 +418,7 @@ export class DictionaryImportController {
         node.value = '';
         void this._importDictionaries(
             this._arrayToAsyncGenerator(files2),
+            null,
             new ImportProgressTracker(this._getFileImportSteps(), files2.length),
         );
     }
@@ -424,19 +427,21 @@ export class DictionaryImportController {
     async _onImportFromURL() {
         const text = this._importURLText.value.trim();
         if (!text) { return; }
-        await this.importFilesFromURLs(text);
+        await this.importFilesFromURLs(text, null);
     }
 
     /**
      * @param {string} text
+     * @param {import('settings-controller').ProfilesDictionarySettings} profilesDictionarySettings
      */
-    async importFilesFromURLs(text) {
+    async importFilesFromURLs(text, profilesDictionarySettings) {
         const urls = text.split('\n');
 
         const importProgressTracker = new ImportProgressTracker(this._getUrlImportSteps(), urls.length);
         const onProgress = importProgressTracker.onProgress.bind(importProgressTracker);
         void this._importDictionaries(
             this._generateFilesFromUrls(urls, onProgress),
+            profilesDictionarySettings,
             importProgressTracker,
         );
     }
@@ -518,9 +523,10 @@ export class DictionaryImportController {
 
     /**
      * @param {AsyncGenerator<File, void, void>} dictionaries
+     * @param {import('settings-controller').ProfilesDictionarySettings} profilesDictionarySettings
      * @param {ImportProgressTracker} importProgressTracker
      */
-    async _importDictionaries(dictionaries, importProgressTracker) {
+    async _importDictionaries(dictionaries, profilesDictionarySettings, importProgressTracker) {
         if (this._modifying) { return; }
 
         const statusFooter = this._statusFooter;
@@ -557,6 +563,7 @@ export class DictionaryImportController {
                     ...errors,
                     ...(await this._importDictionaryFromZip(
                         file,
+                        profilesDictionarySettings,
                         importDetails,
                         onProgress,
                     ) ?? []),
@@ -613,18 +620,19 @@ export class DictionaryImportController {
 
     /**
      * @param {File} file
+     * @param {import('settings-controller').ProfilesDictionarySettings} profilesDictionarySettings
      * @param {import('dictionary-importer').ImportDetails} importDetails
      * @param {import('dictionary-worker').ImportProgressCallback} onProgress
      * @returns {Promise<Error[] | undefined>}
      */
-    async _importDictionaryFromZip(file, importDetails, onProgress) {
+    async _importDictionaryFromZip(file, profilesDictionarySettings, importDetails, onProgress) {
         const archiveContent = await this._readFile(file);
         const {result, errors} = await new DictionaryWorker().importDictionary(archiveContent, importDetails, onProgress);
         if (!result) {
             return errors;
         }
 
-        const errors2 = await this._addDictionarySettings(result);
+        const errors2 = await this._addDictionarySettings(result, profilesDictionarySettings);
 
         await this._settingsController.application.api.triggerDatabaseUpdated('dictionary', 'import');
 
@@ -638,9 +646,10 @@ export class DictionaryImportController {
 
     /**
      * @param {import('dictionary-importer').Summary} summary
+     * @param {import('settings-controller').ProfilesDictionarySettings} profilesDictionarySettings
      * @returns {Promise<Error[]>}
      */
-    async _addDictionarySettings(summary) {
+    async _addDictionarySettings(summary, profilesDictionarySettings) {
         const {title, sequenced, styles} = summary;
         let optionsFull;
         // Workaround Firefox bug sometimes causing getOptionsFull to fail
@@ -659,11 +668,26 @@ export class DictionaryImportController {
         const targets = [];
         const profileCount = optionsFull.profiles.length;
         for (let i = 0; i < profileCount; ++i) {
-            const {options} = optionsFull.profiles[i];
+            const {options, id: profileId} = optionsFull.profiles[i];
             const enabled = profileIndex === i;
-            const value = DictionaryController.createDefaultDictionarySettings(title, enabled, styles);
+            const defaultSettings = DictionaryController.createDefaultDictionarySettings(title, enabled, styles);
             const path1 = `profiles[${i}].options.dictionaries`;
-            targets.push({action: 'push', path: path1, items: [value]});
+
+            if (profilesDictionarySettings === null || typeof profilesDictionarySettings[profileId] === 'undefined') {
+                targets.push({action: 'push', path: path1, items: [defaultSettings]});
+            } else {
+                const {index, ...currentSettings} = profilesDictionarySettings[profileId];
+                targets.push({
+                    action: 'splice',
+                    path: path1,
+                    start: index,
+                    items: [{
+                        ...currentSettings,
+                        name: title,
+                    }],
+                    deleteCount: 0,
+                });
+            }
 
             if (sequenced && options.general.mainDictionary === '') {
                 const path2 = `profiles[${i}].options.general.mainDictionary`;

diff --git a/ext/js/pages/settings/profile-controller.js b/ext/js/pages/settings/profile-controller.js
@@ -17,7 +17,7 @@
  */
 
 import {EventListenerCollection} from '../../core/event-listener-collection.js';
-import {clone} from '../../core/utilities.js';
+import {clone, generateId} from '../../core/utilities.js';
 import {querySelectorNotNull} from '../../dom/query-selector.js';
 import {ProfileConditionsUI} from './profile-conditions-ui.js';
 
@@ -184,6 +184,7 @@ export class ProfileController {
         // Create new profile
         const newProfile = clone(profile);
         newProfile.name = this._createCopyName(profile.name, this._profiles, 100);
+        newProfile.id = generateId(16);
 
         // Update state
         const index = this._profiles.length;

diff --git a/test/options-util.test.js b/test/options-util.test.js
@@ -565,6 +565,7 @@ function createOptionsUpdatedTestData1() {
     return {
         profiles: [
             {
+                id: 'profile-0',
                 name: 'Default',
                 options: createProfileOptionsUpdatedTestData1(),
                 conditionGroups: [
@@ -644,7 +645,7 @@ function createOptionsUpdatedTestData1() {
             },
         ],
         profileCurrent: 0,
-        version: 52,
+        version: 53,
         global: {
             database: {
                 prefixWildcardsSupported: false,

diff --git a/types/ext/settings-controller.d.ts b/types/ext/settings-controller.d.ts
@@ -28,6 +28,10 @@ export type PageExitPrevention = {
     end: () => void;
 };
 
+type ProfileDictionarySettings = Settings.DictionaryOptions & {index: number};
+
+export type ProfilesDictionarySettings = {[profileId: string]: ProfileDictionarySettings} | null;
+
 export type Events = {
     optionsChanged: {
         options: Settings.ProfileOptions;
@@ -42,6 +46,7 @@ export type Events = {
     };
     importDictionaryFromUrl: {
         url: string;
+        profilesDictionarySettings: ProfilesDictionarySettings;
     };
     dictionaryEnabled: Record<string, never>;
     scanInputsChanged: {

diff --git a/types/ext/settings.d.ts b/types/ext/settings.d.ts
@@ -70,6 +70,7 @@ export type GlobalDatabaseOptions = {
 };
 
 export type Profile = {
+    id: string;
     name: string;
     conditionGroups: ProfileConditionGroup[];
     options: ProfileOptions;