Per-library work suppression is idempotent. (PP-1378) (#1905)

* Failing test for this issue.

* Un/suppression is idempotent.

src/palace/manager/api/admin/controller/work_editor.py

@@ -390,12 +390,18 @@ def suppress(
             # Something went wrong.
             return work
 
-        work.suppressed_for.append(library)
-        self.log.info(
-            f"Suppressed {identifier_type}/{identifier} (work id: {work.id}) for library {library.short_name}."
-        )
+        if library in work.suppressed_for:
+            # If the library is already suppressed, we don't need to do anything.
+            message = f"Already suppressed {identifier_type}/{identifier} (work id: {work.id}) for library {library.short_name}."
+        else:
+            # Otherwise, add the library to the suppressed list.
+            work.suppressed_for.append(library)
+            message = f"Suppressed {identifier_type}/{identifier} (work id: {work.id}) for library {library.short_name}."
 
-        return Response("", 200)
+        self.log.info(message)
+        return Response(
+            json.dumps({"message": message}), 200, mimetype="application/json"
+        )
 
     def unsuppress(
         self, identifier_type: str, identifier: str
@@ -416,12 +422,18 @@ def unsuppress(
             # Something went wrong.
             return work
 
-        work.suppressed_for.remove(library)
-        self.log.info(
-            f"Unsuppressed {identifier_type}/{identifier} (work id: {work.id}) for library {library.short_name}."
-        )
+        if library not in work.suppressed_for:
+            # If the library is not suppressed, we don't need to do anything.
+            message = f"Already unsuppressed {identifier_type}/{identifier} (work id: {work.id}) for library {library.short_name}."
+        else:
+            # Otherwise, remove the library from the suppressed list.
+            work.suppressed_for.remove(library)
+            message = f"Unsuppressed {identifier_type}/{identifier} (work id: {work.id}) for library {library.short_name}."
 
-        return Response("", 200)
+        self.log.info(message)
+        return Response(
+            json.dumps({"message": message}), 200, mimetype="application/json"
+        )
 
     def refresh_metadata(self, identifier_type, identifier, provider=None):
         """Refresh the metadata for a book from the content server"""

tests/manager/api/admin/controller/test_work_editor.py

@@ -741,6 +741,14 @@ def test_suppress(self, work_fixture: WorkFixture):
             assert 200 == response.status_code
             assert library in work.suppressed_for
 
+        # We should not fail if we suppress the already-suppressed work again.
+        with work_fixture.request_context_with_library_and_admin("/"):
+            response = work_fixture.manager.admin_work_controller.suppress(
+                lp.identifier.type, lp.identifier.identifier
+            )
+            assert 200 == response.status_code
+            assert library in work.suppressed_for
+
         # test non-existent id
         with work_fixture.request_context_with_library_and_admin("/"):
             response = work_fixture.manager.admin_work_controller.suppress(
@@ -781,7 +789,14 @@ def test_unsuppress(self, work_fixture: WorkFixture):
             response = work_fixture.manager.admin_work_controller.unsuppress(
                 lp.identifier.type, lp.identifier.identifier
             )
+            assert 200 == response.status_code
+            assert library not in work.suppressed_for
 
+        # We should not fail if we unsuppress the already-unsuppressed work again.
+        with work_fixture.request_context_with_library_and_admin("/"):
+            response = work_fixture.manager.admin_work_controller.unsuppress(
+                lp.identifier.type, lp.identifier.identifier
+            )
             assert 200 == response.status_code
             assert library not in work.suppressed_for