-
-
Notifications
You must be signed in to change notification settings - Fork 394
feat(parity): migrate json libraries to sqlite #604
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
Merged
Merged
Changes from all commits
Commits
Show all changes
38 commits
Select commit
Hold shift + click to select a range
c0a01fd
feat(ui): add PagedPanel widget
CyanVoxel f766223
feat(ui): add MigrationModal widget
CyanVoxel c13904c
feat: add basic json to sql conversion
CyanVoxel 1888a20
fix: chose `poolclass` based on file or memory db
CyanVoxel 45f24f8
feat: migrate tag colors from json to sql
CyanVoxel 29d4eea
feat: migrate entry fields from json to sql
CyanVoxel 4b8bd32
set default `is_new` case
CyanVoxel ecabba9
fix: limit correct tag query
CyanVoxel 186c90f
feat: migrate tag aliases and subtags from json to sql
CyanVoxel 7e82534
add migration timer
CyanVoxel a810c70
fix(tests): fix broken tests
CyanVoxel 2159caf
rename methods, add docstrings
CyanVoxel 86098ba
revert tag id search, split tag name search
CyanVoxel 0ffa4b5
fix: use correct type in sidecar macro
CyanVoxel 3df8cfd
tests: add json migration tests
CyanVoxel 6103e15
fix: drop leading dot from json extensions
CyanVoxel 642c856
add special characters to json db test
CyanVoxel a8bfc9d
tests: add file path and entry field parity checks
CyanVoxel a40c6fe
fix(ui): tag manager no longer starts empty
CyanVoxel 4d01baf
fix: read old windows paths as posix
CyanVoxel 19daa68
tests: add posix + windows paths to json library
CyanVoxel d37d408
tests: add subtag, alias, and shorthand parity tests
CyanVoxel 7a2131b
tests: ensure no none values in parity checks
CyanVoxel 5d60797
tests: add tag color test, use tag id in tag tests
CyanVoxel 001019f
tests: fix and optimize tests
CyanVoxel 4cfe23d
tests: add discrepancy tracker
CyanVoxel 4ac893d
refactor: reduce duplicate UI code
CyanVoxel f81be7a
fix: load non-sequential entry ids
CyanVoxel bde75ab
fix(ui): sort tags in the preview panel
CyanVoxel c9d585f
tests(fix): prioritize `None` check over equality
CyanVoxel 6879b84
fix(tests): fix multi "same tag field type" tests
CyanVoxel 2c52057
ui: increase height of migration modal
CyanVoxel 21a0068
feat: add progress bar to migration ui
CyanVoxel f37632c
fix(ui): sql values update earlier
CyanVoxel e2a81e1
refactor: use `get_color_from_str` in test
CyanVoxel 6651085
refactor: migrate tags before aliases and subtags
CyanVoxel 29b8585
remove unused assertion
CyanVoxel 4ec0b8c
refactor: use `json_migration_req` flag
CyanVoxel File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This seems a bit weird to me, why are the sub tags and aliases added before the tags that their foreign keys point to?
I haven't tested this, but I suspect that, because every operation in those loops is its own DB Transaction, should TS crash after the Aliases, but before the Tags have been added, that there would be database inconsistencies stemming from foreign keys in the Aliases and Subtags pointing to tags that don't exist. This might not matter if the migration just restarts after such a crash (or killing by the user), but seems like it is just asking for problems.
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.
Might've been some negative reinforcement during testing that got me to believe that this was needed, but swapping it to do the tags first seems to work just fine (and logically makes more sense).
While the user shouldn't have the option to complete the migration without the full DB migrated, it's still a great observation.