diff --git a/src/palace/manager/api/admin/controller/work_editor.py b/src/palace/manager/api/admin/controller/work_editor.py index 1a8ffbcc43..b4f60dacf3 100644 --- a/src/palace/manager/api/admin/controller/work_editor.py +++ b/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""" diff --git a/tests/manager/api/admin/controller/test_work_editor.py b/tests/manager/api/admin/controller/test_work_editor.py index b09f0344a9..75f4f08495 100644 --- a/tests/manager/api/admin/controller/test_work_editor.py +++ b/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