-
Notifications
You must be signed in to change notification settings - Fork 135
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 react-native migrations on older Android installations #1152
Fix react-native migrations on older Android installations #1152
Conversation
…ure PreMigrationKey has all the properties of Key (as used in the createDatabase migration), to ensure that the change column results in a table with all columns, not dropping for instance the privateKeyHex in Sqlite on older RN environments
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## next #1152 +/- ##
==========================================
+ Coverage 80.25% 85.25% +5.00%
==========================================
Files 118 151 +33
Lines 4056 15418 +11362
Branches 875 1629 +754
==========================================
+ Hits 3255 13145 +9890
- Misses 798 2273 +1475
+ Partials 3 0 -3
... and 58 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
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.
Fantastic, thanks for adding this!
@mirceanis Okay we have tested this against multiple Android versions and it seems to be working for all of them now. |
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.
It looks great. Thank you for contributing this!
What issue is this PR fixing
In React-Native on older Android APIs (<11) the migrations where failing. That has to do with the 2nd and 3rd migration file. The 2nd migration file wasn't correctly creating new temp tables including privateKeyHex, because it picked up the wrong entity it seems. Probably because the matching logic on the metadata against a list of metadata has a different order in these environments. As a result the 3rd migration also failed. It is now using a name when creating new tables, but using the actual matched table from the matching logic on existing tables.
Unrelated, the TypeORM migrations are failing in production settings where minifying is being used. That has to do with the fact that TypeORM relies on classnames and its timestamp suffix. When minifying this goes away, so I simply added the name property to the migration files.
As the same method was being copied over between migrations, also create 2 helper functions, so that the code is now shared across migrations and can also be used by people extending Veramo, which might have their own migrations.
Still needs more testing before converting this from draft to final PR
What is being changed
A clear description of what this PR brings.
Quality
Check all that apply:
pnpm i
,pnpm build
,pnpm test
,pnpm test:browser
locally.