From ff67bed2c45e18125c9f1fc6da906439d1655bdf Mon Sep 17 00:00:00 2001 From: Muppetteer Date: Wed, 10 Dec 2025 18:23:16 +1100 Subject: [PATCH] Fix: locking fields discards unsaved changes (#1799) * Fixes data loss on lock fields * Test combined metadata update and lock field --- .../service/metadata/BookMetadataUpdater.java | 6 ++-- .../metadata/BookMetadataUpdaterTest.java | 30 +++++++++++++++++++ 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/booklore-api/src/main/java/com/adityachandel/booklore/service/metadata/BookMetadataUpdater.java b/booklore-api/src/main/java/com/adityachandel/booklore/service/metadata/BookMetadataUpdater.java index 1b1de044e..937fc7735 100644 --- a/booklore-api/src/main/java/com/adityachandel/booklore/service/metadata/BookMetadataUpdater.java +++ b/booklore-api/src/main/java/com/adityachandel/booklore/service/metadata/BookMetadataUpdater.java @@ -74,8 +74,6 @@ public class BookMetadataUpdater { MetadataClearFlags clearFlags = wrapper.getClearFlags(); BookMetadataEntity metadata = bookEntity.getMetadata(); - updateLocks(newMetadata, metadata); - boolean thumbnailRequiresUpdate = StringUtils.hasText(newMetadata.getThumbnailUrl()); boolean hasMetadataChanges = MetadataChangeDetector.isDifferent(newMetadata, metadata, clearFlags); boolean hasValueChanges = MetadataChangeDetector.hasValueChanges(newMetadata, metadata, clearFlags); @@ -84,7 +82,8 @@ public class BookMetadataUpdater { return; } - if (metadata.areAllFieldsLocked()) { + // If all fields are locked we must allow unlocking, hasValueChanges will be false + if (metadata.areAllFieldsLocked() && hasValueChanges) { log.warn("All fields are locked for book ID {}. Skipping update.", bookId); return; } @@ -103,6 +102,7 @@ public class BookMetadataUpdater { updateTagsIfNeeded(newMetadata, metadata, clearFlags, mergeTags, replaceMode); bookReviewUpdateService.updateBookReviews(newMetadata, metadata, clearFlags, mergeCategories); updateThumbnailIfNeeded(bookId, newMetadata, metadata, updateThumbnail); + updateLocks(newMetadata, metadata); bookRepository.save(bookEntity); diff --git a/booklore-api/src/test/java/com/adityachandel/booklore/service/metadata/BookMetadataUpdaterTest.java b/booklore-api/src/test/java/com/adityachandel/booklore/service/metadata/BookMetadataUpdaterTest.java index bb9b25d31..326554576 100644 --- a/booklore-api/src/test/java/com/adityachandel/booklore/service/metadata/BookMetadataUpdaterTest.java +++ b/booklore-api/src/test/java/com/adityachandel/booklore/service/metadata/BookMetadataUpdaterTest.java @@ -259,4 +259,34 @@ class BookMetadataUpdaterTest { assertTrue(bookEntity.getMetadata().getMoods().stream().anyMatch(m -> m.getName().equals("Mood2"))); assertTrue(bookEntity.getMetadata().getMoods().stream().anyMatch(m -> m.getName().equals("Mood3"))); } + + @Test + void setBookMetadata_withLockField_shouldUpdateAndLock() { + BookEntity bookEntity = new BookEntity(); + bookEntity.setId(1L); + + BookMetadataEntity metadataEntity = new BookMetadataEntity(); + metadataEntity.setTitle("Old Title"); + metadataEntity.setTitleLocked(false); + bookEntity.setMetadata(metadataEntity); + + BookMetadata newMetadata = new BookMetadata(); + newMetadata.setTitle("New Title"); + newMetadata.setTitleLocked(true); + + MetadataUpdateWrapper wrapper = MetadataUpdateWrapper.builder() + .metadata(newMetadata) + .build(); + + MetadataUpdateContext context = MetadataUpdateContext.builder() + .bookEntity(bookEntity) + .metadataUpdateWrapper(wrapper) + .replaceMode(MetadataReplaceMode.REPLACE_ALL) + .build(); + + bookMetadataUpdater.setBookMetadata(context); + + assertEquals("New Title", bookEntity.getMetadata().getTitle()); + assertTrue(bookEntity.getMetadata().getTitleLocked()); + } }