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

Improve error handling when group members unregister #5318

Conversation

haffenloher
Copy link
Contributor

Contributor checklist

  • I am following the Code Style Guidelines
  • I have tested my contribution on these devices:
    • Emulator, Android 5.1
  • My contribution is fully baked and ready to be merged as is
  • I have made the choice whether I want the Bithub reward or not by omitting or adding the word FREEBIE in my commit message

Description

  • Store UnregisteredUsers in the DB (just like NetworkFailures / IdentityKeyMismatches) and display an error text for them in the message details
  • Add a "More info" button to the MessageRecipientListItem of the unregistered recipient which opens an AlertDialog, informing the user of the problem
  • After acknowledging / closing the dialog, group messages to this recipient no longer fail (in the UI)

Screenshots

Edit: These are all obsolete

Fixes #2408
Fixes #4550

@haffenloher
Copy link
Contributor Author

Added information on whether a group member has unregistered to the GroupMembersDialog - thanks to @agrajaghh for the hint:

@agrajaghh
Copy link
Contributor

👍
just an idea: maybe add a newline inbetween? is that possible there?

@haffenloher
Copy link
Contributor Author

Done, looks like this:

@haffenloher haffenloher force-pushed the fix/group_member_unregisters_2 branch from 0006a50 to e73f1cd Compare March 5, 2016 21:52
@2-4601
Copy link
Contributor

2-4601 commented Mar 5, 2016

How about formatting the contact name with strikethrough? (If it's simple enough to do)
Android Emoji Fan
(no longer registered)

@agrajaghh
Copy link
Contributor

I think its a bit better, no? This dialog is so ugly anyway... 😱 I started working on improving this, but didn't had the time to finish it yet. Hopefully I'm less busy soon 😉

@2-4601 good idea!

@haffenloher
Copy link
Contributor Author

@agrajaghh cool, a rework of this thing would be nice for sure!
@2-4601 I think that would require some more changes to the dialog, so I'll leave it to @agrajaghh :)

@agrajaghh
Copy link
Contributor

@haffenloher
Copy link
Contributor Author

you're right, I thought it would be more complicated:

Still not that beautiful though 😊

@moxie0
Copy link
Contributor

moxie0 commented Mar 7, 2016

i'd like it to just not display any error state when someone is unregistered

@2-4601
Copy link
Contributor

2-4601 commented Mar 8, 2016

@haffenloher What happens in this scenario:

  • you have acknowledged the unregistered member with "Got it"
  • you go to a poor network coverage area
  • you try to send a message to the group
  • sending fails for the unregistered contact with the error "failed to send"

Is it going to offer the option to send again to that contact?

@haffenloher
Copy link
Contributor Author

@moxie0 done, cuts the diff in half =)

@2-4601 yes, you would have seen the resend button in that scenario. Anyway, as per moxie's request there's now no error state, no AlertDialog and no "Got it" button anymore ;)

@moxie0
Copy link
Contributor

moxie0 commented Mar 8, 2016

oh no I mean I don't want any state tracking at all. everything's the same, except you don't get an error when someone in the group has unregistered

@haffenloher haffenloher force-pushed the fix/group_member_unregisters_2 branch from acf25c8 to 06b02ab Compare March 8, 2016 23:29
@haffenloher
Copy link
Contributor Author

Oops... okay, I removed all the other stuff and force-pushed. This is just a tiny 10-line change now.

database.markAsPush(messageId);

notifyMediaMessageDeliveryFailed(context, messageId);
if (failures.isEmpty() && e.getUntrustedIdentityExceptions().isEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

if (e.getNetworkExceptions().isEmpty() && e.getUntrustedIdentityExceptions().isEmpty() { ...

@haffenloher haffenloher force-pushed the fix/group_member_unregisters_2 branch from 06b02ab to 1b17989 Compare March 9, 2016 22:33
@haffenloher
Copy link
Contributor Author

@moxie0 thanks! that looks better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants