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

Migration error related to Open prices in some conditions #5732

Closed
teolemon opened this issue Oct 25, 2024 · 15 comments
Closed

Migration error related to Open prices in some conditions #5732

teolemon opened this issue Oct 25, 2024 · 15 comments

Comments

@teolemon
Copy link
Member

teolemon commented Oct 25, 2024

What

  • Migration error related to Open prices in some conditions

Screenshot

@g123k
Copy link
Collaborator

g123k commented Oct 25, 2024

Could you add your version number, please?

@monsieurtanuki
Copy link
Contributor

Looks like a failing database upgrade on a test device that went back and forth on different app versions.

The thing is that once we added a column to a table - according to a database version number (in that case, < 7) - if you install an older version of the app and then again a recent version of the app, there's some confusion and the "add column" query is called again, on a table where the columns where already added. Which causes an error.

@teolemon Did that crash actually happen on a device with back and forth app version numbers?

@teolemon
Copy link
Member Author

Latest commit from main

@teolemon
Copy link
Member Author

yes, some back and forth on my side

@monsieurtanuki
Copy link
Contributor

yes, some back and forth on my side

That's the explanation.
The good news is that it won't affect "normal" users.
Would you like me to fix it for "different" users like you? Won't be nice code, but at least you wouldn't be blocked anymore.

@teolemon
Copy link
Member Author

I can wipe my cache, I do that often.
Since we had that hotfix by @g123k for prod, I would assume all internal android users might be affected (10-20 persons) ?

@monsieurtanuki
Copy link
Contributor

I can wipe my cache, I do that often.

That would do the trick.

Since we had that hotfix by @g123k for prod, I would assume all internal android users might be affected (10-20 persons) ?

I don't know the hotfix you're talking about.

@g123k
Copy link
Collaborator

g123k commented Oct 25, 2024

I can wipe my cache, I do that often.

That would do the trick.

Since we had that hotfix by @g123k for prod, I would assume all internal android users might be affected (10-20 persons) ?

I don't know the hotfix you're talking about.

We have published a .1 version with a fix for the blocking onboarding: https://github.com/openfoodfacts/smooth-app/tree/release/4.16.0-hotfix1

@monsieurtanuki
Copy link
Contributor

We have published a .1 version with a fix for the blocking onboarding: https://github.com/openfoodfacts/smooth-app/tree/release/4.16.0-hotfix1

Oh, I wasn't aware of that. Looks like the sqflite database version is still 6 (and not 7 as expected): probably a recent version of the app used db version 7 (and added the columns) then the hotfix downgraded it to 6, and now we're back to 7 (where we cannot build again the columns).

Anyway, I can "fix" the OP "alter table" bug if needed.

@g123k
Copy link
Collaborator

g123k commented Oct 25, 2024

We have published a .1 version with a fix for the blocking onboarding: https://github.com/openfoodfacts/smooth-app/tree/release/4.16.0-hotfix1

Oh, I wasn't aware of that. Looks like the sqflite database version is still 6 (and not 7 as expected): probably a recent version of the app used db version 7 (and added the columns) then the hotfix downgraded it to 6, and now we're back to 7 (where we cannot build again the columns).

Anyway, I can "fix" the OP "alter table" bug if needed.

Honestly, I'm not sure that for 10 people, there's a need for that change… that will probably stay forever in the code.

@monsieurtanuki
Copy link
Contributor

Honestly, I'm not sure that for 10 people, there's a need for that change… that will probably stay forever in the code.

It depends on who the hotfix was for.
If it was for "normal" users, they'll be impacted when upgrading to the next Smoothie version.
If it was for "internal" users, it was not very relevant, because the user just had to kill the app and restart it to escape from the onboarding.

@g123k
Copy link
Collaborator

g123k commented Oct 25, 2024

Honestly, I'm not sure that for 10 people, there's a need for that change… that will probably stay forever in the code.

It depends on who the hotfix was for. If it was for "normal" users, they'll be impacted when upgrading to the next Smoothie version. If it was for "internal" users, it was not very relevant, because the user just had to kill the app and restart it to escape from the onboarding.

No, I was talking about the DB fix.
For the onboarding, it was for 100% of new users in prod at least for a few weeks.

@monsieurtanuki
Copy link
Contributor

No, I was talking about the DB fix. For the onboarding, it was for 100% of new users in prod at least for a few weeks.

The users affected will be users that installed a recent dev version with db version 7, then downgraded the db version 6 with the hotfix.
Next time they install a recent version - e.g. dev or 4.17.0 - the app will crash at startup.

@monsieurtanuki monsieurtanuki removed db migrations Database migrations 🐛 bug Something isn't working labels Oct 25, 2024
@teolemon
Copy link
Member Author

I've already wiped my cache, honestly 😅

@monsieurtanuki
Copy link
Contributor

Then I guess we can close the issue.

@github-project-automation github-project-automation bot moved this from 💬 To discuss and validate to 🎊 Done in 🤳🥫 The Open Food Facts mobile app (Android & iOS) Oct 25, 2024
@github-project-automation github-project-automation bot moved this from Backlog to Done in 💸 Open Prices Oct 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants