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

feat: archived conversation list [WPB-4429] #2295

Merged
merged 11 commits into from Oct 4, 2023

Conversation

Garzas
Copy link
Contributor

@Garzas Garzas commented Oct 2, 2023

StoryWPB-4429 [UI] Create archived conversation tab and archived conversation list


PR Submission Checklist for internal contributors

  • The PR Title

    • conforms to the style of semantic commits messages¹ supported in Wire's Github Workflow²
    • contains a reference JIRA issue number like SQPIT-764
    • answers the question: If merged, this PR will: ... ³
  • The PR Description

    • is free of optional paragraphs and you have filled the relevant parts to the best of your ability

What's new in this PR?

  • archived conversation list
  • archived conversation list search
  • added test for conversation list view model
search.mp4
list.mp4

@Garzas Garzas self-assigned this Oct 2, 2023
@Garzas Garzas requested review from typfel and borichellow and removed request for a team October 2, 2023 14:20
@codecov
Copy link

codecov bot commented Oct 2, 2023

Codecov Report

Merging #2295 (d36b95a) into develop (b2e2292) will increase coverage by 0.64%.
The diff coverage is 56.36%.

@@              Coverage Diff              @@
##             develop    #2295      +/-   ##
=============================================
+ Coverage      40.72%   41.36%   +0.64%     
- Complexity      1034     1046      +12     
=============================================
  Files            323      324       +1     
  Lines          11748    11731      -17     
  Branches        1564     1554      -10     
=============================================
+ Hits            4784     4853      +69     
+ Misses          6510     6410     -100     
- Partials         454      468      +14     
Files Coverage Δ
...id/ui/home/conversationslist/ConversationRouter.kt 0.00% <ø> (ø)
...oid/ui/home/conversationslist/model/SearchQuery.kt 100.00% <100.00%> (ø)
...id/ui/sharing/ImportMediaAuthenticatedViewModel.kt 0.00% <0.00%> (ø)
...ome/conversationslist/ConversationListViewModel.kt 57.46% <53.06%> (+17.13%) ⬆️

... and 4 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b2e2292...d36b95a. Read the comment docs.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 2, 2023

Test Results

629 tests  +3   629 ✔️ +3   10m 43s ⏱️ + 4m 25s
  92 suites ±0       0 💤 ±0 
  92 files   ±0       0 ±0 

Results for commit d36b95a. ± Comparison against base commit b2e2292.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 2, 2023

APKs built during tests are available here. Scroll down to Artifacts!

@AndroidBob
Copy link
Collaborator

Build 1106 succeeded.

The build produced the following APK's:

@AndroidBob
Copy link
Collaborator

Build 1124 failed.

@@ -112,7 +112,7 @@ class WireApplication : Application(), Configuration.Provider {
.detectDiskReads()
.detectDiskWrites()
.penaltyLog()
.penaltyDeath()
// .penaltyDeath()
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't be reverted?

*/
package com.wire.android.ui.home.conversationslist.model

data class SearchQuery(val text: String, val fromArchive: Boolean)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like this boolean. For now it's enough, but later the app could also have "folders" and "contacts" later. So IMO enum class would be preferable here.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that Boris has a point. Also implementation with let's say sourceType sealed type shouldn't be much more difficult as now we only have Archive and "normal" conversations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added comment about it but I can implement it right now 😄

is SearchQueryUpdate.UpdateConversationsSource -> currentSearchQuery.copy(
text = "",
fromArchive = update.fromArchive
)
Copy link
Contributor

Choose a reason for hiding this comment

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

is SearchQueryUpdate.UpdateConversationsSource -> { 
    if (currentSearchQuery.fromArchive != update.fromArchive)
        currentSearchQuery.copy(
            text = "",
            fromArchive = update.fromArchive
        )
    else currentSearchQuery
}

Copy link
Contributor

@gongracr gongracr left a comment

Choose a reason for hiding this comment

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

Looking good in general. Just a small comment regarding code simplification on the Search query mechanism. Feel free to ignore if the final implementation turns to be much more complex and would compromise the release of the feature.

@AndroidBob
Copy link
Collaborator

Build 1137 failed.

Copy link
Contributor

@gongracr gongracr left a comment

Choose a reason for hiding this comment

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

Much better now 🦾 Thanks for changes!

Copy link
Contributor

@borichellow borichellow left a comment

Choose a reason for hiding this comment

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

great job, thanks :)

@AndroidBob
Copy link
Collaborator

Build 1139 failed.

@Garzas Garzas enabled auto-merge October 4, 2023 12:50
@Garzas Garzas added this pull request to the merge queue Oct 4, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Oct 4, 2023

APKs built during tests are available here. Scroll down to Artifacts!

Merged via the queue into develop with commit 7340e57 Oct 4, 2023
14 of 15 checks passed
@Garzas Garzas deleted the feat/archived-conversation-list branch October 4, 2023 13:26
@AndroidBob
Copy link
Collaborator

Build 1144 succeeded.

The build produced the following APK's:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants