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

fix: handle missing reaction #4094

Merged
merged 14 commits into from
Nov 6, 2024
Merged

Conversation

RoccoSmit
Copy link
Contributor

@RoccoSmit RoccoSmit commented Nov 3, 2024

as per #4060 the 00__reactions.sql migration for v0.23 does not run when you start with a fresh install

Update:
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 reaction
image
image

After reaction update the new reaction shows and thumbs up is not used
image
image

@RoccoSmit RoccoSmit requested a review from boojack as a code owner November 3, 2024 12:37
@RoccoSmit
Copy link
Contributor Author

Not sure what is causing checks to fail. Will revisit tomorrow

@boojack
Copy link
Member

boojack commented Nov 3, 2024

@RoccoSmit Expectedly, the newly started instance should not execute any migrations. So I think what's needed here is to handle the default reactions value for memo setting.

Reference: https://github.com/usememos/memos/blob/main/store/workspace_setting.go#L157

@RoccoSmit
Copy link
Contributor Author

Makes sense. I wasn't sure if the LASTEST.sql files were for creating the initial db structure and the migrations were to add data to the tables.

If migrations are not run on the first install, would it make sense to add the insertion of the initial reactions to the LATEST.sql files? If not, I'll add 👍 as a default reaction value in workspace_setting.go if no value is set during save and load.

@boojack
Copy link
Member

boojack commented Nov 4, 2024

I'll add 👍 as a default reaction value in workspace_setting.go if no value is set during save and load.

SGTM

@RoccoSmit RoccoSmit changed the title Run migrations for fresh installs Handle missing reaction Nov 6, 2024
store/workspace_setting.go Outdated Show resolved Hide resolved
@boojack boojack changed the title Handle missing reaction fix: handle missing reaction Nov 6, 2024
Copy link
Member

@boojack boojack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@boojack boojack merged commit 3b25a6b into usememos:main Nov 6, 2024
2 checks passed
@RoccoSmit RoccoSmit deleted the migration-version branch November 6, 2024 13:15
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 this pull request may close these issues.

2 participants