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

feat(chat): read receipt feature #5824

Merged
merged 12 commits into from
Oct 3, 2023

Conversation

brianp
Copy link
Contributor

@brianp brianp commented Oct 1, 2023

Description

This improves chat by moving chat message types that move over comms between peers into a specific chat wrapper type ChatDispatch. This new type allows for multiple chat message forms such as a standard chat message, as well as read, and delivery confirmations.

Motivation and Context

To enhance the chat FFI feature with check ✅, and double check ✅✅ capabilities such as chat apps like Telegram and WhatsApp. This PR features refactoring on the contacts service to support new message types, as well as FFI functionality to interact with the feature, including callbacks for receiving confirmations.

How Has This Been Tested?

Cucumber, CI.

What process can a PR reviewer use to test or verify this change?

Naming is questionable. But overall does the chat message wrapping fit our standard practice for similar concepts? The proto/prost enum support is unique and makes for some awkward try_from's but it works well enough.

Breaking Changes

  • None
  • Requires data directory on base node to be deleted
  • Requires hard fork
  • Other - Please specify

@github-actions
Copy link

github-actions bot commented Oct 1, 2023

Test Results (CI)

1 219 tests   1 219 ✔️  9m 45s ⏱️
     39 suites         0 💤
       1 files           0

Results for commit 61133ab.

♻️ This comment has been updated with latest results.

@ghpbot-tari-project ghpbot-tari-project added P-acks_required Process - Requires more ACKs or utACKs P-reviews_required Process - Requires a review from a lead maintainer to be merged labels Oct 1, 2023
@github-actions
Copy link

github-actions bot commented Oct 1, 2023

Test Results (Integration tests)

33 tests  +33   33 ✔️ +33   13m 41s ⏱️ + 13m 41s
11 suites +11     0 💤 ±  0 
  2 files   +  2     0 ±  0 

Results for commit 61133ab. ± Comparison against base commit 754fb16.

♻️ This comment has been updated with latest results.

@brianp brianp force-pushed the feat-chat-read-receipts branch from 06dc2b7 to 449f003 Compare October 2, 2023 12:58
@brianp brianp requested review from SWvheerden and hansieodendaal and removed request for SWvheerden October 2, 2023 13:00
@brianp brianp marked this pull request as ready for review October 2, 2023 13:00
Copy link
Collaborator

@SWvheerden SWvheerden left a comment

Choose a reason for hiding this comment

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

looks good, just remove those unwraps

@ghpbot-tari-project ghpbot-tari-project removed the P-reviews_required Process - Requires a review from a lead maintainer to be merged label Oct 3, 2023
@SWvheerden SWvheerden merged commit d81fe7d into tari-project:development Oct 3, 2023
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-acks_required Process - Requires more ACKs or utACKs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants