Skip to content
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

refactor: implement new slow sync - WPB-10727 #1999

Open
wants to merge 19 commits into
base: develop
Choose a base branch
from

Conversation

jullianm
Copy link
Contributor

@jullianm jullianm commented Oct 3, 2024

TaskWPB-10727 [iOS] Implement new slow sync

Key points

This PR is about performing a slow sync to get the database initially up-to-date by triggering necessary calls to repositories and use cases.

Dedicated repositories and use cases will pull and store up-to-date objects into the database.

    func performSlowSync() async throws {
        try await updateEventsRepository.pullLastEventID()
        try await teamRepository.pullSelfTeam()
        try await teamRepository.pullSelfTeamRoles()
        try await teamRepository.pullSelfTeamMembers()
        try await connectionsRepository.pullConnections()
        try await conversationsRepository.pullConversations()
        try await userRepository.pullKnownUsers()
        try await userRepository.pullSelfUser()
        try await teamRepository.pullSelfLegalHoldStatus()
        try await conversationLabelsRepository.pullConversationLabels()
        try await featureConfigsRepository.pullFeatureConfigs()
        try await pushSupportedProtocolsUseCase.invoke()
        try await oneOnOneResolverUseCase.invoke()
    }

Testing

UTs cover the following use cases, ensuring that

  • performing a slow sync invokes the correct methods for repositories and use cases.

Checklist

  • Title contains a reference JIRA issue number like [WPB-XXX].
  • Description is filled and free of optional paragraphs.
  • Adds/updates automated tests.

UI accessibility checklist

If your PR includes UI changes, please utilize this checklist:

  • Make sure you use the API for UI elements that support large fonts.
  • All colors are taken from WireDesign.ColorTheme or constructed using WireDesign.BaseColorPalette.
  • New UI elements have Accessibility strings for VoiceOver.

@echoes-hq echoes-hq bot added echoes: technical-roadmap Work contributing to the Technical Roadmap, to improve our velocity or reduce the technical debt. echoes/initiative: ios-sync-refactoring labels Oct 3, 2024
Copy link
Contributor

github-actions bot commented Oct 3, 2024

Test Results

289 tests   289 ✅  6s ⏱️
 57 suites    0 💤
  2 files      0 ❌

Results for commit f5820cb.

♻️ This comment has been updated with latest results.

@jullianm jullianm requested a review from netbe October 9, 2024 12:16
Comment on lines 56 to 58
enum Error: Swift.Error {
case failedToPerformSlowSync(Swift.Error)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps SyncManagerError, this way we avoid having to fully qualify Swift.Error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since the enum is nested inside the SyncManager, we know the error is a SyncManager error, also it feels more intuitive at callsite: SyncManager.Error rather than SyncManager.SyncManagerError which seems redundant. It is also aligned with what we did for processors.

On a more general perspective, I would go with either of the two following approaches:

1/ Call it NameOfTheObjectError but don't nest it inside the object
2/ Call it Error and have it nested inside the object.

Both solutions work for me, in WireDomain we use both approaches though, we should streamline this to one or the other solution.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We could also nest it and call it Failure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done in e151fbc

Copy link
Contributor

This PR is stale because it has been open 30 days with no activity. Please update it or close it in case is not relevant anymore.

@github-actions github-actions bot removed the stale label Nov 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
echoes/initiative: ios-sync-refactoring echoes: technical-roadmap Work contributing to the Technical Roadmap, to improve our velocity or reduce the technical debt.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants