-
Notifications
You must be signed in to change notification settings - Fork 91
fix(reactions): fix old reactions not being migrated #19280
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
Conversation
Jenkins BuildsClick to see older builds (60)
|
|
I'll rebase the PR |
c99f76c to
669ba53
Compare
|
669ba53 to
2815de8
Compare
|
Rebased to latest |
Yes it should be good. It only changes how we get dependencies, but not the dependencies/code itself. |
I restarted the build but it seems like it fails on Android while building |
|
Yup, it failed to build on Android :( Halp needed @alexjba @igor-sirotin This one is suspicious: it's trying to build |
@caybro Actually, we should have not rebased it to latest UPD: Or we don't want this in release? |
I think we definitely want this PR in the release |
|
I think the easiest would be to backport status-im/status-go#7110 into the |
|
I'm not sure wtf is going on with Android build, not sure how this could have been caused. I tested the changes in desktop before merging. Yeah, cherry-pick to status-go release and then base this PR on release makes the most sense to me now 👍 |
Can you pls take care of this 🙏 |
2815de8 to
fb31e5c
Compare
done, let's see if it builds |
It seems it worked :) Now I noticed you're already targetting the release branch... let's make sure we don't overwrite it later on @alaibe, or just merge this one after the release branch update? 🤔 |
Yes, targeting release branch directly. Not sure I got what order you mean? |
|
Hey @caybro @igor-sirotin we will cut the release again today from |
@noeliaSD we're keeping the |
fb31e5c to
8bc2319
Compare
8bc2319 to
5f45d68
Compare
|
Trying to point directly to status-go |
|
status-im/status-go#7110 was already merged in to |
What does the PR do
Issue by @sunleos , if you migrate an account from 2.35 or lower to master, the old emojis reactions would show blank.
The solution was to add a migration on the backend.
Se the PR here: status-im/status-go#7110
Affected areas
status-go migration
Architecture compliance
My PR is consistent with this document: QML Architecture Guidelines
Screencapture of the functionality
Impact on end user
Fixes the issue
How to test
Migrate an old account that had emoji reactions
Risk
Low