fix(import): tolerate common file variations and say where a file breaks (#19)

CHANGELOG.md

@@ -37,6 +37,17 @@ follow [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
   for any figure the recipe file didn't state (e.g. ABV on an imported BeerXML
   without an estimate) and no color swatch. It now falls back to Werb's
   estimate from the ingredients, as the recipe screen already did.
+- **BeerXML imports that failed on one odd value now go through**
+  ([#19](https://github.com/werb-dev/werb/issues/19)). Decimal commas
+  (`5,5`), values in another case (`boil`, `DRY HOP`) and common synonyms
+  (`Whirlpool`, `Hop stand` → Aroma; `Malt` → Grain; `Whole` → Leaf) are
+  read; an unknown value falls back to the default instead of failing the
+  whole file; a missing hop alpha or amount reads as 0, like an empty one.
+  BeerJSON files that write `null` for optional properties import too.
+- **Import errors say where the file breaks** (#19). BeerXML errors give
+  the element, e.g. `/RECIPES/RECIPE[1]/HOPS/HOP[2]/AMOUNT: not a number:
+  "lots"`; BeerJSON errors name the missing or unknown property, or the
+  value and the allowed ones. Same messages for GitHub Pull.
 - **Default water on the web app.** New recipes now start from your water on
   the web too; they used to start from zero because the saved default loaded
   after the recipe had already picked its water. A recipe you haven't touched

apps/desktop/src/data/recipes.ts

@@ -1,5 +1,5 @@
 import type { BeerJsonFile, BeerJsonRecipe } from "@werb/adapters";
-import { validateBeerJson } from "@werb/validate";
+import { describeErrors, stripNulls, validateBeerJson } from "@werb/validate";
 import type { StorageBackend } from "../storage/index.ts";
 import { isTauri } from "./runtime.ts";
 import { WerbError } from "./errors.ts";
@@ -86,7 +86,7 @@ const bundledRaw = import.meta.glob("../../../../examples/*.beerjson", {
 function parseAndCollect(path: string, raw: string): BeerJsonRecipe[] {
   let parsed: unknown;
   try {
-    parsed = JSON.parse(raw);
+    parsed = stripNulls(JSON.parse(raw));
   } catch (err) {
     console.warn(`[recipes] ${path}: invalid JSON — ${(err as Error).message}`);
     return [];
@@ -150,7 +150,7 @@ export interface ImportResult {
 export function parseBeerJsonText(raw: string): ImportResult {
   let parsed: unknown;
   try {
-    parsed = JSON.parse(raw);
+    parsed = stripNulls(JSON.parse(raw));
   } catch (err) {
     return {
       recipes: [],
@@ -159,8 +159,7 @@ export function parseBeerJsonText(raw: string): ImportResult {
   }
   const result = validateBeerJson(parsed);
   if (!result.valid) {
-    const first = result.errors[0];
-    const detail = first ? `${first.path}: ${first.message}` : "unknown";
+    const detail = describeErrors(parsed, result.errors);
     return { recipes: [], error: new WerbError("import.not_beerjson", { detail }) };
   }
   const recipes = (parsed as BeerJsonFile).beerjson?.recipes ?? [];

apps/desktop/src/screens/Library.tsx

@@ -148,7 +148,9 @@ export function LibraryScreen({
               <p className="text-caption uppercase tracking-widest text-warning font-medium">
                 {t("library.import_failed")}
               </p>
-              <p className="text-body-sm text-text mt-2 font-mono break-all">{importError}</p>
+              <p data-testid="import-error" className="text-body-sm text-text mt-2 font-mono break-all">
+                {importError}
+              </p>
             </div>
           )}
 

apps/desktop/src/storage/github-recipes.ts

@@ -13,7 +13,7 @@
  */
 
 import type { BeerJsonRecipe, BeerJsonFile } from "@werb/adapters";
-import { validateBeerJson } from "@werb/validate";
+import { describeErrors, stripNulls, validateBeerJson } from "@werb/validate";
 import type { GitHubBackendConfig } from "./github.ts";
 import { slugify } from "../data/recipe-export.ts";
 import {
@@ -249,18 +249,14 @@ export async function pullRecipes(
     if (!entry) continue;
     let parsed: BeerJsonFile;
     try {
-      parsed = JSON.parse(entry.raw) as BeerJsonFile;
+      parsed = stripNulls(JSON.parse(entry.raw)) as BeerJsonFile;
     } catch (err) {
       result.failed.push({ file: entry.file, error: `JSON parse: ${(err as Error).message}` });
       continue;
     }
     const validation = validateBeerJson(parsed);
     if (!validation.valid) {
-      const first = validation.errors[0];
-      result.failed.push({
-        file: entry.file,
-        error: first ? `${first.path}: ${first.message}` : "schema invalid",
-      });
+      result.failed.push({ file: entry.file, error: describeErrors(parsed, validation.errors) });
       continue;
     }
     const incoming = parsed.beerjson?.recipes ?? [];

apps/desktop/test/beerjson-roundtrip.test.ts

@@ -108,6 +108,25 @@ describe("BeerJSON round-trip", () => {
     expect(result.error?.code).toBe("import.not_beerjson");
   });
 
+  it("imports a file that writes null for optional properties", () => {
+    const withNulls = JSON.parse(FIXTURE_RAW);
+    withNulls.beerjson.recipes[0].notes = null;
+    withNulls.beerjson.recipes[0].style = null;
+    const result = parseBeerJsonText(JSON.stringify(withNulls));
+    expect(result.error).toBeUndefined();
+    expect(result.recipes[0]).not.toHaveProperty("notes");
+  });
+
+  it("names the property that breaks validation", () => {
+    const broken = JSON.parse(FIXTURE_RAW);
+    delete broken.beerjson.recipes[0].batch_size;
+    const result = parseBeerJsonText(JSON.stringify(broken));
+    expect(result.error?.code).toBe("import.not_beerjson");
+    expect(result.error?.params?.detail).toBe(
+      '/beerjson/recipes/0: missing required property "batch_size"',
+    );
+  });
+
   it("rejects valid BeerJSON with no recipes inside", () => {
     const empty = JSON.stringify({ beerjson: { version: 2.06, recipes: [] } });
     const result = parseBeerJsonText(empty);

apps/desktop/test/beerxml-to-beerjson.test.ts

@@ -170,3 +170,36 @@ describe("BeerXML → BeerJSON validates against the schema", () => {
     expect(result.valid, JSON.stringify(result.errors)).toBe(true);
   });
 });
+
+describe("BeerXML import tolerates common variations (#19)", () => {
+  const wrap = (body: string) =>
+    `<?xml version="1.0"?><RECIPES><RECIPE><NAME>Variations</NAME><VERSION>1</VERSION>` +
+    `<TYPE>all grain</TYPE><BATCH_SIZE>20</BATCH_SIZE><BOIL_TIME>60</BOIL_TIME>${body}</RECIPE></RECIPES>`;
+
+  it("imports decimal commas, lowercase values and common synonyms", () => {
+    const recipe = importXml(
+      wrap(
+        "<FERMENTABLES><FERMENTABLE><NAME>Pale</NAME><TYPE>Malt</TYPE><AMOUNT>4,5</AMOUNT>" +
+          "<YIELD>80</YIELD><COLOR>6</COLOR></FERMENTABLE></FERMENTABLES>" +
+          "<HOPS><HOP><NAME>Cascade</NAME><ALPHA>5,5</ALPHA><AMOUNT>0,03</AMOUNT>" +
+          "<USE>whirlpool</USE><TIME>20</TIME><FORM>pellets</FORM></HOP></HOPS>",
+      ),
+    );
+    expect(validateBeerJson(asDocument(recipe)).valid).toBe(true);
+    expect(recipe.ingredients.fermentable_additions[0]!.amount.value).toBeCloseTo(4.5, 6);
+    expect(recipe.ingredients.fermentable_additions[0]!.type).toBe("grain");
+    expect(recipe.ingredients.hop_additions![0]!.alpha_acid.value).toBeCloseTo(5.5, 6);
+    expect(recipe.ingredients.hop_additions![0]!.form).toBe("pellet");
+  });
+
+  it("says where a file breaks", () => {
+    const xml = wrap(
+      "<HOPS><HOP><NAME>A</NAME><ALPHA>5</ALPHA><AMOUNT>0.01</AMOUNT></HOP>" +
+        "<HOP><NAME>B</NAME><ALPHA>5</ALPHA><AMOUNT>lots</AMOUNT></HOP></HOPS>",
+    );
+    expect(() => wasm.parseBeerXmlJson(xml)).toThrow(
+      '/RECIPES/RECIPE[1]/HOPS/HOP[2]/AMOUNT: not a number: "lots"',
+    );
+  });
+});
+

apps/desktop/test/e2e/pages.ts

@@ -131,6 +131,13 @@ class LibraryPage {
     }
   }
 
+  /** Import a recipe file through the Import button's file picker. */
+  async importFile(name: string, content: string) {
+    const chooser = this.page.waitForEvent("filechooser");
+    await this.page.getByRole("button", { name: /^Import recipes/i }).click();
+    await (await chooser).setFiles({ name, mimeType: "text/plain", buffer: Buffer.from(content) });
+  }
+
   /** Create a new empty recipe and save it. Lands on the recipe view. */
   async newEmptyRecipe(name: string) {
     await this.page.getByRole("button", { name: /\+\s*New recipe/i }).click();

apps/desktop/test/e2e/smoke.spec.ts

@@ -355,6 +355,61 @@ test.describe("Library cards", () => {
   });
 });
 
+test.describe("Import (#19)", () => {
+  const beerXml = (hops: string) =>
+    `<?xml version="1.0"?><RECIPES><RECIPE><NAME>Comma Pale</NAME><VERSION>1</VERSION>` +
+    `<TYPE>All Grain</TYPE><BATCH_SIZE>20</BATCH_SIZE><BOIL_TIME>60</BOIL_TIME>` +
+    `<FERMENTABLES><FERMENTABLE><NAME>Pale</NAME><TYPE>Malt</TYPE><AMOUNT>4,5</AMOUNT>` +
+    `<YIELD>80</YIELD><COLOR>6</COLOR></FERMENTABLE></FERMENTABLES><HOPS>${hops}</HOPS></RECIPE></RECIPES>`;
+
+  test("a BeerXML with decimal commas and loose values imports", async ({ page }) => {
+    const app = new App(page);
+    await app.go("Library");
+    await app.library.importFile(
+      "comma.xml",
+      beerXml("<HOP><NAME>Cascade</NAME><ALPHA>5,5</ALPHA><AMOUNT>0,03</AMOUNT><USE>whirlpool</USE><TIME>20</TIME></HOP>"),
+    );
+    await expect(page.locator("li h2", { hasText: /comma pale/i })).toBeVisible();
+    await expect(page.getByTestId("import-error")).toHaveCount(0);
+  });
+
+  test("a broken BeerXML says where it breaks", async ({ page }) => {
+    const app = new App(page);
+    await app.go("Library");
+    await app.library.importFile(
+      "broken.xml",
+      beerXml(
+        "<HOP><NAME>A</NAME><ALPHA>5</ALPHA><AMOUNT>0.01</AMOUNT></HOP>" +
+          "<HOP><NAME>B</NAME><ALPHA>5</ALPHA><AMOUNT>lots</AMOUNT></HOP>",
+      ),
+    );
+    await expect(page.getByTestId("import-error")).toContainText(
+      '/RECIPES/RECIPE[1]/HOPS/HOP[2]/AMOUNT: not a number: "lots"',
+    );
+  });
+
+  test("a BeerJSON error names the property", async ({ page }) => {
+    const app = new App(page);
+    await app.go("Library");
+    const doc = {
+      beerjson: {
+        version: 2.06,
+        recipes: [
+          {
+            name: "No batch",
+            type: "all grain",
+            author: "x",
+            efficiency: { brewhouse: { value: 72, unit: "%" } },
+            ingredients: { fermentable_additions: [] },
+          },
+        ],
+      },
+    };
+    await app.library.importFile("bad.beerjson", JSON.stringify(doc));
+    await expect(page.getByTestId("import-error")).toContainText('missing required property "batch_size"');
+  });
+});
+
 test.describe("Discoverability", () => {
   test("Library onboarding mentions local storage + GitHub sync", async ({ page }) => {
     const app = new App(page);

crates/werb-beerxml/Cargo.toml

@@ -13,6 +13,7 @@ readme = "README.md"
 quick-xml = { version = "0.36", features = ["serde", "serialize"] }
 serde = { version = "1", features = ["derive"] }
 serde_json = "1"
+serde_path_to_error = "0.1"
 thiserror = "1"
 werb-beerjson = { path = "../werb-beerjson" }
 

crates/werb-beerxml/src/error.rs

@@ -7,10 +7,20 @@ use thiserror::Error;
 #[derive(Debug, Error)]
 pub enum Error {
     /// The input is not well-formed XML, or does not match the expected
-    /// BeerXML structure (e.g. missing `<RECIPES>` root).
+    /// BeerXML structure, at the document root (e.g. missing `<RECIPES>`).
     #[error("BeerXML parse error: {0}")]
     Xml(#[from] quick_xml::DeError),
 
+    /// An element's content does not fit BeerXML, at `location`: an XPath
+    /// with 1-based indices, e.g. `/RECIPES/RECIPE[1]/HOPS/HOP[3]/AMOUNT`.
+    #[error("{location}: {source}")]
+    Invalid {
+        /// Where in the document the problem is.
+        location: String,
+        /// What is wrong there.
+        source: quick_xml::DeError,
+    },
+
     /// A field that should be a number could not be parsed as one. The
     /// inner string is the offending raw text from the XML.
     #[error("invalid number in field {field}: {raw:?}")]

crates/werb-beerxml/src/joliebulle.rs

@@ -36,7 +36,8 @@ use crate::model::{
 /// the same Option semantics as the BeerXML parser.
 pub fn parse_joliebulle(json: &str) -> Result<Vec<Recipe>, Error> {
     let doc: JoliebulleDocument =
-        serde_json::from_str(json).map_err(|e| Error::Json(e.to_string()))?;
+        serde_path_to_error::deserialize(&mut serde_json::Deserializer::from_str(json))
+            .map_err(|e| Error::Json(format!("{}: {}", e.path(), e.inner())))?;
     Ok(doc.recipes.into_iter().map(Recipe::from).collect())
 }