Skip to content

fix(matrix): add missing txid to send message call - #416

Open
piksel wants to merge 1 commit into
mainfrom
fix/matrix-txid
Open

fix(matrix): add missing txid to send message call#416
piksel wants to merge 1 commit into
mainfrom
fix/matrix-txid

Conversation

@piksel

@piksel piksel commented Nov 21, 2023

Copy link
Copy Markdown
Member

The Matrix Client/Server spec requires that the send message endpoint includes a unique transaction id with each request. The matrix service omits this id for some reason, and the original testing server probably allowed this.

This PR adds an incrementing counter as the transaction ID, which should allow it to work with conduit matrix server.

Fixes #415.

@codecov

codecov Bot commented Nov 21, 2023

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 75.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.24%. Comparing base (3428e5e) to head (fe44270).
⚠️ Report is 16 commits behind head on main.

Files with missing lines Patch % Lines
pkg/services/matrix/matrix_client.go 73.91% 4 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #416      +/-   ##
==========================================
+ Coverage   79.12%   79.24%   +0.11%     
==========================================
  Files         102      102              
  Lines        4488     4480       -8     
==========================================
- Hits         3551     3550       -1     
+ Misses        758      752       -6     
+ Partials      179      178       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@piksel
piksel marked this pull request as ready for review November 21, 2023 19:56
@katiekloss

Copy link
Copy Markdown

Is there anything I can do to help get this over the finish line? More code coverage? It looks like most of the uncovered paths are for handling errors.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cannot send notifications through matrix

2 participants