Imported recipes are inconsistent and inaccurate

[mrmekon]

Brewtarget has inconsistent behavior when importing a recipe that it previously exported. I've observed the mash methods, addition timings, and yeast measurement unit changing on import.

I tested with Brewtarget version 3.0.4 (Arch linux, x86_64, installed from the AUR package).

I created a recipe in Brewtarget (a previous version), exported it to an XML file (with the current version). The original recipe has two sugar additions that are late boil additions.

I then tried three separate things: 1) Import back into the same Brewtarget install, cloning the recipe. 2) Import into a fresh Brewtarget install, as the only copy. 3) Manually change the names of all of the fermentables/hops/yeast (so they aren't in the database), then import into the same fresh Brewtarget install.

Amusingly, all three imports are incorrect, but in different ways!

When importing back into the same install, some (but not all) of the darker malts switch from "mashed" to "steeped". Boil SG drops a bit, accordingly.

When importing into a fresh install, even more of the malts switched to "steeped": Both late additions get marked as normal instead, and boil SG skyrockets accordingly.

And when importing a recipe with all fermentable names changed, all of them are marked as mashed and late additions, and boil SG drops to 1.000 because there are no grains left in the mash...

In all three cases, the yeast switched from being measured in volume (125 mL) to mass (125 g).

The XML files I used for testing are attached (with .txt extensions to make github happy).

boohoo_export.txt

boohoo_modified.txt

And, finally, thanks for all of your work on this great application!

[matty0ung]

Thanks for the detailed report. I've started having a look, initially at why the yeast weight ends up as mass. Seems like we correctly read the <AMOUNT_IS_WEIGHT>FALSE</AMOUNT_IS_WEIGHT> line out of the XML file, but the wrong value is ending up in the DB. Will investigate further...

[matty0ung]

OK, I think I found the culprit: https://github.com/Brewtarget/brewtarget/blob/da0c5786d782606b0577c728ec9511cc3e3a5ed7/src/xml/XmlRecord.cpp#L199 Should say parsedValue.setValue(false); instead of parsedValue.setValue(true);

Bit of an embarrassing copy-and-paste typo on my part. Will do a patch in the next day or so hopefully.

[matty0ung]

I think we also need a couple of extra lines here: https://github.com/Brewtarget/brewtarget/blob/da0c5786d782606b0577c728ec9511cc3e3a5ed7/src/xml/XmlRecipeRecord.cpp#L48

Should additionally say:

   fermentable.setAddAfterBoil(npb(PropertyNames::Fermentable::addAfterBoil).toBool());
   fermentable.setIsMashed(    npb(PropertyNames::Fermentable::isMashed).toBool());

This one is a bit more tedious to explain. It's to do with how we handle the difference between "a variety of hop/fermentable/yeast/etc" and "a use of this variety of hop/fermentable/yeast/etc in a particular recipe". There are long comments in the code about it. The way we do things has various pros and cons, but it can be confusing and we could do a bit more with naming conventions etc in the code to signpost things.

TLDR: I now have it that when you export the recipe after importing it, you get "the same" XML. By "the same" I mean ignoring some rounding tiny errors in long decimal numbers and a slight name tweak of one ingredient. The former at least I think is expected. I'll double-check the latter.

Working on a patch...

[matty0ung]

Closing now that https://github.com/Brewtarget/brewtarget/releases/tag/v3.0.5 is available.