[NEW] Manage Users in a Room(Add/Remove) #2351

Shailesh351 · 2019-05-30T11:11:19Z

@RocketChat/android

Closes #1230

Changes:

Add API for invite and remove user in Kotlin SDK ([NEW] Add invite and kick endpoints Rocket.Chat.Kotlin.SDK#249)
Permission Check for remove user (Show only when user have permission to remove)
Add Remove User Feature
Permission Check for invite user (Show only when user have permission to invite)
Add invite user feature(Using spotlight)

Screenshots or GIF for the change:

raaparin · 2019-05-31T13:11:01Z

I tried your implementation and I noticed a mistake.
Steps To Reproduce:

Add users
Write something added by the user.
To remove the user
Do not log out of the chat by the deleted user
Expected result: user is deleted and can not write in the chat, the chat screen is closed
Actual result: the user can write and read messages, but does not appear in the list of channel members. If you open some other fragment, the chat will disappear from the list of chats.

hfeky

Thank you for this great pull request! 😃 Just a small change I would mention is to add all the newly added strings in all other strings resource files as well, so that it is more clear that they need to be translated and not to be forgotten, and remove TODO Translate from the default file.

app/src/main/java/chat/rocket/android/chatroom/presentation/ChatRoomPresenter.kt

philipbrito · 2019-06-07T12:12:30Z

app/src/main/java/chat/rocket/android/helper/ChatRoomRoleHelper.kt

+import timber.log.Timber
+import javax.inject.Inject
+
+class ChatRoomRoleHelper @Inject constructor(


It is not acting as a helper class. It is mostly a presenter...

@filipedelimabrito It helps in getting ChatRoles from chatroom id. It is used in PermissionsInterector. Should I move methods in PermissionsInterector? It will make PermissionsInterector long.

philipbrito · 2019-06-07T12:14:12Z

app/src/main/java/chat/rocket/android/inviteusers/presentation/InviteUsersPresenter.kt

+
+			try {
+				for (user in usersList) {
+					try {


Why are you using try { ... again here?

@filipedelimabrito For each user it makes API call to server. And for each user we need to catch exception like user already added in the room or something went wrong

Outer try block is used for hiding loading view and showing msg after API call is made for all the user in the list using finally block.

app/src/main/java/chat/rocket/android/inviteusers/ui/InviteUsersFragment.kt

app/src/main/java/chat/rocket/android/userdetails/presentation/UserDetailsPresenter.kt

philipbrito

@Shailesh351 There are lots of issue here with the code indentation. Could you please make sure to follow our coding style?

app/src/main/java/chat/rocket/android/inviteusers/di/InviteUsersFragmentModule.kt

app/src/main/java/chat/rocket/android/helper/ChatRoomRoleHelper.kt

Shailesh351 · 2019-06-27T05:02:08Z

@filipedelimabrito Updated PR with RocketChat Coding Style. Till now I was using Tab instead of 4 Spaces. Sorry for that.

philipbrito · 2019-06-30T04:40:03Z

app/src/main/java/chat/rocket/android/inviteusers/presentation/InviteUsersPresenter.kt

+                            roomTypeOf(getChatRoomType(chatRoomId)),
+                            user.userId
+                        )
+                        stringBuilder.append("Invited : ${user.username}\n")


Hard-coded string. Please remove.

philipbrito · 2019-06-30T04:40:11Z

app/src/main/java/chat/rocket/android/inviteusers/presentation/InviteUsersPresenter.kt

+                        stringBuilder.append("Invited : ${user.username}\n")
+                    } catch (exception: RocketChatException) {
+                        exception.message?.let {
+                            stringBuilder.append("Exception : ${user.username} : $it\n")


Hard-coded string. Please remove.

philipbrito · 2019-06-30T04:40:15Z

app/src/main/java/chat/rocket/android/inviteusers/presentation/InviteUsersPresenter.kt

+                        exception.message?.let {
+                            stringBuilder.append("Exception : ${user.username} : $it\n")
+                        }.ifNull {
+                            stringBuilder.append("Error : ${user.username} : Try again later\n")


Hard-coded string. Please remove.

philipbrito · 2019-06-30T04:41:07Z

app/src/main/java/chat/rocket/android/inviteusers/ui/InviteUsersFragment.kt

+private const val BUNDLE_CHAT_ROOM_ID = "chat_room_id"
+
+class InviteUsersFragment : Fragment(), InviteUsersView {
+


Unneeded blank line.

philipbrito · 2019-06-30T04:42:47Z

app/src/main/java/chat/rocket/android/members/presentation/MembersPresenter.kt


    /**
     * Loads all the chat room members for the given room id.
     *
     * @param roomId The id of the room to get chat room members from.
     */
-    fun loadChatRoomsMembers(roomId: String) {
+    fun loadChatRoomsMembers(roomId: String, offset: Long = 0, clearDataset: Boolean = false) {


What is the reason of adding the offset param on the function declaration here?

Actually there is one bug in MembersFragment where members are not loaded as recycler view is scrolled using EndlessRecyclerViewListener. And only one page of member is shown.

To fix that I followed same pattern we are using for EndlessRecyclerView in ChatRoomFragment
https://github.com/RocketChat/Rocket.Chat.Android/blob/develop/app/src/main/java/chat/rocket/android/chatroom/presentation/ChatRoomPresenter.kt#L240

It is fixed now - you can test by seeing a channel with a lot of users.

philipbrito · 2019-06-30T04:44:21Z

app/src/main/java/chat/rocket/android/members/ui/MembersFragment.kt

+                        totalItemsCount: Int,
+                        recyclerView: RecyclerView
+                    ) {
+                        presenter.loadChatRoomsMembers(chatRoomId, page * 60L)


What is the reason of passing the offset here?

Same pattern as in ChatRoomFragement
https://github.com/RocketChat/Rocket.Chat.Android/blob/develop/app/src/main/java/chat/rocket/android/chatroom/ui/ChatRoomFragment.kt#L810

I have simplified it (based on other classes we have too - e.g MentionsPresenter.kt), but I see your point by choosing that approach. But since the offset is used on the presenter, moving it there can keep the scope...there. But I see your point.

philipbrito · 2019-07-08T03:05:35Z

app/src/main/res/layout/fragment_members.xml

+    <FrameLayout
+        android:id="@+id/text_invite_users"


Widget isn't a text. So it shouldn't be get such id.

philipbrito

Thanks for sending this PR @Shailesh351

All requested changes here were addressed by my own. Pay attention to them for future reference if you want. Thanks ❤️

…ng an user

philipbrito added the 🎉 feature label May 31, 2019

philipbrito added this to the 3.5.0 milestone May 31, 2019

hfeky suggested changes Jun 4, 2019

View reviewed changes

philipbrito suggested changes Jun 7, 2019

View reviewed changes

Shailesh351 force-pushed the sb_add_user branch from 542b2c6 to 6c11033 Compare June 10, 2019 07:37

This comment has been minimized.

Sign in to view

Shailesh351 added 9 commits June 24, 2019 17:35

Check permission based on role of user

129ecae

Add kick user feature

cbf3e33

small fix

19e7881

Add Permission Check to Invite Users

01f1141

Fix layout of invite and remove button

f31763d

Add Invite Users feature

56aac80

chore: Remove wildcard and unused imports

09e5607

feat: Improve code

e7e2866

fix: Members Endless RecyclerView

f87f265

Shailesh351 force-pushed the sb_add_user branch from daaf213 to f87f265 Compare June 24, 2019 12:47

philipbrito suggested changes Jun 26, 2019

View reviewed changes

fix: Code Style(Use 4 Spaces instead of Tab)

9bfa67d

philipbrito suggested changes Jun 30, 2019

View reviewed changes

philipbrito suggested changes Jul 8, 2019

View reviewed changes

Improves code and merge branch with develop

5ebe38c

philipbrito approved these changes Jul 8, 2019

View reviewed changes

Fix an issue making the member list not updated when adding or removi…

9560a22

…ng an user

philipbrito merged commit a115926 into RocketChat:develop Jul 8, 2019

Shailesh351 deleted the sb_add_user branch July 10, 2019 16:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[NEW] Manage Users in a Room(Add/Remove) #2351

[NEW] Manage Users in a Room(Add/Remove) #2351

Shailesh351 commented May 30, 2019

raaparin commented May 31, 2019 •

edited

Loading

hfeky left a comment

philipbrito Jun 7, 2019

Shailesh351 Jun 10, 2019

philipbrito Jun 7, 2019

Shailesh351 Jun 10, 2019 •

edited

Loading

This comment has been minimized.

philipbrito left a comment

Shailesh351 commented Jun 27, 2019

philipbrito Jun 30, 2019

philipbrito Jun 30, 2019

philipbrito Jun 30, 2019

philipbrito Jun 30, 2019

philipbrito Jun 30, 2019

Shailesh351 Jul 8, 2019 •

edited

Loading

philipbrito Jul 8, 2019

philipbrito Jun 30, 2019

Shailesh351 Jul 8, 2019

philipbrito Jul 8, 2019 •

edited

Loading

philipbrito Jul 8, 2019

philipbrito left a comment

		private const val BUNDLE_CHAT_ROOM_ID = "chat_room_id"

		class InviteUsersFragment : Fragment(), InviteUsersView {

[NEW] Manage Users in a Room(Add/Remove) #2351

[NEW] Manage Users in a Room(Add/Remove) #2351

Conversation

Shailesh351 commented May 30, 2019

Changes:

Screenshots or GIF for the change:

raaparin commented May 31, 2019 • edited Loading

hfeky left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Shailesh351 Jun 10, 2019 • edited Loading

Choose a reason for hiding this comment

This comment has been minimized.

philipbrito left a comment

Choose a reason for hiding this comment

Shailesh351 commented Jun 27, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Shailesh351 Jul 8, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

philipbrito Jul 8, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

philipbrito left a comment

Choose a reason for hiding this comment

raaparin commented May 31, 2019 •

edited

Loading

Shailesh351 Jun 10, 2019 •

edited

Loading

Shailesh351 Jul 8, 2019 •

edited

Loading

philipbrito Jul 8, 2019 •

edited

Loading