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: hide group creation and edit options for external user [WPB-5749] #2794

Merged
merged 9 commits into from
Mar 18, 2024

Conversation

saleniuk
Copy link
Contributor

@saleniuk saleniuk commented Mar 15, 2024

TaskWPB-5749 [Android] Hide group creation buttons for external users


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

External users have the possibility to create groups, add participants to existing groups and edit group name resulting in errors because they shouldn't be allowed to.

Solutions

Check if the user is external and if so then hide option to create group, add participants to the existing group and edit name of the group.
Created dedicated class ItemActionType to choose if the items can be clicked or/and checked.
Cleaned up and unified the state defining the possibilities to edit particular group options according to this table:

for given option to be allowed... ...user needs to be
add participants to group allowed group admin & not external team member
group name change allowed group admin & not external team member
group guests option change allowed group admin & team member of the group owner team
group services option change allowed group admin
self deleting option change allowed group admin
group read receipts option change allowed group admin & group created by a team member

Let me know if any of these conditions is incorrect.

Dependencies (Optional)

Needs releases with:

Testing

Test Coverage (Optional)

  • I have added automated test to this contribution

How to Test

Log in as external user and try to create group, add participants to existing group or change the name of the group.

Attachments (Optional)

Screen.Recording.2024-03-14.at.18.27.23.mov

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.

Copy link
Contributor

github-actions bot commented Mar 15, 2024

Test Results

874 tests  +14   874 ✅ +14   16m 48s ⏱️ + 4m 21s
117 suites ± 0     0 💤 ± 0 
117 files   ± 0     0 ❌ ± 0 

Results for commit 71c21b6. ± Comparison against base commit 2ff2e04.

This pull request removes 6 and adds 20 tests. Note that renamed tests count towards both.
com.wire.android.ui.home.conversations.details.GroupConversationDetailsViewModelTest ‑ given user has no teamId and conversation no teamId, when init group options, then read receipt toggle is disabled()
com.wire.android.ui.home.conversations.details.GroupConversationDetailsViewModelTest ‑ given user has no teamId, is admin and conversation has teamId, when init group options, then read receipt toggle is enabled()
com.wire.android.ui.home.conversations.details.GroupConversationDetailsViewModelTest ‑ given user has no teamId, not admin and conversation has teamId, when init group options, then read receipt toggle is enabled()
com.wire.android.ui.home.conversations.details.GroupConversationDetailsViewModelTest ‑ given user has teamId, is admin and conversation teamId, when init group options, then read receipt toggle is enabled()
com.wire.android.ui.home.conversations.details.GroupConversationDetailsViewModelTest ‑ when no guests and disable service dialog confirmed, then use case is called with the correct values()
com.wire.android.ui.home.conversations.details.GroupConversationDetailsViewModelTest ‑ when no guests and enabling services, use case is called with the correct values()
com.wire.android.ui.home.conversations.details.GroupConversationDetailsViewModelTest ‑ given user is admin and external team member, when init group options, then group name update is not allowed()
com.wire.android.ui.home.conversations.details.GroupConversationDetailsViewModelTest ‑ given user is admin and internal team member, when init group options, then group name update is allowed()
com.wire.android.ui.home.conversations.details.GroupConversationDetailsViewModelTest ‑ given user is admin and member of group owner team, when init group options, then guests update is allowed()
com.wire.android.ui.home.conversations.details.GroupConversationDetailsViewModelTest ‑ given user is admin and not member of group owner team, when init group options, then guests update is not allowed()
com.wire.android.ui.home.conversations.details.GroupConversationDetailsViewModelTest ‑ given user is admin and not team group, when init group options, then read receipts update is not allowed()
com.wire.android.ui.home.conversations.details.GroupConversationDetailsViewModelTest ‑ given user is admin and team group, when init group options, then read receipts update is allowed()
com.wire.android.ui.home.conversations.details.GroupConversationDetailsViewModelTest ‑ given user is admin, when init group options, then self deleting update is allowed()
com.wire.android.ui.home.conversations.details.GroupConversationDetailsViewModelTest ‑ given user is admin, when init group options, then services update is allowed()
com.wire.android.ui.home.conversations.details.GroupConversationDetailsViewModelTest ‑ given user is not admin and external team member, when init group options, then group name update is not allowed()
com.wire.android.ui.home.conversations.details.GroupConversationDetailsViewModelTest ‑ given user is not admin and internal team member, when init group options, then group name update is not allowed()
…

♻️ This comment has been updated with latest results.

Copy link

codecov bot commented Mar 15, 2024

Codecov Report

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

Project coverage is 43.61%. Comparing base (2ff2e04) to head (71c21b6).

Additional details and impacted files
@@             Coverage Diff              @@
##           develop    #2794       +/-   ##
============================================
+ Coverage         0   43.61%   +43.61%     
============================================
  Files            0      414      +414     
  Lines            0    13914    +13914     
  Branches         0     2519     +2519     
============================================
+ Hits             0     6069     +6069     
- Misses           0     7137     +7137     
- Partials         0      708      +708     
Files Coverage Δ
...oid/ui/home/conversations/ConversationMemberExt.kt 75.00% <ø> (ø)
...s/details/options/GroupConversationOptionsState.kt 90.90% <100.00%> (ø)
...participants/model/ConversationParticipantsData.kt 100.00% <100.00%> (ø)
...ui/home/conversations/search/SearchPeopleRouter.kt 0.00% <ø> (ø)
...i/home/newconversation/NewConversationViewModel.kt 57.14% <100.00%> (ø)
...tions/details/GroupConversationDetailsViewModel.kt 66.66% <87.50%> (ø)
...participants/GroupConversationParticipantsState.kt 91.30% <0.00%> (ø)
...ecase/ObserveParticipantsForConversationUseCase.kt 86.48% <66.66%> (ø)
...tlin/com/wire/android/migration/MigrationMapper.kt 4.34% <0.00%> (ø)
...in/kotlin/com/wire/android/model/ItemActionType.kt 0.00% <0.00%> (ø)

... and 404 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 2ff2e04...71c21b6. Read the comment docs.

Copy link
Contributor

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

@AndroidBob
Copy link
Collaborator

Build 3623 succeeded.

The build produced the following APK's:

…tions-for-external-user

# Conflicts:
#	app/src/main/kotlin/com/wire/android/ui/home/newconversation/NewConversationViewModel.kt
#	app/src/test/kotlin/com/wire/android/ui/home/newconversation/NewConversationViewModelArrangement.kt
Copy link
Contributor

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

@AndroidBob
Copy link
Collaborator

Build 3628 failed.

@AndroidBob
Copy link
Collaborator

Build 3633 failed.

Copy link
Contributor

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

@AndroidBob
Copy link
Collaborator

Build 3636 failed.

Copy link
Contributor

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

@AndroidBob
Copy link
Collaborator

Build 3637 succeeded.

The build produced the following APK's:

@saleniuk saleniuk requested review from a team, alexandreferris, borichellow, MohamadJaara, vitorhugods and Garzas and removed request for a team March 18, 2024 08:48
@saleniuk saleniuk enabled auto-merge March 18, 2024 16:25
@saleniuk saleniuk added this pull request to the merge queue Mar 18, 2024
Copy link
Contributor

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

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 18, 2024
@saleniuk saleniuk added this pull request to the merge queue Mar 18, 2024
@AndroidBob
Copy link
Collaborator

Build 3642 succeeded.

The build produced the following APK's:

Merged via the queue into develop with commit 40e0afb Mar 18, 2024
15 checks passed
@saleniuk saleniuk deleted the feat/hide-group-options-for-external-user branch March 18, 2024 17:33
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