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

Book cover image sometimes does not save #773

Closed
ssddanbrown opened this Issue Mar 30, 2018 · 3 comments

Comments

2 participants
@ssddanbrown
Member

ssddanbrown commented Mar 30, 2018

Been noticing a strange effect on my Work BookStack instance. Every so often, On seemingly first try, When setting a cover image for a book it does not seem to actually save the new cover image against the book.

Often works when going back to the book and re-trying. Tricky to re-produce.

@ssddanbrown ssddanbrown added the Bug label Mar 30, 2018

@dsaundersWizmo

This comment has been minimized.

dsaundersWizmo commented Mar 30, 2018

@ssddanbrown
I just set book covers on all my books and it happened almost every time-- but only to books I had created prior to 19.0.

So it looks like the problem was that if the image_id column in books is NULL then the first save creates the value as 0. Then the second save updates the value with the correct id.

I replicated this by creating a new book and then setting the image_id to NULL and can reproduce the behavior. Maybe the next upgrade can include a change the column to make it NOT NULL with a default value of 0 and all the existing NULLs people have will be udpated. Otherwise, it doesn't look like an issue going forward if this is the real cause and might not be worth fixing retroactively?

Anyone with this issue can fix it by updating all NULL values in that column with a 0 prior to setting images.

@dsaundersWizmo

This comment has been minimized.

dsaundersWizmo commented Mar 30, 2018

https://github.com/BookStackApp/BookStack/blob/master/resources/views/books/form.blade.php

Line 26, looks like the null is failing the condition check and setting it to 0 on first save?

@ssddanbrown ssddanbrown added this to the BookStack Beta v0.21.0 milestone Mar 31, 2018

@ssddanbrown ssddanbrown self-assigned this Mar 31, 2018

@ssddanbrown

This comment has been minimized.

Member

ssddanbrown commented Mar 31, 2018

Thank you @dsaundersWizmo so much for finding the root issue. Was going a bit mad why I could not reproduce this in dev but it totally makes sense now.

Instead of doing any database updates I've tweaked how the existing image gets set in the view which effectively solves the issue: 7f437c2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment