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(chatffi): get conversationalists #5849

Merged
merged 4 commits into from
Oct 24, 2023

Conversation

brianp
Copy link
Contributor

@brianp brianp commented Oct 20, 2023

Description

Add a feature to fetch a list of TariAddresses that a client currently has conversations with. This is any address a message has been sent to or received from.

Motivation and Context

Without being about to fetch a list of addresses a client has conversations with, the client can't actually fetch any independent conversations, as conversations are fetched with an address id.

How Has This Been Tested?

New test additions

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

Changes are pretty standard throughout. Handler/Service/DB updates.

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 20, 2023

Test Results (CI)

1 233 tests   1 233 ✔️  9m 25s ⏱️
     39 suites         0 💤
       1 files           0

Results for commit f8a514a.

♻️ 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 20, 2023
@github-actions
Copy link

github-actions bot commented Oct 20, 2023

Test Results (Integration tests)

34 tests   34 ✔️  13m 16s ⏱️
11 suites    0 💤
  2 files      0

Results for commit f8a514a.

♻️ This comment has been updated with latest results.

/// ## Safety
/// The `ConversationalistsVector` should be destroyed after use
#[no_mangle]
pub unsafe extern "C" fn get_conversationalists(
Copy link
Contributor

Choose a reason for hiding this comment

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

I am missing the create and destroy test for this function in fn test_retrieving_conversationalists_from_vector

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The destroy is used in the test_retrieving_conversationalists_from_vector.

As for a get_conversationlists test it gets harder because it requires a whole running client/db/service etc. Which I haven't setup for any of the unit testing in chat. It is covered in cucumber though.

Copy link
Contributor

@hansieodendaal hansieodendaal 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 one comment ^^

@SWvheerden SWvheerden merged commit d9e8e22 into tari-project:development Oct 24, 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 P-reviews_required Process - Requires a review from a lead maintainer to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants