Fix:Remove authors with no books when a books is removed #3668

- Handles bulk delete, single delete, deleting library folder, and removing items with issues
- Also handles bulk editing and removing authors

server/controllers/LibraryController.js

@@ -400,19 +400,48 @@ class LibraryController {
                   model: Database.podcastEpisodeModel,
                   attributes: ['id']
                 }
+              },
+              {
+                model: Database.bookModel,
+                attributes: ['id'],
+                include: [
+                  {
+                    model: Database.bookAuthorModel,
+                    attributes: ['authorId']
+                  },
+                  {
+                    model: Database.bookSeriesModel,
+                    attributes: ['seriesId']
+                  }
+                ]
               }
             ]
           })
           Logger.info(`[LibraryController] Removed folder "${folder.path}" from library "${req.library.name}" with ${libraryItemsInFolder.length} library items`)
+          const seriesIds = []
+          const authorIds = []
           for (const libraryItem of libraryItemsInFolder) {
             let mediaItemIds = []
             if (req.library.isPodcast) {
               mediaItemIds = libraryItem.media.podcastEpisodes.map((pe) => pe.id)
             } else {
               mediaItemIds.push(libraryItem.mediaId)
+              if (libraryItem.media.bookAuthors.length) {
+                authorIds.push(...libraryItem.media.bookAuthors.map((ba) => ba.authorId))
+              }
+              if (libraryItem.media.bookSeries.length) {
+                seriesIds.push(...libraryItem.media.bookSeries.map((bs) => bs.seriesId))
+              }
             }
             Logger.info(`[LibraryController] Removing library item "${libraryItem.id}" from folder "${folder.path}"`)
-            await this.handleDeleteLibraryItem(libraryItem.mediaType, libraryItem.id, mediaItemIds)
+            await this.handleDeleteLibraryItem(libraryItem.id, mediaItemIds)
+          }
+
+          if (authorIds.length) {
+            await this.checkRemoveAuthorsWithNoBooks(authorIds)
+          }
+          if (seriesIds.length) {
+            await this.checkRemoveEmptySeries(seriesIds)
           }
 
           // Remove folder
@@ -501,7 +530,7 @@ class LibraryController {
         mediaItemIds.push(libraryItem.mediaId)
       }
       Logger.info(`[LibraryController] Removing library item "${libraryItem.id}" from library "${req.library.name}"`)
-      await this.handleDeleteLibraryItem(libraryItem.mediaType, libraryItem.id, mediaItemIds)
+      await this.handleDeleteLibraryItem(libraryItem.id, mediaItemIds)
     }
 
     // Set PlaybackSessions libraryId to null
@@ -580,6 +609,8 @@ class LibraryController {
    * DELETE: /api/libraries/:id/issues
    * Remove all library items missing or invalid
    *
+   * @this {import('../routers/ApiRouter')}
+   *
    * @param {LibraryControllerRequest} req
    * @param {Response} res
    */
@@ -605,6 +636,20 @@ class LibraryController {
             model: Database.podcastEpisodeModel,
             attributes: ['id']
           }
+        },
+        {
+          model: Database.bookModel,
+          attributes: ['id'],
+          include: [
+            {
+              model: Database.bookAuthorModel,
+              attributes: ['authorId']
+            },
+            {
+              model: Database.bookSeriesModel,
+              attributes: ['seriesId']
+            }
+          ]
         }
       ]
     })
@@ -615,15 +660,30 @@ class LibraryController {
     }
 
     Logger.info(`[LibraryController] Removing ${libraryItemsWithIssues.length} items with issues`)
+    const authorIds = []
+    const seriesIds = []
     for (const libraryItem of libraryItemsWithIssues) {
       let mediaItemIds = []
       if (req.library.isPodcast) {
         mediaItemIds = libraryItem.media.podcastEpisodes.map((pe) => pe.id)
       } else {
         mediaItemIds.push(libraryItem.mediaId)
+        if (libraryItem.media.bookAuthors.length) {
+          authorIds.push(...libraryItem.media.bookAuthors.map((ba) => ba.authorId))
+        }
+        if (libraryItem.media.bookSeries.length) {
+          seriesIds.push(...libraryItem.media.bookSeries.map((bs) => bs.seriesId))
+        }
       }
       Logger.info(`[LibraryController] Removing library item "${libraryItem.id}" with issue`)
-      await this.handleDeleteLibraryItem(libraryItem.mediaType, libraryItem.id, mediaItemIds)
+      await this.handleDeleteLibraryItem(libraryItem.id, mediaItemIds)
+    }
+
+    if (authorIds.length) {
+      await this.checkRemoveAuthorsWithNoBooks(authorIds)
+    }
+    if (seriesIds.length) {
+      await this.checkRemoveEmptySeries(seriesIds)
     }
 
     // Set numIssues to 0 for library filter data

server/controllers/LibraryItemController.js

@@ -96,6 +96,8 @@ class LibraryItemController {
    * Optional query params:
    * ?hard=1
    *
+   * @this {import('../routers/ApiRouter')}
+   *
    * @param {RequestWithUser} req
    * @param {Response} res
    */
@@ -103,14 +105,36 @@ class LibraryItemController {
     const hardDelete = req.query.hard == 1 // Delete from file system
     const libraryItemPath = req.libraryItem.path
 
-    const mediaItemIds = req.libraryItem.mediaType === 'podcast' ? req.libraryItem.media.episodes.map((ep) => ep.id) : [req.libraryItem.media.id]
+    const mediaItemIds = []
-    await this.handleDeleteLibraryItem(req.libraryItem.mediaType, req.libraryItem.id, mediaItemIds)
+    const authorIds = []
+    const seriesIds = []
+    if (req.libraryItem.isPodcast) {
+      mediaItemIds.push(...req.libraryItem.media.episodes.map((ep) => ep.id))
+    } else {
+      mediaItemIds.push(req.libraryItem.media.id)
+      if (req.libraryItem.media.metadata.authors?.length) {
+        authorIds.push(...req.libraryItem.media.metadata.authors.map((au) => au.id))
+      }
+      if (req.libraryItem.media.metadata.series?.length) {
+        seriesIds.push(...req.libraryItem.media.metadata.series.map((se) => se.id))
+      }
+    }
+
+    await this.handleDeleteLibraryItem(req.libraryItem.id, mediaItemIds)
     if (hardDelete) {
       Logger.info(`[LibraryItemController] Deleting library item from file system at "${libraryItemPath}"`)
       await fs.remove(libraryItemPath).catch((error) => {
         Logger.error(`[LibraryItemController] Failed to delete library item from file system at "${libraryItemPath}"`, error)
       })
     }
+
+    if (authorIds.length) {
+      await this.checkRemoveAuthorsWithNoBooks(authorIds)
+    }
+    if (seriesIds.length) {
+      await this.checkRemoveEmptySeries(seriesIds)
+    }
+
     await Database.resetLibraryIssuesFilterData(req.libraryItem.libraryId)
     res.sendStatus(200)
   }
@@ -212,15 +236,6 @@ class LibraryItemController {
     if (hasUpdates) {
       libraryItem.updatedAt = Date.now()
 
-      if (seriesRemoved.length) {
-        // Check remove empty series
-        Logger.debug(`[LibraryItemController] Series was removed from book. Check if series is now empty.`)
-        await this.checkRemoveEmptySeries(
-          libraryItem.media.id,
-          seriesRemoved.map((se) => se.id)
-        )
-      }
-
       if (isPodcastAutoDownloadUpdated) {
         this.cronManager.checkUpdatePodcastCron(libraryItem)
       }
@@ -232,10 +247,12 @@ class LibraryItemController {
       if (authorsRemoved.length) {
         // Check remove empty authors
         Logger.debug(`[LibraryItemController] Authors were removed from book. Check if authors are now empty.`)
-        await this.checkRemoveAuthorsWithNoBooks(
+        await this.checkRemoveAuthorsWithNoBooks(authorsRemoved.map((au) => au.id))
-          libraryItem.libraryId,
+      }
-          authorsRemoved.map((au) => au.id)
+      if (seriesRemoved.length) {
-        )
+        // Check remove empty series
+        Logger.debug(`[LibraryItemController] Series were removed from book. Check if series are now empty.`)
+        await this.checkRemoveEmptySeries(seriesRemoved.map((se) => se.id))
       }
     }
     res.json({
@@ -450,6 +467,8 @@ class LibraryItemController {
    * Optional query params:
    * ?hard=1
    *
+   * @this {import('../routers/ApiRouter')}
+   *
    * @param {RequestWithUser} req
    * @param {Response} res
    */
@@ -477,14 +496,33 @@ class LibraryItemController {
     for (const libraryItem of itemsToDelete) {
       const libraryItemPath = libraryItem.path
       Logger.info(`[LibraryItemController] (${hardDelete ? 'Hard' : 'Soft'}) deleting Library Item "${libraryItem.media.metadata.title}" with id "${libraryItem.id}"`)
-      const mediaItemIds = libraryItem.mediaType === 'podcast' ? libraryItem.media.episodes.map((ep) => ep.id) : [libraryItem.media.id]
+      const mediaItemIds = []
-      await this.handleDeleteLibraryItem(libraryItem.mediaType, libraryItem.id, mediaItemIds)
+      const seriesIds = []
+      const authorIds = []
+      if (libraryItem.isPodcast) {
+        mediaItemIds.push(...libraryItem.media.episodes.map((ep) => ep.id))
+      } else {
+        mediaItemIds.push(libraryItem.media.id)
+        if (libraryItem.media.metadata.series?.length) {
+          seriesIds.push(...libraryItem.media.metadata.series.map((se) => se.id))
+        }
+        if (libraryItem.media.metadata.authors?.length) {
+          authorIds.push(...libraryItem.media.metadata.authors.map((au) => au.id))
+        }
+      }
+      await this.handleDeleteLibraryItem(libraryItem.id, mediaItemIds)
       if (hardDelete) {
         Logger.info(`[LibraryItemController] Deleting library item from file system at "${libraryItemPath}"`)
         await fs.remove(libraryItemPath).catch((error) => {
           Logger.error(`[LibraryItemController] Failed to delete library item from file system at "${libraryItemPath}"`, error)
         })
       }
+      if (seriesIds.length) {
+        await this.checkRemoveEmptySeries(seriesIds)
+      }
+      if (authorIds.length) {
+        await this.checkRemoveAuthorsWithNoBooks(authorIds)
+      }
     }
 
     await Database.resetLibraryIssuesFilterData(libraryId)
@@ -494,6 +532,8 @@ class LibraryItemController {
   /**
    * POST: /api/items/batch/update
    *
+   * @this {import('../routers/ApiRouter')}
+   *
    * @param {RequestWithUser} req
    * @param {Response} res
    */
@@ -503,39 +543,62 @@ class LibraryItemController {
       return res.sendStatus(500)
     }
 
+    // Ensure that each update payload has a unique library item id
+    const libraryItemIds = [...new Set(updatePayloads.map((up) => up.id))]
+    if (!libraryItemIds.length || libraryItemIds.length !== updatePayloads.length) {
+      Logger.error(`[LibraryItemController] Batch update failed. Each update payload must have a unique library item id`)
+      return res.sendStatus(400)
+    }
+
+    // Get all library items to update
+    const libraryItems = await Database.libraryItemModel.getAllOldLibraryItems({
+      id: libraryItemIds
+    })
+    if (updatePayloads.length !== libraryItems.length) {
+      Logger.error(`[LibraryItemController] Batch update failed. Not all library items found`)
+      return res.sendStatus(404)
+    }
+
     let itemsUpdated = 0
 
+    const seriesIdsRemoved = []
+    const authorIdsRemoved = []
+
     for (const updatePayload of updatePayloads) {
       const mediaPayload = updatePayload.mediaPayload
-      const libraryItem = await Database.libraryItemModel.getOldById(updatePayload.id)
+      const libraryItem = libraryItems.find((li) => li.id === updatePayload.id)
-      if (!libraryItem) return null
 
       await this.createAuthorsAndSeriesForItemUpdate(mediaPayload, libraryItem.libraryId)
 
-      let seriesRemoved = []
+      if (libraryItem.isBook) {
-      if (libraryItem.isBook && mediaPayload.metadata?.series) {
+        if (Array.isArray(mediaPayload.metadata?.series)) {
-        const seriesIdsInUpdate = (mediaPayload.metadata?.series || []).map((se) => se.id)
+          const seriesIdsInUpdate = mediaPayload.metadata.series.map((se) => se.id)
-        seriesRemoved = libraryItem.media.metadata.series.filter((se) => !seriesIdsInUpdate.includes(se.id))
+          const seriesRemoved = libraryItem.media.metadata.series.filter((se) => !seriesIdsInUpdate.includes(se.id))
+          seriesIdsRemoved.push(...seriesRemoved.map((se) => se.id))
+        }
+        if (Array.isArray(mediaPayload.metadata?.authors)) {
+          const authorIdsInUpdate = mediaPayload.metadata.authors.map((au) => au.id)
+          const authorsRemoved = libraryItem.media.metadata.authors.filter((au) => !authorIdsInUpdate.includes(au.id))
+          authorIdsRemoved.push(...authorsRemoved.map((au) => au.id))
+        }
       }
 
       if (libraryItem.media.update(mediaPayload)) {
         Logger.debug(`[LibraryItemController] Updated library item media ${libraryItem.media.metadata.title}`)
 
-        if (seriesRemoved.length) {
-          // Check remove empty series
-          Logger.debug(`[LibraryItemController] Series was removed from book. Check if series is now empty.`)
-          await this.checkRemoveEmptySeries(
-            libraryItem.media.id,
-            seriesRemoved.map((se) => se.id)
-          )
-        }
-
         await Database.updateLibraryItem(libraryItem)
         SocketAuthority.emitter('item_updated', libraryItem.toJSONExpanded())
         itemsUpdated++
       }
     }
 
+    if (seriesIdsRemoved.length) {
+      await this.checkRemoveEmptySeries(seriesIdsRemoved)
+    }
+    if (authorIdsRemoved.length) {
+      await this.checkRemoveAuthorsWithNoBooks(authorIdsRemoved)
+    }
+
     res.json({
       success: true,
       updates: itemsUpdated

server/routers/ApiRouter.js

@@ -348,11 +348,10 @@ class ApiRouter {
   //
   /**
    * Remove library item and associated entities
-   * @param {string} mediaType
    * @param {string} libraryItemId
    * @param {string[]} mediaItemIds array of bookId or podcastEpisodeId
    */
-  async handleDeleteLibraryItem(mediaType, libraryItemId, mediaItemIds) {
+  async handleDeleteLibraryItem(libraryItemId, mediaItemIds) {
     const numProgressRemoved = await Database.mediaProgressModel.destroy({
       where: {
         mediaItemId: mediaItemIds
@@ -362,29 +361,6 @@ class ApiRouter {
       Logger.info(`[ApiRouter] Removed ${numProgressRemoved} media progress entries for library item "${libraryItemId}"`)
     }
 
-    // TODO: Remove open sessions for library item
-
-    // Remove series if empty
-    if (mediaType === 'book') {
-      // TODO: update filter data
-      const bookSeries = await Database.bookSeriesModel.findAll({
-        where: {
-          bookId: mediaItemIds[0]
-        },
-        include: {
-          model: Database.seriesModel,
-          include: {
-            model: Database.bookModel
-          }
-        }
-      })
-      for (const bs of bookSeries) {
-        if (bs.series.books.length === 1) {
-          await this.removeEmptySeries(bs.series)
-        }
-      }
-    }
-
     // remove item from playlists
     const playlistsWithItem = await Database.playlistModel.getPlaylistsForMediaItemIds(mediaItemIds)
     for (const playlist of playlistsWithItem) {
@@ -423,6 +399,7 @@ class ApiRouter {
     // purge cover cache
     await CacheManager.purgeCoverCache(libraryItemId)
 
+    // Remove metadata file if in /metadata/items dir
     const itemMetadataPath = Path.join(global.MetadataPath, 'items', libraryItemId)
     if (await fs.pathExists(itemMetadataPath)) {
       Logger.info(`[ApiRouter] Removing item metadata at "${itemMetadataPath}"`)
@@ -437,32 +414,27 @@ class ApiRouter {
   }
 
   /**
-   * Used when a series is removed from a book
+   * After deleting book(s), remove empty series
-   * Series is removed if it only has 1 book
    *
-   * @param {string} bookId
    * @param {string[]} seriesIds
    */
-  async checkRemoveEmptySeries(bookId, seriesIds) {
+  async checkRemoveEmptySeries(seriesIds) {
     if (!seriesIds?.length) return
 
-    const bookSeries = await Database.bookSeriesModel.findAll({
+    const series = await Database.seriesModel.findAll({
       where: {
-        bookId,
+        id: seriesIds
-        seriesId: seriesIds
       },
-      include: [
+      attributes: ['id', 'name', 'libraryId'],
-        {
-          model: Database.seriesModel,
       include: {
-            model: Database.bookModel
+        model: Database.bookModel,
+        attributes: ['id']
       }
-        }
-      ]
     })
-    for (const bs of bookSeries) {
+
-      if (bs.series.books.length === 1) {
+    for (const s of series) {
-        await this.removeEmptySeries(bs.series)
+      if (!s.books.length) {
+        await this.removeEmptySeries(s)
       }
     }
   }
@@ -471,11 +443,10 @@ class ApiRouter {
    * Remove authors with no books and unset asin, description and imagePath
    * Note: Other implementation is in BookScanner.checkAuthorsRemovedFromBooks (can be merged)
    *
-   * @param {string} libraryId
    * @param {string[]} authorIds
    * @returns {Promise<void>}
    */
-  async checkRemoveAuthorsWithNoBooks(libraryId, authorIds) {
+  async checkRemoveAuthorsWithNoBooks(authorIds) {
     if (!authorIds?.length) return
 
     const bookAuthorsToRemove = (
@@ -495,10 +466,10 @@ class ApiRouter {
           },
           sequelize.where(sequelize.literal('(SELECT count(*) FROM bookAuthors ba WHERE ba.authorId = author.id)'), 0)
         ],
-        attributes: ['id', 'name'],
+        attributes: ['id', 'name', 'libraryId'],
         raw: true
       })
-    ).map((au) => ({ id: au.id, name: au.name }))
+    ).map((au) => ({ id: au.id, name: au.name, libraryId: au.libraryId }))
 
     if (bookAuthorsToRemove.length) {
       await Database.authorModel.destroy({
@@ -506,7 +477,7 @@ class ApiRouter {
           id: bookAuthorsToRemove.map((au) => au.id)
         }
       })
-      bookAuthorsToRemove.forEach(({ id, name }) => {
+      bookAuthorsToRemove.forEach(({ id, name, libraryId }) => {
         Database.removeAuthorFromFilterData(libraryId, id)
         // TODO: Clients were expecting full author in payload but its unnecessary
         SocketAuthority.emitter('author_removed', { id, libraryId })