Post Processing config saving select-list values incorrectly (#5165)

* Fix config-post-processing * Add failing tests for select-list * Fix Lint * Update changelog * Move fix to backend
pymedusa · Sep 9, 2018 · 680c199 · 680c199
1 parent 684bec1
commit 680c199
Show file tree

Hide file tree

Showing 11 changed files with 114 additions and 30 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,7 +13,7 @@
   - Home page (when using "split home in tabs") ([#5126](https://github.com/pymedusa/Medusa/pull/5126))
   - Status page ([#5127](https://github.com/pymedusa/Medusa/pull/5127))
   - Preview Rename page ([#5169](https://github.com/pymedusa/Medusa/pull/5169))
-
+  - Post Processing Config page - saving `select-list` values incorrectly ([#5165](https://github.com/pymedusa/Medusa/pull/5165))
 
 -----
 

diff --git a/medusa/__main__.py b/medusa/__main__.py
@@ -95,6 +95,34 @@
 logger = logging.getLogger(__name__)
 
 
+def fix_incorrect_list_values(data):
+    """
+    @TODO: Remove this in a future version.
+
+    Due to a bug introduced in v0.2.9, the value might be a string representing a Python dict.
+    See: https://github.com/pymedusa/Medusa/issues/5155
+
+    Example: `"{u'id': 0, u'value': u'!sync'}"` to `"!sync"`
+    """
+    import ast
+
+    result = []
+    for item in data:
+        if not item:
+            continue
+        if not (item.startswith('{') and item.endswith('}')):
+            # Simple value, don't do anything to it
+            result.append(item)
+            continue
+        try:
+            # Get the value: `{u'id': 0, u'value': u'!sync'}` => `!sync`
+            result.append(ast.literal_eval(item)['value'])
+        except (SyntaxError, KeyError):
+            pass
+
+    return result
+
+
 class Application(object):
     """Main application module."""
 
@@ -604,7 +632,11 @@ def initialize(self, console_logging=True):
             app.RANDOMIZE_PROVIDERS = bool(check_setting_int(app.CFG, 'General', 'randomize_providers', 0))
             app.ALLOW_HIGH_PRIORITY = bool(check_setting_int(app.CFG, 'General', 'allow_high_priority', 1))
             app.SKIP_REMOVED_FILES = bool(check_setting_int(app.CFG, 'General', 'skip_removed_files', 0))
+
             app.ALLOWED_EXTENSIONS = check_setting_list(app.CFG, 'General', 'allowed_extensions', app.ALLOWED_EXTENSIONS)
+            # @TODO: Remove this in a future version.
+            app.ALLOWED_EXTENSIONS = fix_incorrect_list_values(app.ALLOWED_EXTENSIONS)
+
             app.USENET_RETENTION = check_setting_int(app.CFG, 'General', 'usenet_retention', 500)
             app.CACHE_TRIMMING = bool(check_setting_int(app.CFG, 'General', 'cache_trimming', 0))
             app.MAX_CACHE_AGE = check_setting_int(app.CFG, 'General', 'max_cache_age', 30)
@@ -646,7 +678,11 @@ def initialize(self, console_logging=True):
             app.MOVE_ASSOCIATED_FILES = bool(check_setting_int(app.CFG, 'General', 'move_associated_files', 0))
             app.POSTPONE_IF_SYNC_FILES = bool(check_setting_int(app.CFG, 'General', 'postpone_if_sync_files', 1))
             app.POSTPONE_IF_NO_SUBS = bool(check_setting_int(app.CFG, 'General', 'postpone_if_no_subs', 0))
+
             app.SYNC_FILES = check_setting_list(app.CFG, 'General', 'sync_files', app.SYNC_FILES)
+            # @TODO: Remove this in a future version.
+            app.SYNC_FILES = fix_incorrect_list_values(app.SYNC_FILES)
+
             app.NFO_RENAME = bool(check_setting_int(app.CFG, 'General', 'nfo_rename', 1))
             app.CREATE_MISSING_SHOW_DIRS = bool(check_setting_int(app.CFG, 'General', 'create_missing_show_dirs', 0))
             app.ADD_SHOWS_WO_DIR = bool(check_setting_int(app.CFG, 'General', 'add_shows_wo_dir', 0))
@@ -919,6 +955,8 @@ def initialize(self, console_logging=True):
             app.NO_RESTART = bool(check_setting_int(app.CFG, 'General', 'no_restart', 0))
 
             app.EXTRA_SCRIPTS = [x.strip() for x in check_setting_list(app.CFG, 'General', 'extra_scripts')]
+            # @TODO: Remove this in a future version.
+            app.EXTRA_SCRIPTS = fix_incorrect_list_values(app.EXTRA_SCRIPTS)
 
             app.USE_LISTVIEW = bool(check_setting_int(app.CFG, 'General', 'use_listview', 0))
 

diff --git a/themes-default/slim/src/components/config-post-processing.vue b/themes-default/slim/src/components/config-post-processing.vue
@@ -91,7 +91,7 @@
                                             <span>Sync File Extensions</span>
                                         </label>
                                         <div class="col-sm-10 content">
-                                            <select-list name="sync_files" id="sync_files" csv-enabled :list-items="postProcessing.syncFiles" @change="postProcessing.syncFiles = $event"></select-list>
+                                            <select-list name="sync_files" id="sync_files" csv-enabled :list-items="postProcessing.syncFiles" @change="onChangeSyncFiles"></select-list>
                                             <span>comma seperated list of extensions or filename globs Medusa ignores when Post Processing</span>
                                         </div>
                                     </div>
@@ -154,7 +154,7 @@
                                             <span>Keep associated file extensions</span>
                                         </label>
                                         <div class="col-sm-10 content">
-                                            <select-list name="allowed_extensions" id="allowed_extensions" csv-enabled :list-items="postProcessing.allowedExtensions" @change="postProcessing.allowedExtensions = $event"></select-list>
+                                            <select-list name="allowed_extensions" id="allowed_extensions" csv-enabled :list-items="postProcessing.allowedExtensions" @change="onChangeAllowedExtensions"></select-list>
                                             <span>Comma seperated list of associated file extensions Medusa should keep while post processing.</span><br>
                                             <span>Leaving it empty means all associated files will be deleted</span>
                                         </div>
@@ -229,7 +229,7 @@
                                             <span>Extra Scripts</span>
                                         </label>
                                         <div class="col-sm-10 content">
-                                            <select-list name="extra_scripts" id="extra_scripts" csv-enabled :list-items="postProcessing.extraScripts" @change="postProcessing.extraScripts = $event"></select-list>
+                                            <select-list name="extra_scripts" id="extra_scripts" csv-enabled :list-items="postProcessing.extraScripts" @change="onChangeExtraScripts"></select-list>
                                             <span>See <app-link :href="postProcessing.extraScriptsUrl" class="wikie"><strong>Wiki</strong></app-link> for script arguments description and usage.</span>
                                         </div>
                                     </div>
@@ -433,6 +433,15 @@ export default {
         };
     },
     methods: {
+        onChangeSyncFiles(items) {
+            this.postProcessing.syncFiles = items.map(item => item.value);
+        },
+        onChangeAllowedExtensions(items) {
+            this.postProcessing.allowedExtensions = items.map(item => item.value);
+        },
+        onChangeExtraScripts(items) {
+            this.postProcessing.extraScripts = items.map(item => item.value);
+        },
         saveNaming(values) {
             if (!this.configLoaded) {
                 return;

diff --git a/themes-default/slim/src/components/select-list.vue b/themes-default/slim/src/components/select-list.vue
@@ -55,7 +55,6 @@ export default {
     },
     data() {
         return {
-            lock: false,
             editItems: [],
             newItem: '',
             indexCounter: 0,
@@ -64,20 +63,24 @@ export default {
         };
     },
     created() {
+        /*
+        These are needed in order to test the component,
+        but they break the component in the application:
+
+        this.editItems = this.sanitize(this.listItems);
+        this.csv = this.editItems.map(item => item.value).join(', ');
+        */
+
         /**
          * ListItems property might receive values originating from the API,
-         * that are sometimes not avaiable when rendering.
-         * @TODO: Maybe we can remove this in the future.
+         * that are sometimes not available when rendering.
+         * @TODO: This is not ideal! Maybe we can remove this in the future.
          */
         const unwatchProp = this.$watch('listItems', () => {
             unwatchProp();
 
-            this.lock = true;
             this.editItems = this.sanitize(this.listItems);
-            this.$nextTick(() => {
-                this.lock = false;
-            });
-            this.csv = this.editItems.map(x => x.value).join(', ');
+            this.csv = this.editItems.map(item => item.value).join(', ');
         });
     },
     methods: {
@@ -138,7 +141,7 @@ export default {
                     }
                 }));
             } else {
-                this.csv = this.editItems.map(x => x.value).join(', ');
+                this.csv = this.editItems.map(item => item.value).join(', ');
             }
         },
         /**
@@ -154,9 +157,7 @@ export default {
     watch: {
         editItems: {
             handler() {
-                if (!this.lock) {
-                    this.$emit('change', this.editItems);
-                }
+                this.$emit('change', this.editItems);
             },
             deep: true
         },

diff --git a/themes-default/slim/test/specs/select-list.spec.js b/themes-default/slim/test/specs/select-list.spec.js
@@ -20,10 +20,40 @@ test('renders', t => {
     const { localVue, store } = t.context;
     const wrapper = mount(SelectList, {
         localVue,
+        store,
         propsData: {
             listItems: []
-        },
-        store
+        }
+    });
+
+    t.snapshot(wrapper.html());
+});
+
+test.failing('renders with values', t => {
+    const { localVue, store } = t.context;
+
+    const listItems = [
+        'abc',
+        'bcd',
+        'test'
+    ];
+
+    const wrapper = mount(SelectList, {
+        localVue,
+        store,
+        propsData: {
+            listItems
+        }
+    });
+
+    const expectedItems = listItems;
+    const inputWrapperArray = wrapper.findAll('li input[type="text"]');
+
+    t.is(inputWrapperArray.length, expectedItems.length);
+
+    inputWrapperArray.wrappers.forEach((inputWrapper, index) => {
+        const { element } = inputWrapper;
+        t.is(element.value, expectedItems[index]);
     });
 
     t.snapshot(wrapper.html());

diff --git a/themes-default/slim/test/specs/snapshots/select-list.spec.js.md b/themes-default/slim/test/specs/snapshots/select-list.spec.js.md
@@ -9,3 +9,9 @@ Generated by [AVA](https://ava.li).
 > Snapshot 1
 
     '<div class="select-list max-width"><i title="Switch between a list and comma separated values" class="switch-input glyphicon glyphicon-refresh"></i><ul><div class="new-item"><div class="input-group"><input type="text" placeholder="add new values per line" class="form-control input-sm"><div class="input-group-btn"><div class="btn btn-default input-sm" style="font-size: 14px;"><i title="Add" class="glyphicon glyphicon-plus"></i></div></div></div></div><!----></ul></div>'
+
+## renders with values
+
+> Snapshot 1
+
+    '<div class="select-list max-width"><i title="Switch between a list and comma separated values" class="switch-input glyphicon glyphicon-refresh"></i><ul><li><div class="input-group"><input type="text" class="form-control input-sm"><div class="input-group-btn"><div class="btn btn-default input-sm" style="font-size: 14px;"><i title="Remove" class="glyphicon glyphicon-remove"></i></div></div></div></li><li><div class="input-group"><input type="text" class="form-control input-sm"><div class="input-group-btn"><div class="btn btn-default input-sm" style="font-size: 14px;"><i title="Remove" class="glyphicon glyphicon-remove"></i></div></div></div></li><li><div class="input-group"><input type="text" class="form-control input-sm"><div class="input-group-btn"><div class="btn btn-default input-sm" style="font-size: 14px;"><i title="Remove" class="glyphicon glyphicon-remove"></i></div></div></div></li><div class="new-item"><div class="input-group"><input type="text" placeholder="add new values per line" class="form-control input-sm"><div class="input-group-btn"><div class="btn btn-default input-sm" style="font-size: 14px;"><i title="Add" class="glyphicon glyphicon-plus"></i></div></div></div></div><!----></ul></div>'
diff --git a/themes-default/slim/test/specs/snapshots/select-list.spec.js.snap b/themes-default/slim/test/specs/snapshots/select-list.spec.js.snap
diff --git a/themes/dark/assets/js/vendors.js b/themes/dark/assets/js/vendors.js
diff --git a/themes/dark/assets/js/vendors.js.map b/themes/dark/assets/js/vendors.js.map
diff --git a/themes/light/assets/js/vendors.js b/themes/light/assets/js/vendors.js
diff --git a/themes/light/assets/js/vendors.js.map b/themes/light/assets/js/vendors.js.map