Bild uebernehmen: sichtbare Rueckmeldung und Nachholen bei fehlender Kopie
Zwei Ursachen dafuer, dass der Knopf folgenlos wirkte. Sichtbar: Uebernehmen schreibt nur ins Formular, das Artikelbild wechselt erst beim Speichern - es gab aber nichts, woran man das erkannt haette. Die Spalte "Bisher" zeigt jetzt sofort das uebernommene Bild mit Kennzeichnung, der Knopf heisst danach "Uebernommen", und eine Meldung erklaert den naechsten Schritt. Tatsaechlich folgenlos war der haeufigste Fall: Stimmte die Bildadresse laengst und nur der Abruf war damals fehlgeschlagen, aenderte sich beim Speichern nichts - und die Bedingung fragte genau nach einer Aenderung. Sie richtet sich jetzt danach, woher die vorhandene Kopie stammt. Fehlt sie, wird geholt. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -251,19 +251,21 @@ def update_product(
|
|||||||
data.pop("date_precision", None) # Spalte ist NOT NULL
|
data.pop("date_precision", None) # Spalte ist NOT NULL
|
||||||
else:
|
else:
|
||||||
data["date_precision"] = data["date_precision"].value
|
data["date_precision"] = data["date_precision"].value
|
||||||
alte_bildadresse = product.image_url
|
|
||||||
for field, value in data.items():
|
for field, value in data.items():
|
||||||
setattr(product, field, value)
|
setattr(product, field, value)
|
||||||
# Gruppe oder Barcode koennen sich geaendert haben - Code nachziehen.
|
# Gruppe oder Barcode koennen sich geaendert haben - Code nachziehen.
|
||||||
sync_group_code(db, product)
|
sync_group_code(db, product)
|
||||||
if "image_url" in data and data["image_url"] != alte_bildadresse:
|
if "image_url" in data:
|
||||||
# Neue Adresse heisst neues Bild – die alte Kopie waere sonst dauerhaft
|
# Massgeblich ist, woher die vorhandene Kopie stammt - nicht, ob sich
|
||||||
# falsch, weil ``ensure`` nur nachlaedt, wenn gar keine Kopie da ist.
|
# die Adresse am Artikel geaendert hat. Sonst passierte genau dann
|
||||||
|
# nichts, wenn die Adresse schon stimmte, der Abruf damals aber
|
||||||
|
# fehlschlug: Der Artikel bliebe dauerhaft ohne Bild.
|
||||||
alt = db.get(ProductImage, product.id)
|
alt = db.get(ProductImage, product.id)
|
||||||
if alt is not None:
|
if alt is not None and alt.source_url != product.image_url:
|
||||||
db.delete(alt)
|
db.delete(alt)
|
||||||
db.flush()
|
db.flush()
|
||||||
if product.image_url:
|
alt = None
|
||||||
|
if alt is None and product.image_url:
|
||||||
images.store(db, product, product.image_url)
|
images.store(db, product, product.image_url)
|
||||||
db.commit()
|
db.commit()
|
||||||
db.refresh(product)
|
db.refresh(product)
|
||||||
|
|||||||
@@ -99,6 +99,28 @@ def test_import_ueberschreibt_kein_vorhandenes_bild(db, user):
|
|||||||
assert vorhanden.image_url == "https://eigenes.example/bild.jpg"
|
assert vorhanden.image_url == "https://eigenes.example/bild.jpg"
|
||||||
|
|
||||||
|
|
||||||
|
def test_speichern_holt_nach_wenn_die_kopie_fehlt(db, user, monkeypatch):
|
||||||
|
"""Der haeufigste Fall: Adresse stimmt laengst, der Abruf schlug nur einmal fehl.
|
||||||
|
|
||||||
|
Wuerde nur auf eine geaenderte Adresse geachtet, bliebe so ein Artikel
|
||||||
|
dauerhaft ohne Bild - "Uebernehmen" waere dann folgenlos.
|
||||||
|
"""
|
||||||
|
from app.routers.products import update_product
|
||||||
|
from app.schemas import ProductUpdate
|
||||||
|
|
||||||
|
monkeypatch.setattr(images, "fetch", lambda url: (EIN_PIXEL, "image/png"))
|
||||||
|
artikel = _artikel(db, "https://example.org/pesto.png")
|
||||||
|
assert db.get(ProductImage, artikel.id) is None
|
||||||
|
|
||||||
|
update_product(
|
||||||
|
artikel.id,
|
||||||
|
ProductUpdate(image_url="https://example.org/pesto.png"),
|
||||||
|
db=db,
|
||||||
|
_=user,
|
||||||
|
)
|
||||||
|
assert db.get(ProductImage, artikel.id) is not None
|
||||||
|
|
||||||
|
|
||||||
def test_fetch_geht_nur_an_http_adressen(monkeypatch):
|
def test_fetch_geht_nur_an_http_adressen(monkeypatch):
|
||||||
"""Ohne diese Schranke waere eine 'file://'-Adresse ein Weg ins Dateisystem."""
|
"""Ohne diese Schranke waere eine 'file://'-Adresse ein Weg ins Dateisystem."""
|
||||||
monkeypatch.setattr(
|
monkeypatch.setattr(
|
||||||
|
|||||||
@@ -11,7 +11,8 @@ import ProduktBild from "./ProduktBild";
|
|||||||
* felder: [{ schluessel, titel, eigen, fremd, anzeigeEigen?, anzeigeFremd? }]
|
* felder: [{ schluessel, titel, eigen, fremd, anzeigeEigen?, anzeigeFremd? }]
|
||||||
*/
|
*/
|
||||||
export default function OffVergleich({
|
export default function OffVergleich({
|
||||||
felder, bildEigenId, bildFremdUrl, onUebernehmen, onAlle, onSchliessen,
|
felder, bildEigenId, bildFremdUrl, bildUebernommen,
|
||||||
|
onUebernehmen, onAlle, onSchliessen,
|
||||||
}) {
|
}) {
|
||||||
const leer = (wert) => wert === null || wert === undefined || wert === "";
|
const leer = (wert) => wert === null || wert === undefined || wert === "";
|
||||||
const abweichend = felder.filter((f) => !leer(f.fremd) && String(f.fremd) !== String(f.eigen));
|
const abweichend = felder.filter((f) => !leer(f.fremd) && String(f.fremd) !== String(f.eigen));
|
||||||
@@ -62,7 +63,19 @@ export default function OffVergleich({
|
|||||||
|
|
||||||
<tr className={bildAbweichend ? "" : "muted"}>
|
<tr className={bildAbweichend ? "" : "muted"}>
|
||||||
<td data-label="Feld">Bild</td>
|
<td data-label="Feld">Bild</td>
|
||||||
<td data-label="Bisher"><ProduktBild productId={bildEigenId} className="off-bild" /></td>
|
<td data-label="Bisher">
|
||||||
|
{/* Nach dem Übernehmen steht hier schon das neue Bild. Ohne das
|
||||||
|
sah der Knopf wirkungslos aus: Das Artikelbild wechselt erst
|
||||||
|
beim Speichern, weil der Server es dann erst holt. */}
|
||||||
|
{bildUebernommen
|
||||||
|
? (
|
||||||
|
<span className="off-neu">
|
||||||
|
<img className="off-bild" src={bildFremdUrl} alt="" />
|
||||||
|
<span className="badge">neu</span>
|
||||||
|
</span>
|
||||||
|
)
|
||||||
|
: <ProduktBild productId={bildEigenId} className="off-bild" />}
|
||||||
|
</td>
|
||||||
<td data-label="Open Food Facts">
|
<td data-label="Open Food Facts">
|
||||||
{/* Vorschau kommt hier direkt von OFF – lokal gibt es das Bild
|
{/* Vorschau kommt hier direkt von OFF – lokal gibt es das Bild
|
||||||
ja gerade noch nicht. Übernommen und gespeichert, holt der
|
ja gerade noch nicht. Übernommen und gespeichert, holt der
|
||||||
@@ -74,7 +87,7 @@ export default function OffVergleich({
|
|||||||
<td className="num">
|
<td className="num">
|
||||||
<button type="button" className="btn sm" disabled={!bildAbweichend}
|
<button type="button" className="btn sm" disabled={!bildAbweichend}
|
||||||
onClick={() => onUebernehmen("image_url")}>
|
onClick={() => onUebernehmen("image_url")}>
|
||||||
Übernehmen
|
{bildUebernommen ? "Übernommen" : "Übernehmen"}
|
||||||
</button>
|
</button>
|
||||||
</td>
|
</td>
|
||||||
</tr>
|
</tr>
|
||||||
|
|||||||
@@ -176,6 +176,9 @@ export default function ProductForm() {
|
|||||||
if (!offDaten) return;
|
if (!offDaten) return;
|
||||||
if (schluessel === "image_url") {
|
if (schluessel === "image_url") {
|
||||||
set("image_url", offDaten.image_url || "");
|
set("image_url", offDaten.image_url || "");
|
||||||
|
// Das Bild selbst wechselt erst beim Speichern - ohne Hinweis wirkt der
|
||||||
|
// Knopf folgenlos.
|
||||||
|
toast("Bild übernommen. Mit „Speichern“ wird es geholt.");
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
if (schluessel === "package_size") {
|
if (schluessel === "package_size") {
|
||||||
@@ -384,6 +387,7 @@ export default function ProductForm() {
|
|||||||
felder={offFelder()}
|
felder={offFelder()}
|
||||||
bildEigenId={id}
|
bildEigenId={id}
|
||||||
bildFremdUrl={offDaten.image_url || ""}
|
bildFremdUrl={offDaten.image_url || ""}
|
||||||
|
bildUebernommen={Boolean(offDaten.image_url) && form.image_url === offDaten.image_url}
|
||||||
onUebernehmen={uebernehmen}
|
onUebernehmen={uebernehmen}
|
||||||
onAlle={alleUebernehmen}
|
onAlle={alleUebernehmen}
|
||||||
onSchliessen={() => setOffDaten(null)}
|
onSchliessen={() => setOffDaten(null)}
|
||||||
|
|||||||
@@ -230,6 +230,7 @@ input::placeholder { color: var(--muted); opacity: 0.7; }
|
|||||||
width: 56px; aspect-ratio: 3 / 4; object-fit: contain;
|
width: 56px; aspect-ratio: 3 / 4; object-fit: contain;
|
||||||
background: var(--surface-2); border-radius: 4px; display: block;
|
background: var(--surface-2); border-radius: 4px; display: block;
|
||||||
}
|
}
|
||||||
|
.off-neu { display: flex; align-items: center; gap: var(--sp-2); }
|
||||||
.field-inline { display: flex; gap: var(--sp-2); align-items: flex-end; margin-bottom: var(--sp-4); }
|
.field-inline { display: flex; gap: var(--sp-2); align-items: flex-end; margin-bottom: var(--sp-4); }
|
||||||
/* Labels in einer Inline-Reihe duerfen keinen eigenen Abstand haben, sonst
|
/* Labels in einer Inline-Reihe duerfen keinen eigenen Abstand haben, sonst
|
||||||
rutschen daneben stehende Buttons optisch nach unten. */
|
rutschen daneben stehende Buttons optisch nach unten. */
|
||||||
|
|||||||
Reference in New Issue
Block a user