-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
fix: handle missing reaction #4094
Conversation
Not sure what is causing checks to fail. Will revisit tomorrow |
@RoccoSmit Expectedly, the newly started instance should not execute any migrations. So I think what's needed here is to handle the default Reference: https://github.com/usememos/memos/blob/main/store/workspace_setting.go#L157 |
Makes sense. I wasn't sure if the If migrations are not run on the first install, would it make sense to add the insertion of the initial reactions to the |
SGTM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
as per #4060 the
00__reactions.sql
migration forv0.23
does not run when you start with a fresh installUpdate:
Comments suggested we go with a default value set in code rather than migration changes so reverted and made those changes instead.
e.g.
Memo with no reactions in db shows
thumbs up
default reaction in settings and on memo reactionAfter reaction update the new reaction shows and
thumbs up
is not used