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(mls): unify MLSClientIdentity models (WPB-9774) #2818

Merged

Conversation

mchenani
Copy link
Member


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?

To fetch the MLSClientIdentity from CC we were mapping them in different models, due to different changes we needed more data to be exposed, we needed them to be unified and map all the available data from CC.

Issues

Hard to maintain different models
Due to maturity of CC now we have a rich and solid model we can map full object in Kalium and Android App.

Needs releases with:
With AR

  • GitHub link to other pull request

Testing

Test Coverage (Optional)

  • I have added automated test to this contribution

How to Test

All E2EI features must work as before!


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.

mlsConversationRepository.getMembersIdentities(conversationId, userIds).fold(
{ mapOf() },
{
it.mapValues { (_, identities) ->
identities.getUserCertificateStatus()
// todo: we need to check the user name and details!
Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately this was missed from the main code; I noticed it here! I don't want to mix the changes for this one in the PR [it's big already] so it's coming in another PR!

Copy link
Contributor

github-actions bot commented Jun 18, 2024

Test Results

2 891 tests   2 873 ✔️  3m 52s ⏱️
       7 suites       18 💤
       7 files           0

Results for commit 23a09d6.

♻️ This comment has been updated with latest results.

@datadog-wireapp
Copy link

datadog-wireapp bot commented Jun 18, 2024

Datadog Report

All test runs 983e7a1 🔗

2 Total Test Services: 0 Failed, 2 Passed

Test Services
Service Name Failed Known Flaky New Flaky Passed Skipped Total Time Test Service View
kalium-ios 0 0 0 2768 123 1.58s Link
kalium-jvm 0 0 0 2909 107 6.46s Link

@mchenani mchenani marked this pull request as ready for review June 19, 2024 07:59
@mchenani mchenani changed the title chore(mls): unify MLSClientIdentity models chore(mls): unify MLSClientIdentity models (WPB-9774) Jun 20, 2024
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! Now it looks much better and understendable :)

@codecov-commenter
Copy link

codecov-commenter commented Jun 26, 2024

Codecov Report

Attention: Patch coverage is 80.80808% with 19 lines in your changes missing coverage. Please review.

Please upload report for BASE (release/android-cycle-4.6@c7792e3). Learn more about missing BASE report.

Additional details and impacted files
@@                     Coverage Diff                      @@
##             release/android-cycle-4.6    #2818   +/-   ##
============================================================
  Coverage                             ?   58.44%           
  Complexity                           ?        7           
============================================================
  Files                                ?     1184           
  Lines                                ?    46648           
  Branches                             ?     4437           
============================================================
  Hits                                 ?    27264           
  Misses                               ?    17406           
  Partials                             ?     1978           
Files Coverage Δ
...monMain/kotlin/com/wire/kalium/cryptography/IDs.kt 89.58% <100.00%> (ø)
.../data/conversation/DecryptedMessageBundleMapper.kt 50.00% <100.00%> (ø)
...gic/data/conversation/MLSConversationRepository.kt 83.85% <100.00%> (ø)
.../kotlin/com/wire/kalium/logic/di/MapperProvider.kt 91.66% <ø> (ø)
.../feature/e2ei/usecase/GetE2EICertificateUseCase.kt 100.00% <100.00%> (ø)
.../e2ei/usecase/GetUserE2EIAllCertificatesUseCase.kt 100.00% <100.00%> (ø)
...ture/e2ei/usecase/GetUserE2EICertificateUseCase.kt 100.00% <100.00%> (ø)
...m/logic/feature/user/ObserveE2EIRequiredUseCase.kt 97.72% <100.00%> (ø)
...in/kotlin/com/wire/kalium/logic/data/id/PlainId.kt 0.00% <0.00%> (ø)
...otlin/com/wire/kalium/logic/data/id/QualifiedId.kt 60.86% <66.66%> (ø)
... and 6 more

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 c7792e3...23a09d6. Read the comment docs.

@datadog-wireapp
Copy link

datadog-wireapp bot commented Jul 16, 2024

Datadog Report

All test runs b456f95 🔗

2 Total Test Services: 0 Failed, 2 Passed

Test Services
Service Name Failed Known Flaky New Flaky Passed Skipped Total Time Test Service View
kalium-ios 0 0 0 2778 123 1.31s Link
kalium-jvm 0 0 0 2919 107 5.3s Link

Copy link

sonarcloud bot commented Jul 17, 2024

@mchenani mchenani enabled auto-merge (squash) July 17, 2024 15:32
@mchenani mchenani disabled auto-merge July 17, 2024 15:32
@mchenani mchenani merged commit 8f000c0 into release/android-cycle-4.6 Jul 18, 2024
19 checks passed
@mchenani mchenani deleted the fix/unify-mls-client-identity-models branch July 18, 2024 07:21
@echoes-hq echoes-hq bot added the echoes: throughput Changes intended at preserving our ability to evolve the software safely and effectively label Jul 18, 2024
mchenani added a commit that referenced this pull request Jul 25, 2024
* chore: refactor identity models

* fix tests

* user correct clientId and Handle in MLSClientIdentity object

* clean mapping object checker

* fix formatting and remove one line un-used code

---------

Co-authored-by: Vitor Hugo Schwaab <[email protected]>

(cherry picked from commit 8f000c0)
mchenani added a commit that referenced this pull request Jul 25, 2024
* chore: refactor identity models

* fix tests

* user correct clientId and Handle in MLSClientIdentity object

* clean mapping object checker

* fix formatting and remove one line un-used code

---------

Co-authored-by: Vitor Hugo Schwaab <[email protected]>

(cherry picked from commit 8f000c0)
github-actions bot pushed a commit that referenced this pull request Jul 25, 2024
* chore: refactor identity models

* fix tests

* user correct clientId and Handle in MLSClientIdentity object

* clean mapping object checker

* fix formatting and remove one line un-used code

---------

Co-authored-by: Vitor Hugo Schwaab <[email protected]>

(cherry picked from commit 8f000c0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
echoes: throughput Changes intended at preserving our ability to evolve the software safely and effectively 🚨 Potential breaking changes 👕 size: XXL type: chore 🧹
Projects
None yet
4 participants