Bug 2049279 - Add passport record type to autofill#7440
Conversation
mhammond
left a comment
mhammond
left a comment
There was a problem hiding this comment.
This looks like I expected it to :)
| } | ||
|
|
||
| fn upgrade_from_v4(db: &Connection) -> Result<()> { | ||
| let migration_string: &str = include_str!("../../sql/migrations/v5_migration.sql"); |
There was a problem hiding this comment.
Do we need this migration, or can we just re-execute create_shared_schema? That would avoid needing the test for them drifting too. I'm not sure why the others weren't done that way, but that's the approach most other components use
There was a problem hiding this comment.
Nice catch, thanks! I updated the patch!
| CREATE TABLE IF NOT EXISTS passports_tombstones ( | ||
| guid TEXT PRIMARY KEY CHECK(length(guid) != 0), | ||
| time_deleted INTEGER NOT NULL | ||
| ) WITHOUT ROWID; |
There was a problem hiding this comment.
you don't have the shared triggers for this yet. That seems fine I guess until you sync though.
There was a problem hiding this comment.
I'd defer the shared triggers until implementing sync, to keep this patch simpler. I'm keeping the tombstones table in this patch so we don't need a migration when sync is implemented.
| sync_change_counter INTEGER NOT NULL | ||
| ); | ||
|
|
||
| DROP TABLE IF EXISTS passports_sync_staging; |
There was a problem hiding this comment.
because these are temp tables you don't actually need them yet - they can be added later without needing a migration. I guess there's no harm adding them though, but also no real benefit.
Add a passport autofill record type (name, country, passport number, and issue/expiry dates) with full CRUD, modeled on the existing addresses type. The schema is laid out sync-ready: passports_data carries a sync_change_counter alongside passports_mirror / passports_tombstones and the sync staging tables, so a sync engine can be added later without a schema migration. No sync engine is registered yet, so passports are not synced. Fields follow the autofill convention - stored as plaintext, with empty/0 meaning "not provided".
DimiDL
force-pushed
the
passport-autofill
branch
from
1b11b30 to
731f21b
Compare
3 participants
Add a passport autofill record type (name, country, passport number, and issue/expiry dates) with full CRUD, modeled on the existing addresses type.
The schema is laid out sync-ready: passports_data carries a sync_change_counter alongside passports_mirror / passports_tombstones and the sync staging tables, so a sync engine can be added later without a schema migration. No sync engine is registered yet, so passports are not synced. Fields follow the autofill convention - stored as plaintext, with empty/0 meaning "not provided".
Pull Request checklist
[ci full]to the PR title.