Skip to content

refactor(db-repository): extract folder details operations to FolderDetailsRepository - #11478

Merged
rafaeltonholo merged 10 commits into
mainfrom
chore/11470/split-folder-repository
Sep 4, 2026
Merged

rafaeltonholo merged 10 commits into
mainfrom
chore/11470/split-folder-repository

Conversation

@rafaeltonholo

Copy link
Copy Markdown
Member

Contribution Summary

Linked Issue/Ticket: Part of #11470

Description

  • Add FolderDetailsRepository interface with findById and update methods
  • Add DefaultFolderDetailsRepository implementation
  • Remove folder details methods from FolderRepository interface
  • Update FolderSettingsViewModel and FolderSettingsDataStore to use FolderDetailsRepository
  • Update DefaultSpecialFolderUpdater to use partial updates via new repository
  • Add PartialUpdatableFolderDetails for granular folder property updates
  • Add FailedPrecondition error case to FolderError

AI Disclosure

Select one of the following (mandatory)

  • This contribution does not include any changes created or assisted by AI.
  • This contribution includes changes assisted by AI.
  • This contribution includes changes created by AI.

Contribution Checklist

  • I have read and affirm that my contribution adheres to Mozilla’s Community Participation Guidelines
  • This contribution is in Kotlin where possible
  • This contribution does not use merge commits
  • This contribution adheres to the existing codestyle (run gradlew spotlessCheck to check and gradlew spotlessApply to format your source code; will be checked by CI).
  • This contribution does not break existing unit tests (run gradlew testDebugUnitTest; will be checked by CI).
  • This contribution includes tests for any new functionality, and maintains tests for any updated functionality.
  • This contribution adheres to our Engineering process (RFC/Technical Design/ADR)
  • This PR has a descriptive title and body that accurately outlines all changes made, and contains a reference to any issues that it fixes (e.g. Closes #XXX or Fixes #XXX).

Stack created with GitHub Stacks CLIGive Feedback 💬

@rafaeltonholo
rafaeltonholo requested a review from a team as a code owner September 1, 2026 16:07
@github-actions github-actions Bot added the tb-team Tasks and features handled by project maintainers label Sep 1, 2026
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Validation Passed: All report and feature-flag labels are correctly set.

@rafaeltonholo rafaeltonholo added the report: exclude Exclude changes from user-facing reports (internal, minor, or not relevant to users). label Sep 1, 2026
@rafaeltonholo
rafaeltonholo removed the request for review from jbott-tbird September 1, 2026 16:09
@rafaeltonholo rafaeltonholo assigned wmontwe and unassigned jbott-tbird Sep 1, 2026
@rafaeltonholo
rafaeltonholo force-pushed the chore/11470/split-folder-repository branch from 377d788 to b0b642b Compare September 2, 2026 10:50

@wmontwe wmontwe left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please remove the duplicate property and use a single name instead, as both represent the same setting.

I like the use of the logger, but please check if sensitive data is logged when whole objects are passed to the logger.

Comment thread legacy/core/src/main/java/com/fsck/k9/mailstore/KoinModule.kt Outdated
verify(messageStore, never()).setPushEnabled(any(), any())
assertThat(result).isEqualTo(Outcome.success(Unit))
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
@Test
fun `partial update should update unified inbox inclusion when provided`() = runTest {
// Arrange
val partialUpdate = PartialUpdatableFolderDetails(
folderId = REGULAR_FOLDER_ID,
includeInUnifiedInbox = true,
)
// Act
val result = testSubject.update(accountId, partialUpdate)
// Assert
verify(messageStore).setIncludeInUnifiedInbox(REGULAR_FOLDER_ID, true)
assertThat(result).isEqualTo(Outcome.success(Unit))
}

Comment on lines +36 to +37
val includeInUnifiedInbox: Boolean? = null,
val integrate: Boolean? = null,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think these properties represent the same setting. In the database, the integrate column determines whether a folder is included in the unified folder. Elsewhere, the legacy name includeInUnifiedInbox is used. Since “Unified Folders” is the current terminology, includeInUnifiedFolder would be clearer and should be the only property in the partial-update model. The rename is nice to have.

Suggested change
val includeInUnifiedInbox: Boolean? = null,
val integrate: Boolean? = null,
val includeInUnifiedFolder: Boolean? = null,

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Got it. I will drop the integrate property, then.

- Add DefaultFolderPushTrackingRepository with observeEnabled, isEnabled, and disable methods
- Add FolderPushTrackingRepository interface to folder API
- Add FolderError sealed interface with AccountNotFound, NotFound, and Unavailable cases
…pository

- Replace FolderRepository push methods with FolderPushTrackingRepository delegation
- Update PushController to use new repository methods (disable, isEnabled, observeEnabled)
- Make AccountBackendPusherCallback.onPushNotSupported suspend function
- Add AggregateRepositories to compose FolderRepository dependencies
…lsRepository

- Add FolderDetailsRepository interface with findById and update methods
- Add DefaultFolderDetailsRepository implementation
- Remove folder details methods from FolderRepository interface
- Update FolderSettingsViewModel and FolderSettingsDataStore to use FolderDetailsRepository
- Update DefaultSpecialFolderUpdater to use partial updates via new repository
- Add PartialUpdatableFolderDetails for granular folder property updates
- Add FailedPrecondition error case to FolderError
- Add HasPii annotation to mark classes containing sensitive user data
- Add Ignore annotation to exclude properties from logging
- Add Mask annotation to redact sensitive values in log output
- Annotate Folder and FolderDetails with @LoggingPii.HasPii
- Annotate Folder.name with @LoggingPii.Mask
- Add toStringPiiSafe() extension functions for safe logging
- Update DefaultFolderDetailsRepository to use PII-safe logging
- Reformat multi-line log messages for consistency
…alUpdatableFolderDetails

The `integrate` property was redundant with `includeInUnifiedInbox` and has been replaced throughout the codebase.
@rafaeltonholo
rafaeltonholo force-pushed the chore/11470/split-folder-repository branch from b0b642b to 8101bf8 Compare September 3, 2026 14:16
@wmontwe
wmontwe self-requested a review September 4, 2026 09:12

@wmontwe wmontwe left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@rafaeltonholo
rafaeltonholo merged commit f19d7b0 into main Sep 4, 2026
20 checks passed
@rafaeltonholo
rafaeltonholo deleted the chore/11470/split-folder-repository branch September 4, 2026 10:49
@thunderbird-botmobile thunderbird-botmobile Bot added this to the Thunderbird 25 milestone Sep 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

report: exclude Exclude changes from user-facing reports (internal, minor, or not relevant to users). tb-team Tasks and features handled by project maintainers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants