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

refactor(SPV-1020): move paymail servant to separate package and use it in engine #697

Merged
merged 6 commits into from
Sep 13, 2024

Conversation

dorzepowski
Copy link
Contributor

Context

  • in the next step of new create draft transaction method we need to integrate with paymail
  • using paymail servant when it was in engine from the other package used by engine is ending with circular deps error
  • I changed the name of PaymailServant to paymail.ServiceClient - to not have redundant Paymail in package and struct name
  • most important functionality: getCapabilities with cacheing, was done by private function in engine, while it seems like a very useful method that could be used by any thing, that's why I moved it to the paymail.ServiceClient
  • I moved some of the functions from paymail.go to be a methods on paymail service client and make the current code that was using them to use them with help of paymail.ServiceClient
  • I moved a paymail_test.go but it has several things that was mocking paymail host server
  • I created a method to setup paymail client with connection to mocked paymail host server and with methods to help to setup this paymail host server.

Pull Request Checklist

  • 📖 I created my PR using provided : CODE_STANDARDS
  • 📖 I have read the short Code of Conduct: CODE_OF_CONDUCT
  • 🏠 I tested my changes locally.
  • ✅ I have provided tests for my changes.
  • 📝 I have used conventional commits.
  • 📗 I have updated any related documentation.
  • 💾 PR was issued based on the Github or Jira issue.

…it instead of functions from engine; Introduce better mocking API for testing with paymail operations.
@dorzepowski dorzepowski requested a review from a team as a code owner September 11, 2024 12:52
Copy link

github-actions bot commented Sep 11, 2024

Manual Tests

💚 Manual testing by @chris-4chain resulted in success.

@codecov-commenter
Copy link

codecov-commenter commented Sep 11, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 64.57143% with 124 lines in your changes missing coverage. Please review.

Project coverage is 46.42%. Comparing base (ba50ac3) to head (1332f5e).

Files with missing lines Patch % Lines
engine/paymail/paymail_service_client.go 57.60% 34 Missing and 5 partials ⚠️
engine/tester/paymailmock/mock_paymail_client.go 65.45% 36 Missing and 2 partials ⚠️
engine/tester/paymailmock/mock_capability.go 59.25% 8 Missing and 3 partials ⚠️
engine/paymail/paymail_payload_format.go 0.00% 8 Missing ⚠️
engine/client_internal.go 41.66% 5 Missing and 2 partials ⚠️
...ne/tester/paymailmock/mock_capability_factories.go 83.33% 6 Missing ⚠️
engine/tester/cache.go 0.00% 5 Missing ⚠️
engine/model_draft_transactions.go 25.00% 0 Missing and 3 partials ⚠️
engine/tester/logger.go 0.00% 2 Missing ⚠️
...ine/tester/paymailmock/mock_paymail_domain_name.go 87.50% 2 Missing ⚠️
... and 3 more
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #697      +/-   ##
==========================================
+ Coverage   46.32%   46.42%   +0.09%     
==========================================
  Files         290      296       +6     
  Lines       12833    13024     +191     
==========================================
+ Hits         5945     6046     +101     
- Misses       6296     6386      +90     
  Partials      592      592              
Flag Coverage Δ
unittests 46.42% <64.57%> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
engine/action_contact.go 40.65% <100.00%> (-0.95%) ⬇️
engine/paymail.go 0.00% <ø> (-44.83%) ⬇️
...gine/tester/paymailmock/paymail_service_fixture.go 100.00% <100.00%> (ø)
engine/client.go 65.15% <0.00%> (ø)
engine/client_paymail.go 86.36% <83.33%> (-1.14%) ⬇️
engine/model_transaction_config.go 81.76% <88.88%> (+0.62%) ⬆️
engine/tester/logger.go 0.00% <0.00%> (ø)
...ine/tester/paymailmock/mock_paymail_domain_name.go 87.50% <87.50%> (ø)
engine/model_draft_transactions.go 74.80% <25.00%> (-0.68%) ⬇️
engine/tester/cache.go 62.50% <0.00%> (-11.58%) ⬇️
... and 6 more

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ba50ac3...1332f5e. Read the comment docs.

@dorzepowski dorzepowski deleted the refactor/move-paymail-servant-to-package branch September 11, 2024 13:03
@dorzepowski dorzepowski restored the refactor/move-paymail-servant-to-package branch September 11, 2024 13:06
@dorzepowski dorzepowski reopened this Sep 11, 2024
engine/paymail/paymail_service_client.go Outdated Show resolved Hide resolved
engine/paymail/paymail_service_client.go Outdated Show resolved Hide resolved
engine/paymail/paymail_service_client.go Outdated Show resolved Hide resolved
engine/paymail/paymail_service_client.go Outdated Show resolved Hide resolved
engine/paymail/paymail_service_client.go Outdated Show resolved Hide resolved
engine/paymail/paymail_service_client.go Outdated Show resolved Hide resolved
engine/paymail/paymail_service_client.go Outdated Show resolved Hide resolved
engine/action_contact.go Outdated Show resolved Hide resolved
@wregulski
Copy link
Contributor

Please assign to PR :D

@dorzepowski dorzepowski self-assigned this Sep 12, 2024
engine/action_contact.go Outdated Show resolved Hide resolved
engine/client_internal.go Show resolved Hide resolved
engine/paymail/paymail_payload_format.go Show resolved Hide resolved
engine/paymail/paymail_service_client.go Outdated Show resolved Hide resolved
engine/paymail/paymail_service_client.go Outdated Show resolved Hide resolved
engine/paymail/paymail_service_client.go Show resolved Hide resolved
engine/paymail/paymail_service_client.go Outdated Show resolved Hide resolved
@chris-4chain chris-4chain added the tested PR was tested by a team member label Sep 13, 2024
@dorzepowski dorzepowski merged commit 3095842 into main Sep 13, 2024
8 checks passed
@dorzepowski dorzepowski deleted the refactor/move-paymail-servant-to-package branch September 13, 2024 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tested PR was tested by a team member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants