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: support Proteus federation if MLS not supported by backend (WPB-14250) #3126

Merged

Conversation

yamilmedina
Copy link
Contributor

@yamilmedina yamilmedina commented Nov 25, 2024

StoryWPB-14250 [Android] implement fall guards for CC migration


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?

Issues

We need to support federation search, according to if MLS is enabled, or if it is configured or not in the backend.

Solutions

Create a new use case centralizing the logic to decide if we need to support federation or not for this: team/conversation/backend.

Before the logic was in the view model in AR, but here is centralized and we can test all the cases for it, therefore now the view model just calls this use case to decide if we need to enable cross domain search.

Dependencies (Optional)

Needs releases with:

  • GitHub link to other pull request

Testing

Test Coverage (Optional)

  • I have added automated test to this contribution

PR Post Submission Checklist for internal contributors (Optional)

  • Wire's Github Workflow has automatically linked the PR to a JIRA issue

PR Post Merge Checklist for internal contributors

  • If any soft of configuration variable was introduced by this PR, it has been added to the relevant documents and the CI jobs have been updated.

References
  1. https://sparkbox.com/foundry/semantic_commit_messages
  2. https://github.com/wireapp/.github#usage
  3. E.g. feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764.

Comment on lines +69 to +80
private suspend fun isConversationProtocolAbleToFederate(conversationId: ConversationId?): Boolean {
val isProteusTeam = getDefaultProtocol() == SupportedProtocol.PROTEUS
val isOtherDomainAllowed: Boolean = conversationId?.let {
when (val result = getConversationProtocolInfo(it)) {
is GetConversationProtocolInfoUseCase.Result.Failure -> !isProteusTeam

is GetConversationProtocolInfoUseCase.Result.Success ->
!isProteusTeam && result.protocolInfo !is Conversation.ProtocolInfo.Proteus
}
} ?: !isProteusTeam
return isOtherDomainAllowed
}
Copy link
Contributor Author

@yamilmedina yamilmedina Nov 25, 2024

Choose a reason for hiding this comment

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

Note: Logic migrated as-is to preserve retro compatibility with, plus added tests for this here too

Copy link
Contributor

github-actions bot commented Nov 25, 2024

Test Results

3 269 tests  +5   3 162 ✅ +5   4m 56s ⏱️ +28s
  556 suites +1     107 💤 ±0 
  556 files   +1       0 ❌ ±0 

Results for commit 3efc683. ± Comparison against base commit 7440f44.

♻️ This comment has been updated with latest results.

@datadog-wireapp
Copy link

Datadog Report

Branch report: feat/support-proteus-federation-if-mls-disabled
Commit report: e68b737
Test service: kalium-jvm

✅ 0 Failed, 3162 Passed, 107 Skipped, 35.52s Total Time

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 60.00000% with 12 lines in your changes missing coverage. Please review.

Project coverage is 52.64%. Comparing base (7440f44) to head (3efc683).

Files with missing lines Patch % Lines
...feature/search/IsFederationSearchAllowedUseCase.kt 72.00% 2 Missing and 5 partials ⚠️
...om/wire/kalium/logic/feature/search/SearchScope.kt 0.00% 5 Missing ⚠️
Additional details and impacted files
@@                Coverage Diff                 @@
##           release/candidate    #3126   +/-   ##
==================================================
  Coverage              52.64%   52.64%           
==================================================
  Files                   1321     1322    +1     
  Lines                  51615    51645   +30     
  Branches                4781     4791   +10     
==================================================
+ Hits                   27172    27191   +19     
- Misses                 22487    22493    +6     
- Partials                1956     1961    +5     
Files with missing lines Coverage Δ
...om/wire/kalium/logic/feature/search/SearchScope.kt 0.00% <0.00%> (ø)
...feature/search/IsFederationSearchAllowedUseCase.kt 72.00% <72.00%> (ø)

... and 1 file 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 7440f44...3efc683. Read the comment docs.

---- 🚨 Try these New Features:

@MohamadJaara MohamadJaara added this pull request to the merge queue Nov 27, 2024
Merged via the queue into release/candidate with commit 4d334d5 Nov 27, 2024
24 checks passed
@MohamadJaara MohamadJaara deleted the feat/support-proteus-federation-if-mls-disabled branch November 27, 2024 09:25
github-actions bot pushed a commit that referenced this pull request Nov 27, 2024
…-14250) (#3126)

* chore: use case template

* feat: usecase for allowing federated search

* feat: add usecase to check for federation search allowance

* feat: cleanup

* feat: adjust tests to new implementation

* feat: adjust tests to new implementation
github-merge-queue bot pushed a commit that referenced this pull request Nov 29, 2024
…-14250) (#3126) (#3132)

* chore: use case template

* feat: usecase for allowing federated search

* feat: add usecase to check for federation search allowance

* feat: cleanup

* feat: adjust tests to new implementation

* feat: adjust tests to new implementation

Co-authored-by: Yamil Medina <[email protected]>
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.

5 participants