Merge pull request #2668 from wordofglass/edit_logic

Edit plugin logic: Regression from previous PR; incorrect diffs

beets/dbcore/db.py

@@ -217,6 +217,21 @@ class Model(object):
         if need_id and not self.id:
             raise ValueError(u'{0} has no id'.format(type(self).__name__))
 
+    def copy(self):
+        """Create a copy of the model object.
+
+        The field values and other state is duplicated, but the new copy
+        remains associated with the same database as the old object.
+        (A simple `copy.deepcopy` will not work because it would try to
+        duplicate the SQLite connection.)
+        """
+        new = self.__class__()
+        new._db = self._db
+        new._values_fixed = self._values_fixed.copy()
+        new._values_flex = self._values_flex.copy()
+        new._dirty = self._dirty.copy()
+        return new
+
     # Essential field accessors.
 
     @classmethod

beetsplug/edit.py

@@ -23,7 +23,6 @@ from beets import ui
 from beets.dbcore import types
 from beets.importer import action
 from beets.ui.commands import _do_query, PromptChoice
-from copy import deepcopy
 import codecs
 import subprocess
 import yaml
@@ -283,7 +282,7 @@ class EditPlugin(plugins.BeetsPlugin):
                 # Show the changes.
                 # If the objects are not on the DB yet, we need a copy of their
                 # original state for show_model_changes.
-                objs_old = [deepcopy(obj) if not obj._db else None
+                objs_old = [obj.copy() if obj.id < 0 else None
                             for obj in objs]
                 self.apply_data(objs, old_data, new_data)
                 changed = False
@@ -302,9 +301,13 @@ class EditPlugin(plugins.BeetsPlugin):
                 elif choice == u'c':  # Cancel.
                     return False
                 elif choice == u'e':  # Keep editing.
-                    # Reset the temporary changes to the objects.
+                    # Reset the temporary changes to the objects. I we have a
+                    # copy from above, use that, else reload from the database.
+                    objs = [(old_obj or obj)
+                            for old_obj, obj in zip(objs_old, objs)]
                     for obj in objs:
-                        obj.read()
+                        if not obj.id < 0:
+                            obj.load()
                     continue
 
         # Remove the temporary file before returning.
@@ -369,7 +372,8 @@ class EditPlugin(plugins.BeetsPlugin):
         # yet. By using negative values, no clash with items in the database
         # can occur.
         for i, obj in enumerate(task.items, start=1):
-            if not obj._db:
+            # The importer may set the id to None when re-importing albums.
+            if not obj._db or obj.id is None:
                 obj.id = -i
 
         # Present the YAML to the user and let her change it.
@@ -378,7 +382,7 @@ class EditPlugin(plugins.BeetsPlugin):
 
         # Remove temporary ids.
         for obj in task.items:
-            if not obj._db:
+            if obj.id < 0:
                 obj.id = None
 
         # Save the new data.

test/test_edit.py

@@ -22,6 +22,7 @@ from test import _common
 from test.helper import TestHelper, control_stdin
 from test.test_ui_importer import TerminalImportSessionSetup
 from test.test_importer import ImportHelper, AutotagStub
+from beets.dbcore.query import TrueQuery
 from beets.library import Item
 from beetsplug.edit import EditPlugin
 
@@ -363,6 +364,34 @@ class EditDuringImporterTest(TerminalImportSessionSetup, unittest.TestCase,
         # Ensure album is fetched from a candidate.
         self.assertIn('albumid', self.lib.albums()[0].mb_albumid)
 
+    def test_edit_retag_apply(self):
+        """Import the album using a candidate, then retag and edit and apply
+        changes.
+        """
+        self._setup_import_session()
+        self.run_mocked_interpreter({},
+                                    # 1, Apply changes.
+                                    ['1', 'a'])
+
+        # Retag and edit track titles.  On retag, the importer will reset items
+        # ids but not the db connections.
+        self.importer.paths = []
+        self.importer.query = TrueQuery()
+        self.run_mocked_interpreter({'replacements': {u'Applied Title':
+                                                      u'Edited Title'}},
+                                    # eDit, Apply changes.
+                                    ['d', 'a'])
+
+        # Check that 'title' field is modified, and other fields come from
+        # the candidate.
+        self.assertTrue(all('Edited Title ' in i.title
+                            for i in self.lib.items()))
+        self.assertTrue(all('match ' in i.mb_trackid
+                            for i in self.lib.items()))
+
+        # Ensure album is fetched from a candidate.
+        self.assertIn('albumid', self.lib.albums()[0].mb_albumid)
+
     def test_edit_discard_candidate(self):
         """Edit the album field for all items in the library, discard changes,
         using a candidate.