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

Room admins can change user roles #2423

Merged
merged 8 commits into from
Mar 5, 2024

Conversation

jmartinesp
Copy link
Contributor

@jmartinesp jmartinesp commented Feb 21, 2024

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other :

Content

  • Add 'roles and permissions' screen to room details when the user is an admin in that room.
  • Rearrange room details items a bit to match designs.
  • Add 'change roles' screen that allows to batch promote/demote users from admin and moderator roles (admins can only add new admins, not remove then).
  • Add option to demote yourself from admin to user or moderator roles.

Motivation and context

#2257

Screenshots / GIFs

In the PR.

Tests

  • Go into the details screen of a room where you're an admin.
  • Select either admins or moderators items.
  • Try adding and removing users from that role.
  • Change your role to either moderator or user. You will exit the roles and permissions screen automatically when it succeeds.

Tested devices

  • Physical
  • Emulator
  • OS version(s): 14

Checklist

Copy link
Contributor

github-actions bot commented Feb 21, 2024

📱 Scan the QR code below to install the build (arm64 only) for this PR.
QR code
If you can't scan the QR code you can install the build via this link: https://i.diawi.com/5r8nhr

- Add a 'roles and permissions' option for each room.
- Allow promoting users to admins, adding or removing moderators, and demote yourself if you're and admin.
@jmartinesp jmartinesp force-pushed the feature/jme/2257-change-user-role branch from 99d8197 to da4f77c Compare March 5, 2024 13:22
@jmartinesp jmartinesp added the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Mar 5, 2024
@github-actions github-actions bot removed the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Mar 5, 2024
@jmartinesp jmartinesp marked this pull request as ready for review March 5, 2024 13:38
@jmartinesp jmartinesp requested a review from a team as a code owner March 5, 2024 13:38
@jmartinesp jmartinesp requested review from ganfra and removed request for a team March 5, 2024 13:38
@jmartinesp jmartinesp added the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Mar 5, 2024
@github-actions github-actions bot removed the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Mar 5, 2024
@ElementBot
Copy link
Collaborator

ElementBot commented Mar 5, 2024

Warnings
⚠️

features/createroom/impl/src/main/kotlin/io/element/android/features/createroom/impl/configureroom/ConfigureRoomView.kt#L33 - Using a material import while also using the material3 library

⚠️

features/createroom/impl/src/main/kotlin/io/element/android/features/createroom/impl/configureroom/ConfigureRoomView.kt#L34 - Using a material import while also using the material3 library

Generated by 🚫 dangerJS against 1c61b85

Copy link
Contributor

@ganfra ganfra left a comment

Choose a reason for hiding this comment

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

Some small remarks otherwise LGTM!

@@ -35,7 +35,25 @@ data class RoomMember(
enum class Role {
Copy link
Contributor

Choose a reason for hiding this comment

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

Integrate values directly in the enum?

}
val exitState: MutableState<AsyncAction<Unit>> = remember { mutableStateOf(AsyncAction.Uninitialized) }
val saveState: MutableState<AsyncAction<Unit>> = remember { mutableStateOf(AsyncAction.Uninitialized) }
val usersWithRole = produceState(initialValue = persistentListOf()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Create an extension method on room?

@@ -71,6 +71,9 @@ class RoomDetailsPresenter @Inject constructor(
val leaveRoomState = leaveRoomPresenter.present()
val canShowNotificationSettings = remember { mutableStateOf(false) }
val roomInfo by room.roomInfoFlow.collectAsState(initial = null)
val isUserAdmin by produceState(initialValue = false) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This should probably be an extension method on room within the matrix-ui module.
Also there is no key given to produceState so this won't be live? is it ok? maybe use roomInfo.userPowerLevels as key?

Copy link

codecov bot commented Mar 5, 2024

Codecov Report

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

Project coverage is 72.94%. Comparing base (38cea8e) to head (1c61b85).

Files Patch % Lines
...rolesandpermissions/changeroles/ChangeRolesView.kt 68.14% 23 Missing and 20 partials ⚠️
...mpl/rolesandpermissions/RolesAndPermissionsView.kt 68.11% 11 Missing and 11 partials ⚠️
...droid/features/roomdetails/impl/RoomDetailsView.kt 74.35% 0 Missing and 10 partials ⚠️
...andpermissions/changeroles/ChangeRolesPresenter.kt 88.60% 4 Missing and 5 partials ⚠️
...olesandpermissions/RolesAndPermissionsPresenter.kt 82.75% 2 Missing and 3 partials ⚠️
...mpl/rolesandpermissions/RolesAndPermissionsNode.kt 0.00% 3 Missing ⚠️
...ndroid/libraries/matrix/ui/room/MatrixRoomState.kt 25.00% 1 Missing and 2 partials ⚠️
...nt/android/libraries/matrix/api/room/MatrixRoom.kt 75.00% 0 Missing and 2 partials ⚠️
...droid/libraries/matrix/test/room/FakeMatrixRoom.kt 77.77% 2 Missing ⚠️
...atures/createroom/impl/components/SearchUserBar.kt 50.00% 0 Missing and 1 partial ⚠️
... and 3 more
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2423      +/-   ##
===========================================
+ Coverage    72.82%   72.94%   +0.11%     
===========================================
  Files         1374     1386      +12     
  Lines        32723    33171     +448     
  Branches      6358     6432      +74     
===========================================
+ Hits         23829    24195     +366     
- Misses        5585     5619      +34     
- Partials      3309     3357      +48     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jmartinesp jmartinesp added the Run-Maestro Starts a Maestro Cloud session to run integration tests label Mar 5, 2024
@github-actions github-actions bot removed the Run-Maestro Starts a Maestro Cloud session to run integration tests label Mar 5, 2024
Copy link

sonarcloud bot commented Mar 5, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@jmartinesp jmartinesp enabled auto-merge (squash) March 5, 2024 15:37
@jmartinesp
Copy link
Contributor Author

Maestro failed twice by 2 unrelated emulator issues, yay. I hope third time's a charm 🤞 .

@jmartinesp jmartinesp merged commit b9d902e into develop Mar 5, 2024
18 checks passed
@jmartinesp jmartinesp deleted the feature/jme/2257-change-user-role branch March 5, 2024 16:46
@jmartinesp jmartinesp mentioned this pull request Mar 6, 2024
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants