Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Text Notes on Brew Days gone and do not work anymore #457

Closed
cgspeck opened this issue Apr 8, 2020 · 2 comments · Fixed by #458
Closed

Text Notes on Brew Days gone and do not work anymore #457

cgspeck opened this issue Apr 8, 2020 · 2 comments · Fixed by #458

Comments

@cgspeck
Copy link
Contributor

cgspeck commented Apr 8, 2020

Before #453 I was able to store notes against each brew day.

After upgrading all the notes have gone blank. If I attempt to save a note, then an error is thrown and the application crashes:
image

[17:22:55.327] ERROR : Could not translate notes to a column name
Qt has caught an exception thrown from an event handler. Throwing
exceptions from an event handler is not supported in Qt.
You must not let any exception whatsoever propagate through Qt code.
If that is not possible, in Qt 5 you must at least reimplement
QCoreApplication::notify() and catch all exceptions there.

Steps to reproduce:

  1. click on a recipe
  2. click "Brew It" or select an existing brew day
  3. click on the relevant date tab, then enter any text in Notes box
  4. click away from the notes box

The schema of my brewnotes table is this:

CREATE TABLE "brewnote" 
(
    id INTEGER PRIMARY KEY autoincrement ,
    abv real DEFAULT 0,
    attenuation real DEFAULT 1,
    boil_off real DEFAULT 1,
    brewdate timestamp DEFAULT CURRENT_TIMESTAMP,
    brewhouse_eff real DEFAULT 70,
    deleted boolean DEFAULT 0,
    display boolean DEFAULT 1,
    eff_into_bk real DEFAULT 70,
    fermentdate timestamp DEFAULT CURRENT_TIMESTAMP,
    fg real DEFAULT 1,
    final_volume real DEFAULT 1,
    folder text DEFAULT '',
    mash_final_temp real DEFAULT 67,
    og real DEFAULT 1,
    pitch_temp real DEFAULT 20,
    post_boil_volume real DEFAULT 0,
    projected_abv real DEFAULT 1,
    projected_atten real DEFAULT 75,
    projected_boil_grav real DEFAULT 1,
    projected_eff real DEFAULT 1,
    projected_ferm_points real DEFAULT 1,
    projected_fg real DEFAULT 1,
    projected_mash_fin_temp real DEFAULT 67,
    projected_og real DEFAULT 1,
    projected_points real DEFAULT 1,
    projected_strike_temp real DEFAULT 70,
    projected_vol_into_bk real DEFAULT 1,
    projected_vol_into_ferm real DEFAULT 0,
    sg real DEFAULT 1,
    strike_temp real DEFAULT 70,
    volume_into_bk real DEFAULT 0,
    volume_into_fermenter real DEFAULT 0,
    recipe_id integer,
    FOREIGN KEY(recipe_id) REFERENCES recipe(id)
)

If I add a TEXT field called notes to the brewnote table, then the contents of this field will show up in the Notes box, but if I attempt to add any of it or edit it, then the error above happens and Brewtarget crashes.

Notes on recipes still appear to work.

@mikfire
Copy link
Contributor

mikfire commented Apr 8, 2020

Ugh. This is a bad combo.

The root cause here is that I did not include the notes column in the TableProperties for brewnotes. As part of the upgrade, I dropped two columns from brewnotes that were not used.

On SQLite, dropping a column requires making a temporary table, copying the data from the old to the temp, dropping the old and renaming the temp. Since I didn't define the notes column, the notes weren't copied.

I am fixing the code and will have a pull request later today.

I am not sure if you lost data or not. I would think you have, but you seem to indicate the data is still there, just not available until you modify the table.

If you didn't lose data, then my later PR will fix the remaining issues.

If you did lose data, you will need to:

  1. stop brewtarget
  2. Find your DATA_DIR (~/.config/brewtarget for me, but yours may be somewhere else).
  3. Copy database.sqlite to somewhere safe, eg, cp database.sqlite /var/tmp
  4. There should be a mess of backup copies named bt_database.[datestamp]. Find the one timestamped to closest before you ran the new code. Alternately, find the most recent one that shows settings::version to be 7.
  5. Verify you followed step three
  6. Copy the bt_database.[datastamp] you found in step 4 to database.sqlite, eg cp bt_database.20200303 database.sqlite
  7. Start brewtarget
    That will fire the database upgrade again, but this time without forgetting about brewnotes notes.

mikfire added a commit to mikfire/brewtarget that referenced this issue Apr 8, 2020
A simple bug with an unfortunate side effect.

In defining the TableProperty for brewnotes, I didn't include the notes
column. I am putting it back.

The problem is that the upgrade from version 7 -> 8 drops two columns
from brewnote. In sqlite, this means you create a temp table, copy the
data from the old table to the temp, drop the old table and rename the
temp.

Because I didn't define the notes column, it was dropped as well.
Anybody impacted will either need to find an existing backup that is
still version 7 and copy that into place, or will need to manually add
the column to the brewnote table. There is some indication that the data
was preserved, even though the column was dropped?
@cgspeck
Copy link
Contributor Author

cgspeck commented Apr 10, 2020

Thanks for the instructions, and it's no stress to me I took a backup before trying the new version so I can roll back at any time. I would be happy to test the fix once it is ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants