Skip to content

add logins store api method add_with_record#6754

Merged
jo merged 1 commit intomozilla:mainfrom
jo:logins-add-with-meta
May 22, 2025
Merged

add logins store api method add_with_record#6754
jo merged 1 commit intomozilla:mainfrom
jo:logins-add-with-meta

Conversation

@jo
Copy link
Contributor

@jo jo commented May 14, 2025

  • add logins store api methods for bulk insert and meta insert, intended to be used during migration and CSV import on desktop:
    • fn add_with_record(&self, entry_with_record: LoginEntryWithRecordFields): add a login together with metadata
    • fn add_many(&self, entries: Vec<LoginEntry>): add multiple logins with single transaction
    • fn add_many_with_records(&self, entries_with_records: Vec<LoginEntryWithRecordFields>): add multiple logins with metadata within single transaction

Pull Request checklist

  • Breaking changes: This PR follows our breaking change policy
    • This PR follows the breaking change policy:
      • This PR has no breaking API changes, or
      • There are corresponding PRs for our consumer applications that resolve the breaking changes and have been approved
  • Quality: This PR builds and tests run cleanly
    • Note:
      • For changes that need extra cross-platform testing, consider adding [ci full] to the PR title.
      • If this pull request includes a breaking change, consider cutting a new release after merging.
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Changelog: This PR includes a changelog entry in CHANGELOG.md or an explanation of why it does not need one
    • Any breaking changes to Swift or Kotlin binding APIs are noted explicitly
  • Dependencies: This PR follows our dependency management guidelines
    • Any new dependencies are accompanied by a summary of the due diligence applied in selecting them.

@jo jo force-pushed the logins-add-with-meta branch from c9c9b73 to b7f3719 Compare May 14, 2025 10:33
@jo jo requested review from bendk and mhammond May 14, 2025 11:17
@jo jo force-pushed the logins-add-with-meta branch 9 times, most recently from 160e078 to 5f6c439 Compare May 19, 2025 20:23
Copy link
Contributor

@bendk bendk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great to me. The only thing I'm really concerned about is finding a better name for RecordFields. At the very least it should be LoginRecordFields, otherwise there's a high chance for confusion or name conflicts on Swift.

@jo jo force-pushed the logins-add-with-meta branch from 5f6c439 to 6a7c160 Compare May 20, 2025 17:56
@jo jo requested a review from bendk May 20, 2025 18:00
@jo jo force-pushed the logins-add-with-meta branch from 6a7c160 to b79567e Compare May 21, 2025 09:19
The a few new method
```
fn add_with_record(&self, entry_with_record: LoginEntryWithRecordFields);
fn add_many_with_records(&self, entries_with_records: Vec<LoginEntryWithRecordFields>);
fn add_many(&self, entries: Vec<LoginEntry>);
```
is intended to be used during migration on desktop.

This renames previously only internally used struct `RecordFields` to
now exposed `LoginMeta` for clarity. Also, all structs referencing these
are now called `meta` instead of `record`.
@jo jo force-pushed the logins-add-with-meta branch from b79567e to fc2c250 Compare May 21, 2025 09:21
Copy link
Contributor

@bendk bendk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new names and docstrings look great to me. I think this is ready to merge.

@jo jo added this pull request to the merge queue May 22, 2025
Merged via the queue into mozilla:main with commit ee34bcf May 22, 2025
15 checks passed
@jo jo deleted the logins-add-with-meta branch May 22, 2025 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants