Frontend: Refactor Batch Edit model and use Promises #271 #5324

Signed-off-by: Michael Mayer <michael@photoprism.app>
This commit is contained in:
Michael Mayer
2025-11-17 05:28:13 +01:00
parent 7240096965
commit b97809589e
11 changed files with 194 additions and 142 deletions

21
.prettierignore Normal file
View File

@@ -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

View File

@@ -4,8 +4,12 @@ tests/screenshots/
tests/acceptance/screenshots/ tests/acceptance/screenshots/
tests/upload-files/ tests/upload-files/
*.html *.html
*.md
.*
.idea .idea
.codex .codex
.local
.config
.github .github
.tmp .tmp
.local .local

View File

@@ -76,7 +76,8 @@ export default defineConfig([
}, },
}, },
rules: { rules: {
"indent": ["error", 2, { SwitchCase: 1 }], // Defer indentation to Prettier so we don't get conflicting expectations.
"indent": "off",
"linebreak-style": ["error", "unix"], "linebreak-style": ["error", "unix"],
"quotes": [ "quotes": [
"off", "off",

View File

@@ -295,38 +295,40 @@ export default {
this.performPlaceSearch(query); this.performPlaceSearch(query);
}, 300); // 300ms delay after user stops typing }, 300); // 300ms delay after user stops typing
}, },
async performPlaceSearch(query) { performPlaceSearch(query) {
if (!query || query.length < 2) { if (!query || query.length < 2) {
this.searchLoading = false; this.searchLoading = false;
return; return Promise.resolve();
} }
try { return this.$api
const response = await this.$api.get("places/search", { .get("places/search", {
params: { params: {
q: query, q: query,
count: 10, count: 10,
locale: this.$config.getLanguageLocale() || "en", locale: this.$config.getLanguageLocale() || "en",
}, },
}); })
.then((response) => {
if (this.searchQuery === query) { if (this.searchQuery === query) {
if (response.data && Array.isArray(response.data)) { if (response.data && Array.isArray(response.data)) {
this.searchResults = this.normalizeSearchResults(response.data); this.searchResults = this.normalizeSearchResults(response.data);
} else { } else {
this.searchResults = [];
}
}
})
.catch((error) => {
console.error("Place search error:", error);
if (this.searchQuery === query) {
this.searchResults = []; this.searchResults = [];
} }
} })
} catch (error) { .finally(() => {
console.error("Place search error:", error); if (this.searchQuery === query) {
if (this.searchQuery === query) { this.searchLoading = false;
this.searchResults = []; }
} });
} finally {
if (this.searchQuery === query) {
this.searchLoading = false;
}
}
}, },
onPlaceSelected(place) { onPlaceSelected(place) {
if (place && place.lat && place.lng) { if (place && place.lat && place.lng) {

View File

@@ -149,7 +149,7 @@ export default {
const message = this.messages.shift(); const message = this.messages.shift();
if (this.message.timer > 0) { if (this.message.timer > 0) {
clearTimeout(this.message.timer); clearTimeout(this.message.timer);
}; }
if (message) { if (message) {
this.message = message; this.message = message;

View File

@@ -747,7 +747,7 @@ export default {
// Refresh available options each time the dialog opens to avoid stale caches // Refresh available options each time the dialog opens to avoid stale caches
await this.fetchAvailableOptions(); await this.fetchAvailableOptions();
await this.model.getData(this.selection); await this.model.load(this.selection);
this.values = this.model.values; this.values = this.model.values;
this.setFormData(); this.setFormData();
this.allSelectedLength = this.model.getLengthOfAllSelected(); this.allSelectedLength = this.model.getLengthOfAllSelected();
@@ -1375,7 +1375,7 @@ export default {
this.locationDialog = false; this.locationDialog = false;
}, },
async save(close) { save(close) {
this.saving = true; this.saving = true;
// Filter form data to only include fields with changes // Filter form data to only include fields with changes
@@ -1386,36 +1386,31 @@ export default {
if (close) { if (close) {
this.$emit("close"); this.$emit("close");
} }
return; return Promise.resolve();
} }
// Get currently selected photo UIDs from the model // Get currently selected photo UIDs from the model
const currentlySelectedUIDs = this.model.selection.filter((photo) => photo.selected).map((photo) => photo.id); const currentlySelectedUIDs = this.model.selection.filter((photo) => photo.selected).map((photo) => photo.id);
try { return this.model
await this.model.save(currentlySelectedUIDs, filteredFormData); .save(currentlySelectedUIDs, filteredFormData)
.then(() => {
// Update form data with new values from backend (force-refresh to avoid stale UI) // Save response already includes updated values, so reuse them to avoid a second POST.
try {
// Only refresh the values for the current selection to avoid losing sidebar items
await this.model.getValuesForSelection(currentlySelectedUIDs);
this.values = this.model.values; this.values = this.model.values;
} catch { this.setFormData();
// Fallback to response values if re-fetch fails
this.values = this.model.values;
}
this.setFormData();
this.$notify.success(this.$gettext("Changes successfully saved")); this.$notify.success(this.$gettext("Changes successfully saved"));
if (close) { if (close) {
this.$emit("close"); this.$emit("close");
} }
} catch { })
this.$notify.error(this.$gettext("Failed to save changes")); .catch(() => {
} finally { this.$notify.error(this.$gettext("Failed to save changes"));
this.saving = false; })
} .finally(() => {
this.saving = false;
});
}, },
getFilteredFormData() { getFilteredFormData() {
const filtered = {}; const filtered = {};

View File

@@ -116,6 +116,8 @@ import Subject from "model/subject";
import PConfirmDialog from "component/confirm/dialog.vue"; import PConfirmDialog from "component/confirm/dialog.vue";
import PActionMenu from "component/action/menu.vue"; import PActionMenu from "component/action/menu.vue";
const SUBJECT_NOT_FOUND = "subject-not-found";
export default { export default {
name: "PTabPhotoPeople", name: "PTabPhotoPeople",
components: { PConfirmDialog, PActionMenu }, components: { PConfirmDialog, PActionMenu },
@@ -242,68 +244,84 @@ export default {
}, },
]; ];
}, },
async loadSubject(uid) { loadSubject(uid) {
try { return new Subject({ UID: uid }).find(uid).catch((err) => {
return await new Subject({ UID: uid }).find(uid);
} catch (err) {
console.error("faces: failed loading subject", err); console.error("faces: failed loading subject", err);
return null; return null;
} });
}, },
async onGoToPerson(marker) { onGoToPerson(marker) {
if (!marker?.SubjUID) { 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) { return subjectPromise
subject = await this.loadSubject(marker.SubjUID); .then((subject) => {
if (!subject) { if (!subject) {
this.$notify.error(this.$gettext("Person not found")); return;
return; }
} const route = subject.route("all");
this.updatePersonList(subject); const resolved = this.$router.resolve(route);
} else { this.$util.openUrl(resolved.href);
subject = new Subject(subject); })
} .catch((err) => {
if (!err || err.message !== SUBJECT_NOT_FOUND) {
const route = subject.route("all"); console.error("faces: failed opening person", err);
const resolved = this.$router.resolve(route); }
this.$util.openUrl(resolved.href); });
}, },
async onSetPersonCover(marker) { onSetPersonCover(marker) {
if (this.busy || !marker?.SubjUID || !marker?.Thumb) { if (this.busy || !marker?.SubjUID || !marker?.Thumb) {
return; return Promise.resolve();
} }
this.busy = true; this.busy = true;
this.$notify.blockUI("busy"); this.$notify.blockUI("busy");
try { const cached = this.findPerson(marker.SubjUID);
let subject = 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) { return subjectPromise
subject = new Subject(subject); .then((subject) => {
} else { if (!subject) {
subject = await this.loadSubject(marker.SubjUID); return null;
} }
return subject.setCover(marker.Thumb);
if (!subject) { })
this.$notify.error(this.$gettext("Person not found")); .then((updated) => {
return; this.updatePersonList(updated);
} this.$notify.success(this.$gettext("Person cover updated"));
})
const updated = await subject.setCover(marker.Thumb); .catch((err) => {
this.updatePersonList(updated); if (err) {
this.$notify.success(this.$gettext("Person cover updated")); console.error("faces: failed setting person cover", err);
} catch (err) { this.$notify.error(this.$gettext("Could not update person cover"));
console.error("faces: failed setting person cover", err); }
this.$notify.error(this.$gettext("Could not update person cover")); })
} finally { .finally(() => {
this.$notify.unblockUI(); this.$notify.unblockUI();
this.busy = false; this.busy = false;
} });
}, },
onApprove(model) { onApprove(model) {
if (this.busy || !model) return; if (this.busy || !model) return;

View File

@@ -58,36 +58,47 @@ export class Batch extends Model {
} }
save(selection, values) { save(selection, values) {
return $api return $api.post("batch/photos/edit", { photos: selection, values }).then((response) => {
.post("batch/photos/edit", { photos: selection, values: values }) if (response?.data?.models?.length) {
.then((response) => { const updatedMap = new Map(
if (response.data.values) { response.data.models.map((raw) => {
this.values = response.data.values; const photo = new Photo();
} photo.setValues(raw);
return Promise.resolve(this); return [photo.UID, photo];
}) })
.catch((error) => { );
throw error;
});
}
async getData(selection) { this.models = this.models.map((existing) => {
const response = await $api.post("batch/photos/edit", { photos: selection }); const updated = updatedMap.get(existing.UID);
const models = response.data.models || []; if (updated) {
existing.setValues(updated);
updatedMap.delete(existing.UID);
}
return existing;
});
this.models = models.map((m) => { updatedMap.forEach((photo) => {
const modelInstance = new Photo(); this.models.push(photo);
return modelInstance.setValues(m); });
}
if (response?.data?.values) {
this.values = response.data.values;
}
return this;
}); });
this.values = response.data.values;
this.setSelections(selection);
} }
async getValuesForSelection(selection) { // load fetches the current selection (+ aggregated form values) and hydrates Photo instances.
const response = await $api.post("batch/photos/edit", { photos: selection }); load(selection) {
this.values = response.data.values; return $api.post("batch/photos/edit", { photos: selection }).then((response) => {
return this.values; 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) { setSelections(selection) {

View File

@@ -39,15 +39,7 @@ function splitSegments(message) {
} }
// All log levels ordered by severity. // All log levels ordered by severity.
export const AuditSeverityNames = Object.freeze([ export const AuditSeverityNames = Object.freeze(["panic", "fatal", "error", "warning", "info", "debug", "trace"]);
"panic",
"fatal",
"error",
"warning",
"info",
"debug",
"trace",
]);
// Audit logs currently only exist with the following levels: // Audit logs currently only exist with the following levels:
// error, warning, info, and debug. // error, warning, info, and debug.

View File

@@ -116,9 +116,8 @@ describe("component/photo/edit/batch", () => {
{ id: "uid2", selected: true }, { id: "uid2", selected: true },
{ id: "uid3", selected: true }, { id: "uid3", selected: true },
], ],
getData: vi.fn(), load: vi.fn(),
save: vi.fn(), save: vi.fn(),
getValuesForSelection: vi.fn(),
getDefaultFormData: vi.fn(), getDefaultFormData: vi.fn(),
getLengthOfAllSelected: vi.fn(), getLengthOfAllSelected: vi.fn(),
isSelected: vi.fn(), isSelected: vi.fn(),
@@ -127,9 +126,8 @@ describe("component/photo/edit/batch", () => {
}; };
// Configure mock method behaviors // Configure mock method behaviors
mockBatchInstance.getData.mockResolvedValue(mockBatchInstance); mockBatchInstance.load.mockResolvedValue(mockBatchInstance);
mockBatchInstance.save.mockResolvedValue(mockBatchInstance); mockBatchInstance.save.mockResolvedValue(mockBatchInstance);
mockBatchInstance.getValuesForSelection.mockResolvedValue(mockValues);
mockBatchInstance.getDefaultFormData.mockReturnValue(mockDefaultFormData); mockBatchInstance.getDefaultFormData.mockReturnValue(mockDefaultFormData);
mockBatchInstance.getLengthOfAllSelected.mockReturnValue(3); mockBatchInstance.getLengthOfAllSelected.mockReturnValue(3);
mockBatchInstance.isSelected.mockReturnValue(true); mockBatchInstance.isSelected.mockReturnValue(true);
@@ -447,7 +445,7 @@ describe("component/photo/edit/batch", () => {
await wrapper.setProps({ visible: true }); await wrapper.setProps({ visible: true });
await nextTick(); await nextTick();
await nextTick(); await nextTick();
expect(mockBatchInstance.getData).toHaveBeenCalledWith(mockSelection); expect(mockBatchInstance.load).toHaveBeenCalledWith(mockSelection);
}); });
it("should emit close event", () => { it("should emit close event", () => {

View File

@@ -1,6 +1,7 @@
import { describe, it, expect } from "vitest"; import { describe, it, expect } from "vitest";
import "../fixtures"; import "../fixtures";
import { Batch } from "model/batch-edit"; import { Batch } from "model/batch-edit";
import { Photo } from "model/photo";
describe("model/batch-edit", () => { describe("model/batch-edit", () => {
it("should return defaults", () => { it("should return defaults", () => {
@@ -88,23 +89,29 @@ describe("model/batch-edit", () => {
it("should call save and update values from response", async () => { it("should call save and update values from response", async () => {
const b = new Batch(); const b = new Batch();
const selection = [5, 7]; const selection = ["pt20fg34bbwdm2ld", "pt20fg2qikiy7zax"];
const values = { Title: { value: "New" } }; 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"); const { Mock } = await import("../fixtures");
Mock.onPost("api/v1/batch/photos/edit", { photos: selection, values }).reply( Mock.onPost("api/v1/batch/photos/edit", { photos: selection, values }).reply(200, {
200, models: [
{ values: { Title: { value: "Saved" } } }, { UID: "pt20fg34bbwdm2ld", Title: "Updated" },
{ "Content-Type": "application/json; charset=utf-8" } { UID: "pt20fg2qikiy7zb0", Title: "New" },
); ],
values: { Title: { value: "Saved" } },
});
const result = await b.save(selection, values); const result = await b.save(selection, values);
expect(result).toBe(b); expect(result).toBe(b);
expect(b.values).toEqual({ Title: { value: "Saved" } }); 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 b = new Batch();
const selection = [101, 102]; const selection = [101, 102];
@@ -122,10 +129,13 @@ describe("model/batch-edit", () => {
{ "Content-Type": "application/json; charset=utf-8" } { "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(Array.isArray(b.models)).toBe(true);
expect(b.models.length).toBe(2); 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.values).toEqual({ Title: { mixed: true } });
expect(b.selection).toEqual([ expect(b.selection).toEqual([
{ id: 101, selected: true }, { id: 101, selected: true },