Skip to content

103-bug-notes-not-visible-after-deleting-a-tag-used-for-filtering#111

Merged
mshdabiola merged 18 commits into
developfrom
103-bug-notes-not-visible-after-deleting-a-tag-used-for-filtering
Jun 10, 2025
Merged

103-bug-notes-not-visible-after-deleting-a-tag-used-for-filtering#111
mshdabiola merged 18 commits into
developfrom
103-bug-notes-not-visible-after-deleting-a-tag-used-for-filtering

Conversation

@mshdabiola

@mshdabiola mshdabiola commented Jun 10, 2025

Copy link
Copy Markdown
Owner

PR Type

Enhancement


Description

  • Added MainData sealed class to manage main screen type.

  • Updated repositories and view models to support MainData.

  • Improved UI to reflect the selected MainData.

  • Removed unused code and improved code style.


Changes walkthrough 📝

Relevant files
Enhancement
14 files
MainActivityViewModel.kt
Added setMainData function and updated uiState flow           
+9/-2     
SharedActivityViewModel.kt
Added mainData to SharedActivityUiState                                   
+2/-0     
NoteApp.kt
Updated NoteApp to use MainData and removed unused code   
+7/-10   
LabelViewModel.kt
Updated onDelete function to set MainData                               
+4/-0     
MainState.kt
Added mainData to MainState                                                           
+2/-0     
MainViewModel.kt
Updated MainViewModel to use MainData and removed unused code
+20/-8   
OfflineFirstUserDataRepository.kt
Added setMainData function                                                             
+4/-0     
UserDataRepository.kt
Added setMainData function                                                             
+1/-0     
UserPreferencesRepository.kt
Added setMainData function and updated userData flow         
+13/-0   
MainData.kt
Created MainData sealed class                                                       
+9/-0     
UserData.kt
Added mainData property                                                                   
+1/-0     
FakeUserDataRepository.kt
Added setMainData function                                                             
+4/-0     
TestUserDataRepository.kt
Added setMainData function and updated userData flow         
+8/-0     
user_preferences.proto
Added main_screen_type field                                                         
+2/-1     
Refactoring
7 files
NoteAppState.kt
Removed unused code and improved code style                           
+4/-6     
MainNavigation.kt
Removed unused code and improved code style                           
+2/-33   
ChangeListVersions.kt
Removed unused file                                                                           
+0/-13   
IntToStringIdsMigration.kt
Removed unused file                                                                           
+0/-23   
DataStoreModule.kt
Removed unused code and improved code style                           
+0/-4     
IntToStringIdsMigrationTest.kt
Removed unused file                                                                           
+0/-18   
ListToMapMigrationTest.kt
Removed unused file                                                                           
+0/-60   
Tests
1 files
OfflineFirstUserDataRepositoryTest.kt
Updated test to include MainData                                                 
+2/-0     

Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • Added a new property `mainData` of type `MainData` to the `UserData` data class.
    Created a new sealed class `MainData` with the following subtypes:
    - `Note`
    - `Achieve`
    - `Trash`
    - `Label` (data class with an `id` property)
    - `Remainder`
    
    Each subtype of `MainData` has an `index` property.
    This commit introduces a new field `main_screen_type` to the `UserPreferences` proto and updates `UserPreferencesRepository` to support managing the main screen's data type.
    
    - Added `main_screen_type` (int64) to `user_preferences.proto`.
    - Updated `UserPreferencesRepository` to map the stored `main_screen_type` integer to the corresponding `MainData` sealed class (Note, Achieve, Trash, Remainder, Label).
    - Added `setMainData` function to `UserPreferencesRepository` to update the `main_screen_type` in DataStore.
    Added a new field `mainData` to the `MainActivityUiState` data class in `SharedActivityViewModel.kt`. This field is of type `MainData` and defaults to `MainData.Note`.
    
    The `mainData` field will be used to determine which main screen content to display (e.g., Notes, Labels).
    Added a trailing comma to the `mainData` property in the `UserData` data class. This is a stylistic change for consistency.
    
    Also, removed an extra newline from the end of `MainData.kt`.
    The `setMainData` function in `UserPreferencesRepository.kt` now accepts a `Long` parameter named `index` instead of a `MainData` object.
    
    The function's implementation was updated to directly use the `index` parameter when setting the `mainScreenType` in user preferences.
    This commit introduces a new function `setMainData(mainData: Long)` to the `UserDataRepository` interface and its implementation `OfflineFirstUserDataRepository`.
    
    This function allows setting a `Long` value, presumably related to main application data, which is then stored via the `userPreferencesRepository`.
    
    - Added `suspend fun setMainData(mainData: Long)` to `UserDataRepository.kt`
    - Implemented `setMainData` in `OfflineFirstUserDataRepository.kt` to call `userPreferencesRepository.setMainData(mainData)`
    Removed the `TypeArg` navigation argument from `MainRoute`. The `MainViewModel` now retrieves the main data type from `UserDataRepository` instead of `SavedStateHandle`.
    
    This change simplifies the navigation to `MainScreen` and centralizes the logic for determining the content to display.
    
    Files affected:
    - `feature/main/src/main/kotlin/com/mshdabiola/main/navigation/MainNavigation.kt`
    - `feature/main/src/main/kotlin/com/mshdabiola/main/MainViewModel.kt`
    This commit introduces changes to how main data (e.g., Note, Checklist) is handled and improves navigation within the application.
    
    - Modified `SharedActivityViewModel` to correctly initialize `mainData` in `UserData`.
    - Added `setMainData` function to `MainActivityViewModel` to allow updating the main data type (Note or Checklist) in `UserDataRepository`.
    - Updated `NoteApp` to:
        - Use `uiState` from `MainActivityViewModel` to determine the current `mainData` for the `MainNavigation` drawer.
        - Call `viewModel.setMainData(it)` when a navigation item in the drawer is selected.
        - Navigate to the main screen without passing the type argument, as this is now handled by `setMainData`.
    - Removed the `mainArg` property from `NoteAppState` as it's no longer needed for determining the main screen's content type.
    @github-actions

    Copy link
    Copy Markdown
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Unclear Logic

    The logic for handling mainData in MainActivityViewModel needs further clarification. The purpose and interaction with other components should be more explicitly documented.

    fun setMainData(mainData: Long) {
        viewModelScope.launch {
            userDataRepository.setMainData(mainData)
        }
    }
    Potential NPE

    The when statement in MainViewModel handling mainArg.value might throw a NullPointerException if mainArg.value is null. Consider adding a null check or handling this case appropriately.

    when (mainArg.value) {
        NoteType.LABEL -> {
            notepad.filter { it.labels.any { it.id == mainArg.type } }
        }
    Data Handling

    The way MainData is handled and converted in UserPreferencesRepository requires careful review to ensure data consistency and prevent unexpected behavior. The when statement should be thoroughly tested for all possible scenarios.

    mainData = when (it.mainScreenType) {
        MainData.Note.index -> MainData.Note
        MainData.Achieve.index -> MainData.Achieve
        MainData.Trash.index -> MainData.Trash
        MainData.Remainder.index -> MainData.Remainder
        else -> MainData.Label(it.mainScreenType)
    }

    @github-actions

    Copy link
    Copy Markdown
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Handle potential null UI state

    The code uses a safe-call operator (?.) on uiState which might return null. Handle
    the potential null case explicitly to prevent crashes. Consider using a when
    statement to handle different MainActivityUiState types or provide a default value.

    app/src/main/java/com/mshdabiola/playnotepad/ui/NoteApp.kt [74]

     val uiState by viewModel.uiState.collectAsStateWithLifecycle()
     var showAudio by remember { mutableStateOf(false) }
     var showImage by remember { mutableStateOf(false) }
    +val currentMainArg = when (uiState) {
    +    is MainActivityUiState.Success -> uiState.userData.mainData.index
    +    else -> MainData.Note.index // Or another suitable default
    +}
    Suggestion importance[1-10]: 7

    __

    Why: The suggestion correctly points out a potential null pointer exception. Handling the null case with a when statement improves the code's robustness. The score is not higher because the existing code is already functional, but the suggestion improves its safety.

    Medium
    General
    Improve state flow sharing

    The SharingStarted.WhileSubscribed(5000) parameter in stateIn might cause unexpected
    behavior. Consider using SharingStarted.Eagerly or SharingStarted.WhileSubscribed()
    without the timeout to ensure consistent data updates. The timeout could lead to
    stale data if the flow is not actively collected.

    feature/main/src/main/kotlin/com/mshdabiola/main/MainViewModel.kt [61-63]

     private val mainArg = userDataRepository.userData
         .mapLatest { it.mainData }
    -    .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5000), MainData.Note)
    +    .stateIn(viewModelScope, SharingStarted.WhileSubscribed(), MainData.Note)
    Suggestion importance[1-10]: 5

    __

    Why: The suggestion to remove the timeout from SharingStarted is a valid optimization. While the impact might not be immediately significant, removing the timeout can prevent potential issues with stale data and improve responsiveness.

    Low
    Use positive indices

    The MainData indices are negative for some objects. While this might work, using
    positive indices would improve code readability and reduce potential confusion.
    Consider adjusting the indices to be positive integers.

    modules/model/src/main/java/com/mshdabiola/model/MainData.kt [3-8]

     sealed class MainData(val index: Long) {
    -    object Note : MainData(-1L)
    -    object Achieve : MainData(-2L)
    -    object Trash : MainData(-3L)
    -    data class Label(val id: Long) : MainData(0L) 
    -    object Remainder : MainData(-4L)
    +    object Note : MainData(1L)
    +    object Achieve : MainData(2L)
    +    object Trash : MainData(3L)
    +    data class Label(val id: Long) : MainData(id) 
    +    object Remainder : MainData(4L)
     }
    Suggestion importance[1-10]: 4

    __

    Why: Using positive indices for MainData improves readability and reduces the chance of confusion. The impact is moderate because the current negative indices don't cause immediate errors, but the change enhances code clarity.

    Low

    This commit introduces `MainData` into the `MainState` to manage the type of notes being displayed (e.g., standard notes, labeled notes, reminders).
    
    - Added `mainData: MainData` property to `MainState.Success`.
    - `MainViewModel` now observes `userDataRepository.userData` to update `mainData` in `MainState.Success`.
    - Logic for filtering notes in `MainViewModel` now uses `mainState.mainData` instead of a separate `mainArg` StateFlow.
    - When `MainState` is initialized to `Success` with an empty list, it now also sets the initial `mainData` fetched from `userDataRepository`.
    When a label is deleted in `LabelViewModel`, the `userDataRepository.setMainData` method is now called with `MainData.Note.index`. This ensures that the main screen is updated to reflect the change, likely by navigating to the notes screen.
    Modified `SharingStarted.WhileSubscribed` in `MainActivityViewModel.kt` to remove the 5-second timeout.
    Removed unused imports `TypeArg` and `NoteType` from `NoteAppState.kt`.
    
    - `SharingStarted.WhileSubscribed(5_000)` -> `SharingStarted.WhileSubscribed()` in `MainActivityViewModel.kt`
    Removed the unused import `com.mshdabiola.model.NoteType` from `UserPreferencesRepository.kt`.
    This commit refactors the `MainViewModel.kt` file.
    
    - Removed unused imports: `SharingStarted` and `stateIn`.
    - Simplified conditional logic for updating `_mainState.value` based on `userDataRepository.userData`.
    - Streamlined the logic for initializing `_mainState.value` within the `init` block by directly checking the current state.
    Removed the `androidx.navigation.navOptions` import from `NoteApp.kt` as it was no longer needed.
    This commit adds the `setMainData` method to `FakeUserDataRepository` and `TestUserDataRepository`.
    
    - In `FakeUserDataRepository`, `setMainData` now calls the corresponding method in `userPreferencesRepository`.
    - In `TestUserDataRepository`, `setMainData` updates the `mainData` field of the current `UserData` to `MainData.Note` and emits the updated data.
    - The initial `UserData` in `TestUserDataRepository` now includes `mainData = MainData.Note`.
    Updated the `MainData.Label` class to use its `id` property as its `index`. Previously, it was hardcoded to `0L`.
    
    - `MainData.Label(val id: Long) : MainData(0L)` -> `MainData.Label(val id: Long) : MainData(index = id)` in `MainData.kt`
    This commit removes several unused files related to data migrations and data classes from the `datastore` module.
    
    - Deleted `ChangeListVersions.kt`.
    - Deleted `IntToStringIdsMigration.kt` and its corresponding test file `IntToStringIdsMigrationTest.kt`.
    - Deleted `ListToMapMigrationTest.kt`.
    - Removed `IntToStringIdsMigration` from the `DataStoreModule.kt` as it's no longer needed.
    This commit updates `OfflineFirstUserDataRepositoryTest.kt` to include `mainData` in the `UserData` initialization.
    
    - Added `MainData.Label(0)` as the initial value for the `mainData` property in the `UserData` object within the test setup.
    @github-actions

    Copy link
    Copy Markdown
    Contributor

    Test results

    27 tests   - 1   27 ✅  - 1   17s ⏱️ ±0s
    18 suites  - 1    0 💤 ±0 
    18 files    - 1    0 ❌ ±0 

    Results for commit d0e7e9c. ± Comparison against base commit 06dd48e.

    This pull request removes 1 test.
    com.mshdabiola.datastore.IntToStringIdsMigrationTest ‑ IntToStringIdsMigration_should_migrate_topic_ids
    

    @codecov-commenter

    codecov-commenter commented Jun 10, 2025

    Copy link
    Copy Markdown

    Codecov Report

    ❌ Patch coverage is 5.26316% with 36 lines in your changes missing coverage. Please review.
    ✅ Project coverage is 0.89%. Comparing base (06dd48e) to head (d0e7e9c).
    ⚠️ Report is 799 commits behind head on develop.

    Files with missing lines Patch % Lines
    ...c/main/kotlin/com/mshdabiola/main/MainViewModel.kt 0.00% 11 Missing ⚠️
    .../mshdabiola/datastore/UserPreferencesRepository.kt 25.00% 2 Missing and 4 partials ⚠️
    ...main/java/com/mshdabiola/playnotepad/ui/NoteApp.kt 0.00% 5 Missing ⚠️
    ...om/mshdabiola/playnotepad/MainActivityViewModel.kt 0.00% 4 Missing ⚠️
    ...biola/testing/repository/TestUserDataRepository.kt 0.00% 3 Missing ⚠️
    .../java/com/mshdabiola/labelscreen/LabelViewModel.kt 0.00% 2 Missing ⚠️
    .../mshdabiola/playnotepad/SharedActivityViewModel.kt 0.00% 1 Missing ⚠️
    ...n/src/main/kotlin/com/mshdabiola/main/MainState.kt 0.00% 1 Missing ⚠️
    ...n/com/mshdabiola/main/navigation/MainNavigation.kt 0.00% 1 Missing ⚠️
    .../data/repository/OfflineFirstUserDataRepository.kt 0.00% 1 Missing ⚠️
    ... and 1 more
    Additional details and impacted files
    @@            Coverage Diff             @@
    ##           develop    #111      +/-   ##
    ==========================================
    + Coverage     0.86%   0.89%   +0.02%     
    ==========================================
      Files          158     156       -2     
      Lines         6840    6843       +3     
      Branches       537     539       +2     
    ==========================================
    + Hits            59      61       +2     
    + Misses        6772    6769       -3     
    - Partials         9      13       +4     

    ☔ View full report in Codecov by Sentry.
    📢 Have feedback on the report? Share it here.

    🚀 New features to boost your workflow:
    • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

    @mshdabiola

    Copy link
    Copy Markdown
    Owner Author

    /describe

    @github-actions

    Copy link
    Copy Markdown
    Contributor

    PR Description updated to latest commit (d0e7e9c)

    @mshdabiola mshdabiola merged commit b0f07ca into develop Jun 10, 2025
    7 checks passed
    @mshdabiola mshdabiola deleted the 103-bug-notes-not-visible-after-deleting-a-tag-used-for-filtering branch June 10, 2025 19:03
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    [Bug]: Notes Not Visible After Deleting a Tag Used for Filtering

    2 participants