Implement Contacts #469 #500
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @bnsports, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
This pull request implements the Contacts feature, as described in issue #469. It introduces a new Contacts feature module with associated navigation, scenes, view models, and data types for managing a list of contacts and their addresses. The pull request also includes UI elements for adding, editing, and deleting contacts and their addresses, as well as integrating contact functionality into the recipient selection flow for transfers. The changes also include adding ContactService and DataValidation as new packages.
Highlights
- New Feature: Contacts: Implements the core Contacts feature, allowing users to manage a list of contacts with names, descriptions, and associated addresses.
- UI Implementation: Introduces new SwiftUI views and navigation flows for displaying contact lists, adding new contacts, and managing contact addresses.
- Data Validation: Adds a new
DataValidationpackage with validators for contact names, descriptions, and blockchain addresses. - Service Layer: Introduces a
ContactServicefor managing contact data, including adding, editing, and deleting contacts and addresses. - Integration with Transfer Flow: Integrates contact address selection into the recipient selection flow for initiating transfers.
- Settings Integration: Adds a Contacts entry to the Settings screen.
Changelog
Click here to see the changelog
- Features/Contacts/.gitignore
- Added standard git ignore entries for swift packages and build artifacts.
- Features/Contacts/Package.swift
- Created a new Swift package for the Contacts feature, declaring dependencies on
ContactServiceandDataValidation.
- Created a new Swift package for the Contacts feature, declaring dependencies on
- Features/Contacts/Sources/Navigation/AddContactAddressNavigationView.swift
- Introduced a navigation view for adding or editing contact addresses, including chain selection and QR code scanning.
- Features/Contacts/Sources/Navigation/AddContactNavigationView.swift
- Introduced a navigation view for adding or editing contact details (name, description).
- Features/Contacts/Sources/Navigation/ContactAddressListNavigationView.swift
- Created a navigation view for displaying a list of contact addresses and providing options to add or edit them.
- Features/Contacts/Sources/Navigation/ContactListNavigationView.swift
- Implemented the main contact list navigation view, allowing users to view, add, edit, and delete contacts.
- Features/Contacts/Sources/Scenes/AddContactAddressScene.swift
- Implemented the scene for adding or editing contact addresses, including UI elements for address input, chain selection, and memo.
- Features/Contacts/Sources/Scenes/AddContactScene.swift
- Implemented the scene for adding or editing contact details (name, description).
- Features/Contacts/Sources/Scenes/ContactAddressListScene.swift
- Implemented the scene for displaying a list of contact addresses, with options to add, edit, and delete addresses.
- Features/Contacts/Sources/Scenes/ContactListScene.swift
- Implemented the main contact list scene, displaying a list of contacts and providing options to add, edit, and delete contacts.
- Features/Contacts/Sources/Types/AddContactAddressInput.swift
- Defined the
AddContactAddressInputstruct for managing input data when adding or editing contact addresses, including validation logic.
- Defined the
- Features/Contacts/Sources/Types/AddContactInput.swift
- Defined the
AddContactInputstruct for managing input data when adding or editing contact details (name, description), including validation logic.
- Defined the
- Features/Contacts/Sources/Types/ContactAddressListViewItem.swift
- Defined the
ContactAddressListViewItemstruct for representing contact addresses in a list view.
- Defined the
- Features/Contacts/Sources/Types/ContactListViewItem.swift
- Defined the
ContactListViewItemstruct for representing contacts in a list view.
- Defined the
- Features/Contacts/Sources/Types/ContactViewTypes.swift
- Defined type aliases for contact-related handlers.
- Features/Contacts/Sources/View/ContactAddressListItemView.swift
- Implemented the
ContactAddressListItemViewfor displaying contact address information in a list.
- Implemented the
- Features/Contacts/Sources/View/ContactListItemView.swift
- Implemented the
ContactListItemViewfor displaying contact information in a list.
- Implemented the
- Features/Contacts/Sources/ViewModels/AddContactAddressViewModel.swift
- Implemented the
AddContactAddressViewModelfor managing the state and logic of the add/edit contact address scene.
- Implemented the
- Features/Contacts/Sources/ViewModels/AddContactViewModel.swift
- Implemented the
AddContactViewModelfor managing the state and logic of the add/edit contact scene.
- Implemented the
- Features/Contacts/Sources/ViewModels/ContactAddressListViewModel.swift
- Implemented the
ContactAddressListViewModelfor managing the state and logic of the contact address list scene.
- Implemented the
- Features/Contacts/Sources/ViewModels/ContactListViewModel.swift
- Implemented the
ContactListViewModelfor managing the state and logic of the contact list scene.
- Implemented the
- Features/Contacts/Tests/ContactsTests/ContactsTests.swift
- Added an example test case for the Contacts feature.
- Features/Settings/Sources/Scenes/SettingsScene.swift
- Added a navigation link to the Contacts feature in the Settings scene (line 68).
- Features/Settings/Sources/ViewModels/SettingsViewModel.swift
- Added properties for the Contacts title and image in the Settings view model (lines 45-46).
- Gem.xcodeproj/project.pbxproj
- Updated the project file to include the new Contacts feature, ContactService, and DataValidation packages, and to link the Contacts feature to the main app target.
- Gem/App-Configuration.swift
- Registered the
ContactServiceandContactStoreas main services and extensions.
- Registered the
- Gem/Assets/Navigation/SelectAssetSceneNavigationStack.swift
- Updated
RecipientNavigationViewto useRecipientViewModel.
- Updated
- Gem/Core/AppResolver/AppResolver+Services.swift
- Added
contactServiceto theAppResolver.
- Added
- Gem/Core/AppResolver/ServicesFactory.swift
- Added
contactServiceto theServicesFactory.
- Added
- Gem/Core/Types/Environment.swift
- Added
contactServiceto theEnvironmentValues.
- Added
- Gem/Root/Navigation/SelectedAsset/SelectedAssetNavigationStack.swift
- Updated
RecipientNavigationViewto useRecipientViewModel.
- Updated
- Gem/Root/Navigation/Settings/SettingsNavigationStack.swift
- Added navigation to the
ContactListNavigationViewfrom the settings screen.
- Added navigation to the
- Gem/Transfer/Navigation/RecipientNavigationView.swift
- Updated
RecipientNavigationViewto useRecipientViewModel.
- Updated
- Gem/Transfer/Scenes/RecipientScene.swift
- Updated
RecipientSceneto useRecipientViewModeland display contact addresses.
- Updated
- Gem/Transfer/ViewsModels/RecipientViewModel.swift
- Added
RecipientViewModelto manage recipient data and contact address selection.
- Added
- Packages/DataValidation/.gitignore
- Added standard git ignore entries for swift packages and build artifacts.
- Packages/DataValidation/Package.swift
- Created a new Swift package for data validation, declaring a dependency on
PrimitivesandKeystore.
- Created a new Swift package for data validation, declaring a dependency on
- Packages/DataValidation/Sources/Implementations/*.swift
- Implemented validators for blockchain addresses, chain selection, contact address memos, contact descriptions, and contact names.
- Packages/DataValidation/Sources/Types/ValidatedInput.swift
- Defined the
ValidatedInputstruct for managing validated input data.
- Defined the
- Packages/DataValidation/Sources/ValidatorConvertible.swift
- Defined the
ValidatorConvertibleprotocol for creating validators.
- Defined the
- Packages/DataValidation/Tests/DataValidationTests/*.swift
- Added test cases for the data validators.
- Packages/Primitives/Sources/Contact.swift
- Defined the
ContactandContactIdstructs for representing contact information.
- Defined the
- Packages/Primitives/Sources/ContactAddress.swift
- Defined the
ContactAddressandContactAddressIdstructs for representing contact addresses.
- Defined the
- Packages/Primitives/Sources/ContactAddressInfo.swift
- Defined the
ContactAddressInfostruct for representing contact address information.
- Defined the
- Packages/Primitives/Sources/Errors/ValidationError.swift
- Defined the
ValidationErrorenum for representing validation errors.
- Defined the
- Packages/Primitives/Sources/Scenes.swift
- Added new scenes for Contacts.
- Packages/PrimitivesComponents/Sources/Types/CreateUpdateViewType.swift
- Defined the
EntityEditorViewTypeenum for representing create/update view types.
- Defined the
- Packages/PrimitivesComponents/Sources/ViewModels/EntityEditorViewModel.swift
- Defined the
EntityEditorViewModelclass for managing entity editor view models.
- Defined the
- Packages/Store/Sources/Columns.swift
- Added columns for Contact and ContactAddress.
- Packages/Store/Sources/Migrations.swift
- Added migrations for Contact and ContactAddress.
- Packages/Store/Sources/Models/*.swift
- Added models for ContactAddressInfoRecord, ContactAddressRecord, and ContactRecord.
- Packages/Store/Sources/Requests/*.swift
- Added requests for ContactAddressInfoListRequest, ContactAddressListRequest, and ContactListRequest.
- Packages/Store/Sources/Stores/*.swift
- Added stores for ContactStore and updated StoreManager.
- Packages/Style/Sources/Images.swift
- Added images for contacts.
- Packages/Style/Sources/Resources/Assets.xcassets/settings/settings_contacts.imageset/*.svg
- Added SVG asset for contacts.
- Packages/Style/Sources/TextStyle.swift
- Added bodyBold text style.
- Services/ContactService/.gitignore
- Added standard git ignore entries for swift packages and build artifacts.
- Services/ContactService/Package.swift
- Created a new Swift package for the ContactService, declaring dependencies on
Primitives,Store, andGemAPI.
- Created a new Swift package for the ContactService, declaring dependencies on
- Services/ContactService/Sources/ContactService.swift
- Implemented the ContactService for managing contact data.
- Services/ContactService/Tests/ContactServiceTests/ContactServiceTests.swift
- Added an example test case for the ContactService.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Did you know?
The term 'contact list' has evolved from physical address books to digital databases, reflecting the shift from paper-based record-keeping to electronic data management.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
The pull request implements the Contacts feature, allowing users to manage a list of contacts and their associated addresses. The implementation appears well-structured and integrates with existing components. However, there are some areas that could benefit from further review and potential improvements, particularly regarding error handling and data validation.
Summary of Findings
- Missing Error Handling: There are several places where errors are caught but simply localized and presented to the user. More robust error handling, such as logging or more specific error messages, could improve debugging and user experience.
- Data Validation: While data validation is implemented, ensuring that validation occurs at the appropriate layers and that error messages are user-friendly is crucial. Review the validators to ensure they cover all edge cases.
- ViewModel Initialization: Some ViewModels are initialized directly in the View structs. Consider using dependency injection or a factory pattern to improve testability and maintainability.
Merge Readiness
The pull request introduces a significant feature and requires careful review before merging. While the code appears functional, the identified issues regarding error handling and data validation should be addressed to ensure a robust and user-friendly experience. I am unable to approve this pull request, and recommend that it not be merged until the critical and high severity issues are addressed, and that others review and approve this code before merging.
| private func onSelectConfirm() { | ||
| do { | ||
| try model.confirmAddContact() | ||
| } catch { | ||
| presentingErrorMessage = error.localizedDescription | ||
| } |
There was a problem hiding this comment.
The confirmAddContact function catches errors and presents a localized description to the user. Consider logging the error for debugging purposes or providing a more specific error message to the user.
} catch {
presentingErrorMessage = error.localizedDescription
// Log the error for debugging
print("Error adding contact: \(error)")
}| do { | ||
| try model.confirmAddContact() | ||
| } catch { | ||
| presentingErrorMessage = error.localizedDescription | ||
| } | ||
| } |
There was a problem hiding this comment.
Similar to AddContactAddressScene.swift, the error handling here is basic. Logging the error or providing more context to the user could be beneficial.
} catch {
presentingErrorMessage = error.localizedDescription
// Log the error for debugging
print("Error adding contact: \(error)")
}| do { | ||
| try model.delete(address: address) | ||
| } catch { | ||
| presentingErrorMessage = error.localizedDescription | ||
| } | ||
| } |
There was a problem hiding this comment.
The didTapConfirmDelete function catches errors and presents a localized description. Consider logging the error for debugging purposes.
} catch {
presentingErrorMessage = error.localizedDescription
// Log the error for debugging
print("Error deleting address: \(error)")
}| private func didTapConfirmDelete(contact: Contact) { | ||
| do { | ||
| try model.delete(contact: contact) | ||
| } catch { | ||
| presentingErrorMessage = error.localizedDescription | ||
| } |
There was a problem hiding this comment.
The didTapConfirmDelete function catches errors and presents a localized description. Consider logging the error for debugging purposes.
} catch {
presentingErrorMessage = error.localizedDescription
// Log the error for debugging
print("Error deleting contact: \(error)")
}…ontacts # Conflicts: # Packages/Store/Sources/Migrations.swift
Packages/Store/Sources/Requests/ContactAddressInfoListRequest.swift
Outdated
Show resolved
Hide resolved
| ].allSatisfy({$0}) | ||
| } | ||
|
|
||
| static private func createIdForNewEntity() -> String { |
There was a problem hiding this comment.
can we delegate sqlite to create ID automatically?
There was a problem hiding this comment.
as we want to have cross-device contact synchronization in the future we cannot use auto-incremented integer ids cause it will lead to conflicts. so we need 'random' string identifier which sqlite cannot create automatically
I can suggest to have ContactAddressRecord initializer with nullable id parameter and we can move id generation to database layer
Packages/Store/Sources/Requests/ContactAddressListRequest.swift
Outdated
Show resolved
Hide resolved
Packages/Store/Sources/Requests/ContactAddressListRequest.swift
Outdated
Show resolved
Hide resolved
# Conflicts: # Gem.xcodeproj/project.pbxproj # Gem/Root/Navigation/SelectedAsset/SelectedAssetNavigationStack.swift # Gem/Transfer/Navigation/RecipientNavigationView.swift # Gem/Transfer/ViewsModels/RecipientViewModel.swift
| ChainView(model: ChainViewModel(chain: model.input.chain)) | ||
| } | ||
| FloatTextField(model.addressTextFieldTitle, text: $model.projectedValue.input.address.value ?? "", allowClean: false) { | ||
| HStack(spacing: .large/2) { |
There was a problem hiding this comment.
Consider to use default Spacing, for consistency UI
| } | ||
|
|
||
| private func onSelectPaste() { | ||
| guard let address = UIPasteboard.general.string else { return } |
There was a problem hiding this comment.
Move the logic to viewModel
| } | ||
|
|
||
| private func presentCreateError(_ error: Error) { | ||
| if (error as? DatabaseError)?.extendedResultCode == .SQLITE_CONSTRAINT_UNIQUE { |
There was a problem hiding this comment.
The same, move the logic to viewModel
| } | ||
| } | ||
| .overlay { | ||
| if contacts.isEmpty { |
There was a problem hiding this comment.
Need to use EmptyContentView
| } catch { | ||
| presentCreateError(error) | ||
| } | ||
| default: |
There was a problem hiding this comment.
We need to process all the cases
| @@ -19,10 +22,15 @@ enum RecipientScanResult { | |||
| case transferData(TransferData) | |||
| } | |||
|
|
|||
| class RecipientViewModel: ObservableObject { | |||
| @Observable | |||
| class RecipientViewModel { | |||
| } | ||
|
|
||
| public func fetch(_ db: Database) throws -> [ContactAddressData] { | ||
| do { |
There was a problem hiding this comment.
Remove do catch block, use just try ContactAddressRecord
| } | ||
|
|
||
| public func fetch(_ db: Database) throws -> [ContactAddress] { | ||
| guard let contact else { |
There was a problem hiding this comment.
grdb should return defaultValue ?
| public init() { } | ||
|
|
||
| public func fetch(_ db: Database) throws -> [Contact] { | ||
| do { |
|
|
||
| extension ContactService { | ||
| public func add(contact: Contact) throws -> ContactId { | ||
| return try contactStore.addContact(contact) |
Implemented Feature description
Screenshots:
To Do: