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

set store hint on receipts and type='chat' #1382

Merged
merged 1 commit into from Dec 17, 2018

Conversation

@ChaosKid42
Copy link
Contributor

commented Dec 16, 2018

I believe it's necesary to store receipts. Otherwise they might not reach the original sender (e.g. in case sender and receiver are not online at the same time). If they are stored in the archive, they will be retrieved later.

@jcbrand

This comment has been minimized.

Copy link
Member

commented Dec 16, 2018

AFAIK the store hint is for storing the message in MAM and I'm not sure that we want to store receipts in MAM.

The XMPP server already keeps track of messages that were sent to offline users and sends it to them when they come online (without the store hint). I assume this also applies to receipts.

@ChaosKid42

This comment has been minimized.

Copy link
Contributor Author

commented Dec 16, 2018

I believe you are right regarding offline storage. Nevertheless this won't help if the user is online with another device. If the user's first device comes online again, only mam will help to retrieve the receipt I believe.
btw: Conversations seems to set the store hint, too.

@ChaosKid42

This comment has been minimized.

Copy link
Contributor Author

commented Dec 17, 2018

@jcbrand I really believe this is necessary in order to synchronize state between different clients of the same user. If your unhappy with this PR I could also make this an configurable feature. What do you think?

@jcbrand jcbrand changed the title set store hint on receipts set store hint on XEP-0184 receipts Dec 17, 2018

@ChaosKid42 ChaosKid42 changed the title set store hint on XEP-0184 receipts set store hint on receipts and type='chat' Dec 17, 2018

@ChaosKid42 ChaosKid42 force-pushed the ChaosKid42:set_store_hint_on_receipts branch from aee17b8 to 99ea671 Dec 17, 2018

@jcbrand jcbrand merged commit d2d6495 into conversejs:master Dec 17, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jcbrand

This comment has been minimized.

Copy link
Member

commented Dec 17, 2018

Thanks.

Merged after further discussion in discuss@conference.conversejs.org.

Summary:

  • Set type="chat" so that receipts get carbon copied.
  • Add messages to MAM to ensure we always know whether messages were received.

Converse's handling of MAM probably needs improving so that we can handle metadata messages being included in results better (e.g. continuing querying further pages to get the right amount of actual messages).

@ChaosKid42 ChaosKid42 deleted the ChaosKid42:set_store_hint_on_receipts branch Dec 17, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.