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

chore: extract send message to viemodel #2824

Merged
merged 7 commits into from
Mar 27, 2024

Conversation

Garzas
Copy link
Contributor

@Garzas Garzas commented Mar 26, 2024


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?

Extracted send message functionality to separate view model to have it in future reusable

Copy link

Ups 🫰🟨

This PR is too big. Please try to break it up into smaller PRs.

Copy link
Contributor

github-actions bot commented Mar 26, 2024

Test Results

878 tests  +2   878 ✅ +2   14m 8s ⏱️ +47s
120 suites +2     0 💤 ±0 
120 files   +2     0 ❌ ±0 

Results for commit 94cb684. ± Comparison against base commit de629ad.

This pull request removes 24 and adds 26 tests. Note that renamed tests count towards both.
com.wire.android.ui.home.conversations.MessageComposerViewModelTest ‑ given a user picks an image asset larger than 15MB, when invoked, then sendAssetMessageUseCase isn't called()
com.wire.android.ui.home.conversations.MessageComposerViewModelTest ‑ given a valid observed enforced self-deleting message timer, when invoked, then the timer gets successfully updated()
com.wire.android.ui.home.conversations.MessageComposerViewModelTest ‑ given attachment picked and null when getting asset bundle from uri, then show message to user()
com.wire.android.ui.home.conversations.MessageComposerViewModelTest ‑ given that a free user picks an asset larger than 25MB, when invoked, then sendAssetMessageUseCase isn't called()
com.wire.android.ui.home.conversations.MessageComposerViewModelTest ‑ given that a team user sends an asset message larger than 25MB, when invoked, then sendAssetMessageUseCase is called()
com.wire.android.ui.home.conversations.MessageComposerViewModelTest ‑ given that a user picks too large asset that needs saving if invalid, when invoked, then saveToExternalMediaStorage is called()
com.wire.android.ui.home.conversations.MessageComposerViewModelTest ‑ given that a user sends an ping message, when invoked, then sendKnockUseCase and pingRinger are called()
com.wire.android.ui.home.conversations.MessageComposerViewModelTest ‑ given that a user updates the self-deleting message timer, when invoked, then the timer gets successfully updated()
com.wire.android.ui.home.conversations.MessageComposerViewModelTest ‑ given that user chose to dismiss when enabled legal hold before sending, then message is not sent and dialog hidden()
com.wire.android.ui.home.conversations.MessageComposerViewModelTest ‑ given that user chose to dismiss when enabled legal hold when sending fails, then message is not resent and dialog hidden()
…
com.wire.android.ui.home.conversations.composer.MessageComposerViewModelTest ‑ given a valid observed enforced self-deleting message timer, when invoked, then the timer gets successfully updated()
com.wire.android.ui.home.conversations.composer.MessageComposerViewModelTest ‑ given that a user updates the self-deleting message timer, when invoked, then the timer gets successfully updated()
com.wire.android.ui.home.conversations.composer.MessageComposerViewModelTest ‑ given that user saves a draft message, then save draft use case is triggered()
com.wire.android.ui.home.conversations.composer.MessageComposerViewModelTest ‑ given that user types a text message, when invoked typing invoked, then send typing event is called()
com.wire.android.ui.home.conversations.sendmessage.SendMessageViewModelTest ‑ given a user picks an too large image asset, when invoked, then sendAssetMessageUseCase isn't called()
com.wire.android.ui.home.conversations.sendmessage.SendMessageViewModelTest ‑ given attachment picked and error when handling asset from uri, then show message to user()
com.wire.android.ui.home.conversations.sendmessage.SendMessageViewModelTest ‑ given that a free user picks an asset larger than 25MB, when invoked, then sendAssetMessageUseCase isn't called()
com.wire.android.ui.home.conversations.sendmessage.SendMessageViewModelTest ‑ given that a user sends an ping message, when invoked, then sendKnockUseCase and pingRinger are called()
com.wire.android.ui.home.conversations.sendmessage.SendMessageViewModelTest ‑ given that user chose to dismiss when enabled legal hold before sending, then message is not sent and dialog hidden()
com.wire.android.ui.home.conversations.sendmessage.SendMessageViewModelTest ‑ given that user chose to dismiss when enabled legal hold when sending fails, then message is not resent and dialog hidden()
…

♻️ This comment has been updated with latest results.

@AndroidBob
Copy link
Collaborator

Build 3768 failed.

Base automatically changed from chore/move-delete-message-to-messages-view-model to develop March 26, 2024 15:48
@AndroidBob
Copy link
Collaborator

Build 3771 failed.

Copy link

codecov bot commented Mar 27, 2024

Codecov Report

Attention: Patch coverage is 53.75723% with 80 lines in your changes are missing coverage. Please review.

Project coverage is 43.76%. Comparing base (de629ad) to head (94cb684).

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2824      +/-   ##
===========================================
+ Coverage    43.63%   43.76%   +0.13%     
===========================================
  Files          419      421       +2     
  Lines        14022    14019       -3     
  Branches      2535     2537       +2     
===========================================
+ Hits          6118     6135      +17     
+ Misses        7186     7164      -22     
- Partials       718      720       +2     
Files Coverage Δ
...rsations/messages/ConversationMessagesViewModel.kt 64.64% <100.00%> (+0.19%) ⬆️
...android/ui/home/conversations/model/AssetBundle.kt 100.00% <100.00%> (ø)
...ome/conversations/usecase/HandleUriAssetUseCase.kt 96.96% <96.96%> (ø)
.../conversations/sendmessage/SendMessageViewModel.kt 83.76% <78.57%> (ø)
...c/main/kotlin/com/wire/android/util/FileManager.kt 7.14% <0.00%> (-4.40%) ⬇️
.../com/wire/android/ui/sharing/ImportedMediaAsset.kt 0.00% <0.00%> (ø)
...conversations/composer/MessageComposerViewModel.kt 72.72% <72.72%> (ø)
...id/ui/sharing/ImportMediaAuthenticatedViewModel.kt 0.00% <0.00%> (ø)

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 de629ad...94cb684. Read the comment docs.

Copy link
Contributor

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

@AndroidBob
Copy link
Collaborator

Build 3779 succeeded.

The build produced the following APK's:

@@ -130,8 +131,8 @@ fun ConversationMediaScreen(
)

SnackBarMessage(
messageComposerViewModel.infoMessage,
conversationMessagesViewModel.infoMessage
composerMessages = MutableSharedFlow<SnackBarMessage>().asSharedFlow(),
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 we be passing a shared flow variable here correctly so it can be listened to? this just seems like a new MutableSharedFlow but no one is emitting to it? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's used in 2 places where in one place it doesn't have second info message. I will try to figure better way to do it 💪

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe just make this parameter nullable 😄

Copy link
Contributor

@alexandreferris alexandreferris left a comment

Choose a reason for hiding this comment

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

Great work! I just have a tiny question 🤔

Copy link
Contributor

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

@AndroidBob
Copy link
Collaborator

Build 3795 succeeded.

The build produced the following APK's:

@Garzas Garzas enabled auto-merge March 27, 2024 14:56
@Garzas Garzas added this pull request to the merge queue Mar 27, 2024
Copy link
Contributor

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

Merged via the queue into develop with commit fec0ad9 Mar 27, 2024
13 of 14 checks passed
@Garzas Garzas deleted the chore/extract-send-message-to-viemodel branch March 27, 2024 15:40
@AndroidBob
Copy link
Collaborator

Build 3798 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
Development

Successfully merging this pull request may close these issues.

5 participants