-
Notifications
You must be signed in to change notification settings - Fork 29
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
fix(conversation): disable addMember on a one-on-one conversation with deleted account (WPB-10259) #3349
fix(conversation): disable addMember on a one-on-one conversation with deleted account (WPB-10259) #3349
Conversation
…h deleted account
Test Results863 tests 863 ✅ 9m 46s ⏱️ Results for commit de9d18c. ♻️ This comment has been updated with latest results. |
APKs built during tests are available here. Scroll down to Artifacts! |
@@ -394,7 +394,8 @@ private fun GroupConversationDetailsContent( | |||
openFullListPressed = openFullListPressed, | |||
onAddParticipantsPressed = onAddParticipantsPressed, | |||
onProfilePressed = onProfilePressed, | |||
lazyListState = lazyListStates[pageIndex] | |||
lazyListState = lazyListStates[pageIndex], | |||
allowAddMember = conversationSheetState.conversationSheetContent?.title?.isNotEmpty() ?: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would change this to check if the name is not null and there is only one member, since legacy group conversations did allow empty names
this way, legacy groups will still continue to work as before.
APKs built during tests are available here. Scroll down to Artifacts! |
…ted-member-oneonone
else UIText.StringResource(R.string.group_unavailable_label).asString() | ||
else UIText.StringResource(R.string.member_name_deleted_label).asString() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: is there a reason for the title change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, but it was showing different message for the same situation! e.g. when you see a conversation for a deleted user, and open the conversation details, you see the title is not available! so I decided to make it concise.
allowAddMember = conversationSheetState.conversationSheetContent?.title?.isNotEmpty() == true | ||
|| groupParticipantsState.data.allCount > 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets move this into the conversationSheetState this way it can be tested
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about it, but still I need to pass the participants counts to it; since the groupParticipants stored out of the conversationSheetState.
…ndroid-reloaded into fix/disable-add-member-for-deleted-member-oneonone
…ed-member-oneonone' into fix/disable-add-member-for-deleted-member-oneonone
APKs built during tests are available here. Scroll down to Artifacts! |
…ted-member-oneonone
Quality Gate passedIssues Measures |
APKs built during tests are available here. Scroll down to Artifacts! |
…h deleted account
PR Submission Checklist for internal contributors
The PR Title
SQPIT-764
The PR Description
What's new in this PR?
Issues
When the other peer of a one-on-one conversation gets deleted, the backend changes the conversation to a group conversation. Android app was allowing the user to add member to that conversation and it was a confusing flow; in this PR we're preventing this.
PR Post Submission Checklist for internal contributors (Optional)
PR Post Merge Checklist for internal contributors
References
feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764
.