Brewtarget / brewtarget

Main brewtarget source code repository.
GNU General Public License v3.0
312 stars 134 forks source link

Custom instructions aren't added to Recipe #656

Closed dsheets4 closed 1 year ago

dsheets4 commented 2 years ago

Definitely like the recent updates to BrewTarget! I've been using the software for many years and always enjoy it. The recipe format gives most of the required information to brew, from measuring ingredients to the timing in which to add them. For fields that aren't covered, I use the custom Brewday instructions. For example, adding recipe attribution, grain conditioning calculations, fermentation schedules, expected first and final mash running gravity, etc. Using Instructions results in individual rows in the recipe output, making that more appealing than the Notes on the Extras tab, which are compressed to a single block of text in the recipe output.

In the current develop branch (f3b8ba996d77768372838c3103528b7161b456a2) of BrewTarget, creating custom instructions doesn't add them to the recipe. I type a name in the Name field and have tried both adding the Step # and leaving that blank. After hitting the + button, the step does not appear in the list.

Below are some notes from investigating.

matty0ung commented 2 years ago

Glad you're enjoying the software! Thanks for the detailed bug report. I think this will almost certainly be related to the new database access layer. Will hopefully be a small-ish fix. I'll try to have a look this week and aim to get something in the 2.4.0 release.

matty0ung commented 2 years ago

OK, I've done the easy bit and got the instruction being inserted. Now I have to fix why it's being inserted in the wrong order and not showing up on the screen until after you restart the program. Might be the weekend before I get enough time to work on this properly.

matty0ung commented 2 years ago

I've coded the rest of the fix and bundled it in with the 2.4.0 release work. So, hopefully, should be appearing in the main code soon.

dsheets4 commented 2 years ago

Looking forward to officially pulling the update / new release! Thank you.

I checked out https://github.com/matty0ung/brewtarget/tree/release2.4. The steps appear in the Brewday tab right away now when hitting +. Appears to work well with adding steps and re-arranging them. Thank you for the quick turn-around in getting a fix in the queue!

dsheets4 commented 2 years ago

Perhaps only for posterity, as I'm not sure what else to do with this. There was one interesting behavior, which I expect would be a different ticket anyway but also could just be specific to my database. I did what amounts to copy/pasting steps from one recipe to another to test. That is, add an instruction to one recipe, click to a different recipe, open that recipe's Brewday tab, copy text from an instruction there, then return to the original recipe's Brewday tab, select the earlier created instruction, paste that text into that instruction, then click back into the instruction list, the display then flips to Recipe tab. I'm not sure all those steps are necessary... When it flips to the Recipe tab there are WARNING messages printed to the log that seem to indicate perhaps something is off with my database (see below). I did the same steps with a fresh BrewTarget database using the Bt: recipes and that works just fine. I'm not sure what the warning means. For example, fermentable 858 only appears in the fermentable_in_recipe table once for recipe 104. Recipe 104 is the one to which I was adding instructions. I also seem to have 6 rows in the database for that recipe, all but 104 flagged as locked. This might be an artifact of moving the recipe between folders.

[00:29:58.271] (1dvxtqb7ls) WARNING : bool {anonymous}::isUnusedInstanceOfUseOf(NE&) [with NE = Fermentable] Fermentable # 858 is unexpectedly already used in recipe # 104  [model/Recipe.cpp:94]
[00:29:58.272] (1dvxtqb7ls) WARNING : bool {anonymous}::isUnusedInstanceOfUseOf(NE&) [with NE = Fermentable] Fermentable # 859 is unexpectedly already used in recipe # 104  [model/Recipe.cpp:94]
[00:29:58.273] (1dvxtqb7ls) WARNING : bool {anonymous}::isUnusedInstanceOfUseOf(NE&) [with NE = Fermentable] Fermentable # 860 is unexpectedly already used in recipe # 104  [model/Recipe.cpp:94]
[00:29:58.273] (1dvxtqb7ls) WARNING : bool {anonymous}::isUnusedInstanceOfUseOf(NE&) [with NE = Fermentable] Fermentable # 861 is unexpectedly already used in recipe # 104  [model/Recipe.cpp:94]
[00:29:58.273] (1dvxtqb7ls) WARNING : bool {anonymous}::isUnusedInstanceOfUseOf(NE&) [with NE = Fermentable] Fermentable # 863 is unexpectedly already used in recipe # 104  [model/Recipe.cpp:94]
[00:29:58.274] (1dvxtqb7ls) WARNING : bool {anonymous}::isUnusedInstanceOfUseOf(NE&) [with NE = Hop] Hop # 350 is unexpectedly already used in recipe # 104  [model/Recipe.cpp:94]
[00:29:58.275] (1dvxtqb7ls) WARNING : bool {anonymous}::isUnusedInstanceOfUseOf(NE&) [with NE = Misc] Misc # 402 is unexpectedly already used in recipe # 104  [model/Recipe.cpp:94]
[00:29:58.275] (1dvxtqb7ls) WARNING : bool {anonymous}::isUnusedInstanceOfUseOf(NE&) [with NE = Misc] Misc # 403 is unexpectedly already used in recipe # 104  [model/Recipe.cpp:94]
[00:29:58.276] (1dvxtqb7ls) WARNING : bool {anonymous}::isUnusedInstanceOfUseOf(NE&) [with NE = Misc] Misc # 411 is unexpectedly already used in recipe # 104  [model/Recipe.cpp:94]
[00:29:58.276] (1dvxtqb7ls) WARNING : bool {anonymous}::isUnusedInstanceOfUseOf(NE&) [with NE = Misc] Misc # 412 is unexpectedly already used in recipe # 104  [model/Recipe.cpp:94]
[00:29:58.276] (1dvxtqb7ls) WARNING : bool {anonymous}::isUnusedInstanceOfUseOf(NE&) [with NE = Misc] Misc # 424 is unexpectedly already used in recipe # 104  [model/Recipe.cpp:94]
[00:29:58.277] (1dvxtqb7ls) WARNING : bool {anonymous}::isUnusedInstanceOfUseOf(NE&) [with NE = Misc] Misc # 429 is unexpectedly already used in recipe # 104  [model/Recipe.cpp:94]
[00:29:58.277] (1dvxtqb7ls) WARNING : bool {anonymous}::isUnusedInstanceOfUseOf(NE&) [with NE = Misc] Misc # 435 is unexpectedly already used in recipe # 104  [model/Recipe.cpp:94]
[00:29:58.278] (1dvxtqb7ls) WARNING : bool {anonymous}::isUnusedInstanceOfUseOf(NE&) [with NE = Misc] Misc # 446 is unexpectedly already used in recipe # 104  [model/Recipe.cpp:94]
[00:29:58.278] (1dvxtqb7ls) WARNING : bool {anonymous}::isUnusedInstanceOfUseOf(NE&) [with NE = Yeast] Yeast # 276 is unexpectedly already used in recipe # 104  [model/Recipe.cpp:94]
[00:29:58.282] (1dvxtqb7ls) WARNING : QObject::connect: No such signal BrewNote::brewDateChanged(QDateTime) in /home/user/brewtarget.project/brewtarget/src/BtTreeModel.cpp:1411  [:0]
[00:29:58.285] (1dvxtqb7ls) WARNING : QObject::connect: No such signal BrewNote::brewDateChanged(QDateTime) in /home/user/brewtarget.project/brewtarget/src/BtTreeModel.cpp:1411  [:0]
matty0ung commented 1 year ago

Sorry for slow reply. I think the warnings are likely a result of the DB getting in a funny state before the bug was fixed. I'm going to close this ticket but please don't hesitate to open a new one if you see other problems.