Fix digest second-scan bug; Break down into smaller functions.

There was a bug where moving an album out of its determined parent
caused future digests to fail because I wanted to use the GroupExists
for control flow, but due to the @transaction decorator it was rolling
back the changes.
Moved some of the logical pieces of the function into subfunctions
as a visual and readability aid.

README.md

@@ -9,7 +9,7 @@ Etiquette is a tag-based file organization system with a web front-end.
 
 Documentation is still a work in progress. In general,
 
-- You must make the `etiquette` package importable by placing it in one of your lib paths. I use filesystem junctions for this purpose.
+- You must make the `etiquette` package importable by placing it in one of your lib paths because I have not made a setup.py yet. Rather than actually moving the folder I just use filesystem junctions.
 - Run `python etiquette_flask_launch.py [port]` to launch the flask server. Port defaults to 5000 if not provided.
 - Run `python -i etiquette_repl.py` to launch the Python interpreter with the PhotoDB pre-loaded into a variable called `P`. Try things like `P.new_photo` or `P.digest_directory`.
 
@@ -81,6 +81,7 @@ If you are interested in helping, please raise an issue before making any pull r
 - Perhaps instead of actually deleting objects, they should just have a `deleted` flag, to make easy restoration possible. Also consider regrouping the children of restored Groupables if those children haven't already been reassigned somewhere else.
 - Add a new table to store permanent history of add/remove of tags on photos, so that accidents or trolling can be reversed.
 - Currently, the photo clipboard only stores IDs and therefore when we construct the clipboard tray elements we cannot provide more rich information like filename, the user is only presented with a list of IDs which they probably don't care about. Should the localstorage cache some other more user-friendly information?
+- Improve transaction rollbacking. I'm not satisfied with the @transaction decorator because sometimes I want to use exceptions as control flow without them rolling things back. Context managers are good but it's a matter of how abstracted they should be.
 
 ### To do list: User permissions
 Here are some thoughts about the kinds of features that need to exist within the permission system. I don't know how I'll actually manage it just yet. Possibly a `permissions` table in the database with `user_id | permission` where `permission` is some reliably-formatted string.

etiquette/photodb.py

@@ -1232,9 +1232,41 @@ class PhotoDB(PDBAlbumMixin, PDBBookmarkMixin, PDBPhotoMixin, PDBTagMixin, PDBUs
         If a Photo object already exists for a file, it will be added to the
         correct album.
         '''
+        def create_or_fetch_photos(files):
+            photos = []
+            for filepath in files:
+                try:
+                    photo = self.get_photo_by_path(filepath)
+                except exceptions.NoSuchPhoto:
+                    photo = self.new_photo(filepath.absolute_path, commit=False, **new_photo_kwargs)
+
+                photos.append(photo)
+            return photos
+
+        def create_or_fetch_current_album(albums_by_path, current_directory):
+            current_album = albums_by_path.get(current_directory.absolute_path, None)
+            if current_album is None:
+                try:
+                    current_album = self.get_album_by_path(current_directory.absolute_path)
+                except exceptions.NoSuchAlbum:
+                    current_album = self.new_album(
+                        associated_directory=current_directory.absolute_path,
+                        commit=False,
+                        title=current_directory.basename,
+                    )
+                albums_by_path[current_directory.absolute_path] = current_album
+            return current_album
+
+        def orphan_join_parent_album(albums_by_path, current_album, current_directory):
+            if current_album.parent() is None:
+                parent = albums_by_path.get(current_directory.parent.absolute_path, None)
+                if parent is not None:
+                    parent.add_child(current_album, commit=False)
+
         directory = pathclass.Path(directory)
         if not directory.is_dir:
             raise ValueError('Not a directory: %s' % directory)
+        directory.correct_case()
 
         if exclude_directories is None:
             exclude_directories = self.config['digest_exclude_dirs']
@@ -1246,15 +1278,6 @@ class PhotoDB(PDBAlbumMixin, PDBBookmarkMixin, PDBPhotoMixin, PDBTagMixin, PDBUs
         if 'filepath' in new_photo_kwargs:
             new_photo_kwargs.pop('filepath')
 
-        directory.correct_case()
-        generator = spinal.walk_generator(
-            directory,
-            exclude_directories=exclude_directories,
-            exclude_filenames=exclude_filenames,
-            recurse=recurse,
-            yield_style='nested',
-        )
-
         if make_albums:
             try:
                 album = self.get_album_by_path(directory.absolute_path)
@@ -1264,47 +1287,26 @@ class PhotoDB(PDBAlbumMixin, PDBBookmarkMixin, PDBPhotoMixin, PDBTagMixin, PDBUs
                     commit=False,
                     title=directory.basename,
                 )
-            albums = {directory.absolute_path: album}
+            albums_by_path = {directory.absolute_path: album}
 
-        for (current_location, directories, files) in generator:
+        walk_generator = spinal.walk_generator(
-            # Create the photo object
+            directory,
-            new_photos = []
+            exclude_directories=exclude_directories,
-            for filepath in files:
+            exclude_filenames=exclude_filenames,
-                try:
+            recurse=recurse,
-                    photo = self.get_photo_by_path(filepath)
+            yield_style='nested',
-                except exceptions.NoSuchPhoto:
+        )
-                    makenew = True
-                else:
-                    makenew = False
 
-                if makenew:
+        for (current_directory, subdirectories, files) in walk_generator:
-                    photo = self.new_photo(filepath.absolute_path, commit=False, **new_photo_kwargs)
+            photos = create_or_fetch_photos(files)
-
-                new_photos.append(photo)
 
             if not make_albums:
                 continue
 
-            # Ensure that the current directory is an album.
+            current_album = create_or_fetch_current_album(albums_by_path, current_directory)
-            current_album = albums.get(current_location.absolute_path, None)
+            orphan_join_parent_album(albums_by_path, current_album, current_directory)
-            if current_album is None:
-                try:
-                    current_album = self.get_album_by_path(current_location.absolute_path)
-                except exceptions.NoSuchAlbum:
-                    current_album = self.new_album(
-                        associated_directory=current_location.absolute_path,
-                        commit=False,
-                        title=current_location.basename,
-                    )
-                albums[current_location.absolute_path] = current_album
 
-            parent = albums.get(current_location.parent.absolute_path, None)
+            for photo in photos:
-            if parent is not None:
-                try:
-                    parent.add_child(current_album, commit=False)
-                except exceptions.GroupExists:
-                    pass
-            for photo in new_photos:
                 current_album.add_photo(photo, commit=False)
 
         if commit: