From b97809589e6687692ea3152f59e24cc4f67e651f Mon Sep 17 00:00:00 2001 From: Michael Mayer Date: Mon, 17 Nov 2025 05:28:13 +0100 Subject: [PATCH] Frontend: Refactor Batch Edit model and use Promises #271 #5324 Signed-off-by: Michael Mayer --- .prettierignore | 21 ++++ frontend/.prettierignore | 4 + frontend/eslint.config.mjs | 3 +- frontend/src/component/location/dialog.vue | 44 +++---- frontend/src/component/notify.vue | 2 +- frontend/src/component/photo/batch-edit.vue | 43 +++---- frontend/src/component/photo/edit/people.vue | 112 ++++++++++-------- frontend/src/model/batch-edit.js | 61 ++++++---- frontend/src/model/logs.js | 10 +- .../vitest/component/photo/batch-edit.test.js | 8 +- .../tests/vitest/model/batch-edit.test.js | 28 +++-- 11 files changed, 194 insertions(+), 142 deletions(-) create mode 100644 .prettierignore diff --git a/.prettierignore b/.prettierignore new file mode 100644 index 000000000..373a78f9b --- /dev/null +++ b/.prettierignore @@ -0,0 +1,21 @@ +coverage/ +node_modules/ +screenshots/ +acceptance/ +build/ +dist/ +bin/ +tests/upload-files/ +*.html +*.md +.* +.idea +.codex +.local +.config +.github +.tmp +.local +.cache +.gocache +.var diff --git a/frontend/.prettierignore b/frontend/.prettierignore index 54b09490b..dce684fd7 100644 --- a/frontend/.prettierignore +++ b/frontend/.prettierignore @@ -4,8 +4,12 @@ tests/screenshots/ tests/acceptance/screenshots/ tests/upload-files/ *.html +*.md +.* .idea .codex +.local +.config .github .tmp .local diff --git a/frontend/eslint.config.mjs b/frontend/eslint.config.mjs index f51d41cb0..6adae7df4 100644 --- a/frontend/eslint.config.mjs +++ b/frontend/eslint.config.mjs @@ -76,7 +76,8 @@ export default defineConfig([ }, }, rules: { - "indent": ["error", 2, { SwitchCase: 1 }], + // Defer indentation to Prettier so we don't get conflicting expectations. + "indent": "off", "linebreak-style": ["error", "unix"], "quotes": [ "off", diff --git a/frontend/src/component/location/dialog.vue b/frontend/src/component/location/dialog.vue index a7a21fc1e..7e940b0ed 100644 --- a/frontend/src/component/location/dialog.vue +++ b/frontend/src/component/location/dialog.vue @@ -295,38 +295,40 @@ export default { this.performPlaceSearch(query); }, 300); // 300ms delay after user stops typing }, - async performPlaceSearch(query) { + performPlaceSearch(query) { if (!query || query.length < 2) { this.searchLoading = false; - return; + return Promise.resolve(); } - try { - const response = await this.$api.get("places/search", { + return this.$api + .get("places/search", { params: { q: query, count: 10, locale: this.$config.getLanguageLocale() || "en", }, - }); - - if (this.searchQuery === query) { - if (response.data && Array.isArray(response.data)) { - this.searchResults = this.normalizeSearchResults(response.data); - } else { + }) + .then((response) => { + if (this.searchQuery === query) { + if (response.data && Array.isArray(response.data)) { + this.searchResults = this.normalizeSearchResults(response.data); + } else { + this.searchResults = []; + } + } + }) + .catch((error) => { + console.error("Place search error:", error); + if (this.searchQuery === query) { this.searchResults = []; } - } - } catch (error) { - console.error("Place search error:", error); - if (this.searchQuery === query) { - this.searchResults = []; - } - } finally { - if (this.searchQuery === query) { - this.searchLoading = false; - } - } + }) + .finally(() => { + if (this.searchQuery === query) { + this.searchLoading = false; + } + }); }, onPlaceSelected(place) { if (place && place.lat && place.lng) { diff --git a/frontend/src/component/notify.vue b/frontend/src/component/notify.vue index 3a534a794..c916745b8 100644 --- a/frontend/src/component/notify.vue +++ b/frontend/src/component/notify.vue @@ -149,7 +149,7 @@ export default { const message = this.messages.shift(); if (this.message.timer > 0) { clearTimeout(this.message.timer); - }; + } if (message) { this.message = message; diff --git a/frontend/src/component/photo/batch-edit.vue b/frontend/src/component/photo/batch-edit.vue index a36b7215d..9e3fa8619 100644 --- a/frontend/src/component/photo/batch-edit.vue +++ b/frontend/src/component/photo/batch-edit.vue @@ -747,7 +747,7 @@ export default { // Refresh available options each time the dialog opens to avoid stale caches await this.fetchAvailableOptions(); - await this.model.getData(this.selection); + await this.model.load(this.selection); this.values = this.model.values; this.setFormData(); this.allSelectedLength = this.model.getLengthOfAllSelected(); @@ -1375,7 +1375,7 @@ export default { this.locationDialog = false; }, - async save(close) { + save(close) { this.saving = true; // Filter form data to only include fields with changes @@ -1386,36 +1386,31 @@ export default { if (close) { this.$emit("close"); } - return; + return Promise.resolve(); } // Get currently selected photo UIDs from the model const currentlySelectedUIDs = this.model.selection.filter((photo) => photo.selected).map((photo) => photo.id); - try { - await this.model.save(currentlySelectedUIDs, filteredFormData); - - // Update form data with new values from backend (force-refresh to avoid stale UI) - try { - // Only refresh the values for the current selection to avoid losing sidebar items - await this.model.getValuesForSelection(currentlySelectedUIDs); + return this.model + .save(currentlySelectedUIDs, filteredFormData) + .then(() => { + // Save response already includes updated values, so reuse them to avoid a second POST. this.values = this.model.values; - } catch { - // Fallback to response values if re-fetch fails - this.values = this.model.values; - } - this.setFormData(); + this.setFormData(); - this.$notify.success(this.$gettext("Changes successfully saved")); + this.$notify.success(this.$gettext("Changes successfully saved")); - if (close) { - this.$emit("close"); - } - } catch { - this.$notify.error(this.$gettext("Failed to save changes")); - } finally { - this.saving = false; - } + if (close) { + this.$emit("close"); + } + }) + .catch(() => { + this.$notify.error(this.$gettext("Failed to save changes")); + }) + .finally(() => { + this.saving = false; + }); }, getFilteredFormData() { const filtered = {}; diff --git a/frontend/src/component/photo/edit/people.vue b/frontend/src/component/photo/edit/people.vue index 6c7648672..4eec9dc54 100644 --- a/frontend/src/component/photo/edit/people.vue +++ b/frontend/src/component/photo/edit/people.vue @@ -116,6 +116,8 @@ import Subject from "model/subject"; import PConfirmDialog from "component/confirm/dialog.vue"; import PActionMenu from "component/action/menu.vue"; +const SUBJECT_NOT_FOUND = "subject-not-found"; + export default { name: "PTabPhotoPeople", components: { PConfirmDialog, PActionMenu }, @@ -242,68 +244,84 @@ export default { }, ]; }, - async loadSubject(uid) { - try { - return await new Subject({ UID: uid }).find(uid); - } catch (err) { + loadSubject(uid) { + return new Subject({ UID: uid }).find(uid).catch((err) => { console.error("faces: failed loading subject", err); return null; - } + }); }, - async onGoToPerson(marker) { + onGoToPerson(marker) { if (!marker?.SubjUID) { - return; + return Promise.resolve(); } - let subject = this.findPerson(marker.SubjUID); + const cached = this.findPerson(marker.SubjUID); + const subjectPromise = cached + ? Promise.resolve(new Subject(cached)) + : this.loadSubject(marker.SubjUID).then((subject) => { + if (!subject) { + this.$notify.error(this.$gettext("Person not found")); + return null; + } + this.updatePersonList(subject); + return subject; + }); - if (!subject) { - subject = await this.loadSubject(marker.SubjUID); - if (!subject) { - this.$notify.error(this.$gettext("Person not found")); - return; - } - this.updatePersonList(subject); - } else { - subject = new Subject(subject); - } - - const route = subject.route("all"); - const resolved = this.$router.resolve(route); - this.$util.openUrl(resolved.href); + return subjectPromise + .then((subject) => { + if (!subject) { + return; + } + const route = subject.route("all"); + const resolved = this.$router.resolve(route); + this.$util.openUrl(resolved.href); + }) + .catch((err) => { + if (!err || err.message !== SUBJECT_NOT_FOUND) { + console.error("faces: failed opening person", err); + } + }); }, - async onSetPersonCover(marker) { + onSetPersonCover(marker) { if (this.busy || !marker?.SubjUID || !marker?.Thumb) { - return; + return Promise.resolve(); } this.busy = true; this.$notify.blockUI("busy"); - try { - let subject = this.findPerson(marker.SubjUID); + const cached = this.findPerson(marker.SubjUID); + const subjectPromise = cached + ? Promise.resolve(new Subject(cached)) + : this.loadSubject(marker.SubjUID).then((subject) => { + if (!subject) { + this.$notify.error(this.$gettext("Person not found")); + return null; + } + return subject; + }); - if (subject) { - subject = new Subject(subject); - } else { - subject = await this.loadSubject(marker.SubjUID); - } - - if (!subject) { - this.$notify.error(this.$gettext("Person not found")); - return; - } - - const updated = await subject.setCover(marker.Thumb); - this.updatePersonList(updated); - this.$notify.success(this.$gettext("Person cover updated")); - } catch (err) { - console.error("faces: failed setting person cover", err); - this.$notify.error(this.$gettext("Could not update person cover")); - } finally { - this.$notify.unblockUI(); - this.busy = false; - } + return subjectPromise + .then((subject) => { + if (!subject) { + return null; + } + return subject.setCover(marker.Thumb); + }) + .then((updated) => { + this.updatePersonList(updated); + this.$notify.success(this.$gettext("Person cover updated")); + }) + .catch((err) => { + if (err) { + console.error("faces: failed setting person cover", err); + this.$notify.error(this.$gettext("Could not update person cover")); + } + }) + .finally(() => { + this.$notify.unblockUI(); + this.busy = false; + }); }, onApprove(model) { if (this.busy || !model) return; diff --git a/frontend/src/model/batch-edit.js b/frontend/src/model/batch-edit.js index 0e87710d4..e230ad26c 100644 --- a/frontend/src/model/batch-edit.js +++ b/frontend/src/model/batch-edit.js @@ -58,36 +58,47 @@ export class Batch extends Model { } save(selection, values) { - return $api - .post("batch/photos/edit", { photos: selection, values: values }) - .then((response) => { - if (response.data.values) { - this.values = response.data.values; - } - return Promise.resolve(this); - }) - .catch((error) => { - throw error; - }); - } + return $api.post("batch/photos/edit", { photos: selection, values }).then((response) => { + if (response?.data?.models?.length) { + const updatedMap = new Map( + response.data.models.map((raw) => { + const photo = new Photo(); + photo.setValues(raw); + return [photo.UID, photo]; + }) + ); - async getData(selection) { - const response = await $api.post("batch/photos/edit", { photos: selection }); - const models = response.data.models || []; + this.models = this.models.map((existing) => { + const updated = updatedMap.get(existing.UID); + if (updated) { + existing.setValues(updated); + updatedMap.delete(existing.UID); + } + return existing; + }); - this.models = models.map((m) => { - const modelInstance = new Photo(); - return modelInstance.setValues(m); + updatedMap.forEach((photo) => { + this.models.push(photo); + }); + } + + if (response?.data?.values) { + this.values = response.data.values; + } + + return this; }); - - this.values = response.data.values; - this.setSelections(selection); } - async getValuesForSelection(selection) { - const response = await $api.post("batch/photos/edit", { photos: selection }); - this.values = response.data.values; - return this.values; + // load fetches the current selection (+ aggregated form values) and hydrates Photo instances. + load(selection) { + return $api.post("batch/photos/edit", { photos: selection }).then((response) => { + const models = response.data.models || []; + this.models = models.map((m) => new Photo(m)); + this.values = response.data.values; + this.setSelections(selection); + return this; + }); } setSelections(selection) { diff --git a/frontend/src/model/logs.js b/frontend/src/model/logs.js index e56ebd10b..9baae6ad2 100644 --- a/frontend/src/model/logs.js +++ b/frontend/src/model/logs.js @@ -39,15 +39,7 @@ function splitSegments(message) { } // All log levels ordered by severity. -export const AuditSeverityNames = Object.freeze([ - "panic", - "fatal", - "error", - "warning", - "info", - "debug", - "trace", -]); +export const AuditSeverityNames = Object.freeze(["panic", "fatal", "error", "warning", "info", "debug", "trace"]); // Audit logs currently only exist with the following levels: // error, warning, info, and debug. diff --git a/frontend/tests/vitest/component/photo/batch-edit.test.js b/frontend/tests/vitest/component/photo/batch-edit.test.js index b65647508..d4d73f2f6 100644 --- a/frontend/tests/vitest/component/photo/batch-edit.test.js +++ b/frontend/tests/vitest/component/photo/batch-edit.test.js @@ -116,9 +116,8 @@ describe("component/photo/edit/batch", () => { { id: "uid2", selected: true }, { id: "uid3", selected: true }, ], - getData: vi.fn(), + load: vi.fn(), save: vi.fn(), - getValuesForSelection: vi.fn(), getDefaultFormData: vi.fn(), getLengthOfAllSelected: vi.fn(), isSelected: vi.fn(), @@ -127,9 +126,8 @@ describe("component/photo/edit/batch", () => { }; // Configure mock method behaviors - mockBatchInstance.getData.mockResolvedValue(mockBatchInstance); + mockBatchInstance.load.mockResolvedValue(mockBatchInstance); mockBatchInstance.save.mockResolvedValue(mockBatchInstance); - mockBatchInstance.getValuesForSelection.mockResolvedValue(mockValues); mockBatchInstance.getDefaultFormData.mockReturnValue(mockDefaultFormData); mockBatchInstance.getLengthOfAllSelected.mockReturnValue(3); mockBatchInstance.isSelected.mockReturnValue(true); @@ -447,7 +445,7 @@ describe("component/photo/edit/batch", () => { await wrapper.setProps({ visible: true }); await nextTick(); await nextTick(); - expect(mockBatchInstance.getData).toHaveBeenCalledWith(mockSelection); + expect(mockBatchInstance.load).toHaveBeenCalledWith(mockSelection); }); it("should emit close event", () => { diff --git a/frontend/tests/vitest/model/batch-edit.test.js b/frontend/tests/vitest/model/batch-edit.test.js index f696e774a..45df193ae 100644 --- a/frontend/tests/vitest/model/batch-edit.test.js +++ b/frontend/tests/vitest/model/batch-edit.test.js @@ -1,6 +1,7 @@ import { describe, it, expect } from "vitest"; import "../fixtures"; import { Batch } from "model/batch-edit"; +import { Photo } from "model/photo"; describe("model/batch-edit", () => { it("should return defaults", () => { @@ -88,23 +89,29 @@ describe("model/batch-edit", () => { it("should call save and update values from response", async () => { const b = new Batch(); - const selection = [5, 7]; + const selection = ["pt20fg34bbwdm2ld", "pt20fg2qikiy7zax"]; const values = { Title: { value: "New" } }; - // Mock endpoint expected by $api: baseURL is "/api/v1" + const existing = new Photo({ UID: "pt20fg34bbwdm2ld", Title: "Old" }); + b.models = [existing]; + const { Mock } = await import("../fixtures"); - Mock.onPost("api/v1/batch/photos/edit", { photos: selection, values }).reply( - 200, - { values: { Title: { value: "Saved" } } }, - { "Content-Type": "application/json; charset=utf-8" } - ); + Mock.onPost("api/v1/batch/photos/edit", { photos: selection, values }).reply(200, { + models: [ + { UID: "pt20fg34bbwdm2ld", Title: "Updated" }, + { UID: "pt20fg2qikiy7zb0", Title: "New" }, + ], + values: { Title: { value: "Saved" } }, + }); const result = await b.save(selection, values); expect(result).toBe(b); expect(b.values).toEqual({ Title: { value: "Saved" } }); + expect(b.models.find((m) => m.UID === "pt20fg34bbwdm2ld").Title).toBe("Updated"); + expect(b.models.some((m) => m.UID === "pt20fg2qikiy7zb0")).toBe(true); }); - it("should load data (models and values) via getData", async () => { + it("should load data (models and values) via load", async () => { const b = new Batch(); const selection = [101, 102]; @@ -122,10 +129,13 @@ describe("model/batch-edit", () => { { "Content-Type": "application/json; charset=utf-8" } ); - await b.getData(selection); + const result = await b.load(selection); + expect(result).toBe(b); expect(Array.isArray(b.models)).toBe(true); expect(b.models.length).toBe(2); + expect(b.models[0]).toBeInstanceOf(Photo); + expect(b.models[1]).toBeInstanceOf(Photo); expect(b.values).toEqual({ Title: { mixed: true } }); expect(b.selection).toEqual([ { id: 101, selected: true },