Skip to content
This repository has been archived by the owner on Jun 7, 2020. It is now read-only.

[FIX] roomid or username required bug #1930

Merged
merged 10 commits into from
Dec 18, 2018

Conversation

dabbler011
Copy link
Contributor

@RocketChat/android

Closes #1912

chatRoom object was instantiated with empty id. So, toDirectMessage() was called with empty id in clickListeners.

Works fine now.

@rafaelks rafaelks requested a review from philipbrito December 14, 2018 19:49
@rafaelks rafaelks added this to the 3.2.0 milestone Dec 14, 2018
@rafaelks
Copy link
Contributor

Thanks for the PR, but this problem fixes the alert, but you can't chat with the person because there's no "Send" button or the emoji button doesn't work.

@dabbler011
Copy link
Contributor Author

The problem was because the newly created room was not added in the database due to which listeners were not instantiated. I suppose refreshing chat rooms in db before accessing them fixes the issue. I have commited changes for the same.

@rafaelks please review.

@rafaelks
Copy link
Contributor

@dabbler011 "Send button" still don't show up on new conversations.

@dabbler011
Copy link
Contributor Author

@rafaelks are you sure? As it is working just fine for me.

video

@rafaelks
Copy link
Contributor

Just tested the latest build here, check it out:

2018-12-17 10 36 57

@leonardoaramaki
Copy link
Contributor

leonardoaramaki commented Dec 17, 2018

@dabbler011 change the following method on ChatRoomPresenter to this and we should be good to merge:

private suspend fun subscribeRoomChanges() {
    withContext(CommonPool + strategy.jobs) {
            chatRoomId?.let {
                manager.addRoomChannel(it, roomChangesChannel)
                for (room in roomChangesChannel) {
                    dbManager.getRoom(room.id)?.let {
                        view.onRoomUpdated(roomMapper.map(chatRoom = it, showLastMessage = true))
                    }
                }
            }
        }
}

Copy link
Contributor

@leonardoaramaki leonardoaramaki left a comment

Choose a reason for hiding this comment

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

Changes pointed out comments.

@dabbler011
Copy link
Contributor Author

@leonardoaramaki updated the method.

@leonardoaramaki
Copy link
Contributor

Thanks @dabbler011. Testing

@leonardoaramaki leonardoaramaki merged commit 27b1007 into RocketChat:develop Dec 18, 2018
@dabbler011 dabbler011 deleted the nullfix branch December 31, 2018 17:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants