Merge pull request #4088 from nichwall/checkRemoveAuthors_fix

Fix empty series delete check

server/routers/ApiRouter.js

@@ -392,21 +392,51 @@ class ApiRouter {
   async checkRemoveEmptySeries(seriesIds) {
     if (!seriesIds?.length) return
 
-    const series = await Database.seriesModel.findAll({
-      where: {
-        id: seriesIds
-      },
-      attributes: ['id', 'name', 'libraryId'],
-      include: {
-        model: Database.bookModel,
-        attributes: ['id']
-      }
-    })
+    const transaction = await Database.sequelize.transaction()
+    try {
+      const seriesToRemove = (
+        await Database.seriesModel.findAll({
+          where: [
+            {
+              id: seriesIds
+            },
+            sequelize.where(sequelize.literal('(SELECT count(*) FROM bookSeries bs WHERE bs.seriesId = series.id)'), 0)
+          ],
+          attributes: ['id', 'name', 'libraryId'],
+          include: {
+            model: Database.bookModel,
+            attributes: ['id'],
+            required: false // Ensure it includes series even if no books exist
+          },
+          transaction
+        })
+      ).map((s) => ({ id: s.id, name: s.name, libraryId: s.libraryId }))
 
-    for (const s of series) {
-      if (!s.books.length) {
-        await this.removeEmptySeries(s)
+      if (seriesToRemove.length) {
+        await Database.seriesModel.destroy({
+          where: {
+            id: seriesToRemove.map((s) => s.id)
+          },
+          transaction
+        })
       }
+
+      await transaction.commit()
+
+      seriesToRemove.forEach(({ id, name, libraryId }) => {
+        Logger.info(`[ApiRouter] Series "${name}" is now empty. Removing series`)
+
+        // Remove series from library filter data
+        Database.removeSeriesFromFilterData(libraryId, id)
+        SocketAuthority.emitter('series_removed', { id: id, libraryId: libraryId })
+      })
+      // Close rss feeds - remove from db and emit socket event
+      if (seriesToRemove.length) {
+        await RssFeedManager.closeFeedsForEntityIds(seriesToRemove.map((s) => s.id))
+      }
+    } catch (error) {
+      await transaction.rollback()
+      Logger.error(`[ApiRouter] Error removing empty series: ${error.message}`)
     }
   }
 
@@ -420,61 +450,56 @@ class ApiRouter {
   async checkRemoveAuthorsWithNoBooks(authorIds) {
     if (!authorIds?.length) return
 
-    const bookAuthorsToRemove = (
-      await Database.authorModel.findAll({
-        where: [
-          {
-            id: authorIds,
-            asin: {
-              [sequelize.Op.or]: [null, '']
+    const transaction = await Database.sequelize.transaction()
+    try {
+      // Select authors with locking to prevent concurrent updates
+      const bookAuthorsToRemove = (
+        await Database.authorModel.findAll({
+          where: [
+            {
+              id: authorIds,
+              asin: {
+                [sequelize.Op.or]: [null, '']
+              },
+              description: {
+                [sequelize.Op.or]: [null, '']
+              },
+              imagePath: {
+                [sequelize.Op.or]: [null, '']
+              }
             },
-            description: {
-              [sequelize.Op.or]: [null, '']
-            },
-            imagePath: {
-              [sequelize.Op.or]: [null, '']
-            }
-          },
-          sequelize.where(sequelize.literal('(SELECT count(*) FROM bookAuthors ba WHERE ba.authorId = author.id)'), 0)
-        ],
-        attributes: ['id', 'name', 'libraryId'],
-        raw: true
-      })
-    ).map((au) => ({ id: au.id, name: au.name, libraryId: au.libraryId }))
+            sequelize.where(sequelize.literal('(SELECT count(*) FROM bookAuthors ba WHERE ba.authorId = author.id)'), 0)
+          ],
+          attributes: ['id', 'name', 'libraryId'],
+          raw: true,
+          transaction
+        })
+      ).map((au) => ({ id: au.id, name: au.name, libraryId: au.libraryId }))
 
-    if (bookAuthorsToRemove.length) {
-      await Database.authorModel.destroy({
-        where: {
-          id: bookAuthorsToRemove.map((au) => au.id)
-        }
-      })
+      if (bookAuthorsToRemove.length) {
+        await Database.authorModel.destroy({
+          where: {
+            id: bookAuthorsToRemove.map((au) => au.id)
+          },
+          transaction
+        })
+      }
+
+      await transaction.commit()
+
+      // Remove all book authors after completing remove from database
       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 })
         Logger.info(`[ApiRouter] Removed author "${name}" with no books`)
       })
+    } catch (error) {
+      await transaction.rollback()
+      Logger.error(`[ApiRouter] Error removing authors: ${error.message}`)
     }
   }
 
-  /**
-   * Remove an empty series & close an open RSS feed
-   * @param {import('../models/Series')} series
-   */
-  async removeEmptySeries(series) {
-    await RssFeedManager.closeFeedForEntityId(series.id)
-    Logger.info(`[ApiRouter] Series "${series.name}" is now empty. Removing series`)
-
-    // Remove series from library filter data
-    Database.removeSeriesFromFilterData(series.libraryId, series.id)
-    SocketAuthority.emitter('series_removed', {
-      id: series.id,
-      libraryId: series.libraryId
-    })
-
-    await series.destroy()
-  }
-
   async getUserListeningSessionsHelper(userId) {
     const userSessions = await Database.getPlaybackSessions({ userId })
     return userSessions.sort((a, b) => b.updatedAt - a.updatedAt)