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

Add test to Django command retry_sage_transactions #326

Merged
merged 1 commit into from
Dec 13, 2024

Conversation

Tiago-Salles
Copy link
Contributor

@Tiago-Salles Tiago-Salles commented Oct 31, 2024

This PR refactors retry sending transaction and increase the tests coverage.

  • Removed the command functionality from the command layer

    • The command business logic is part of TransactionService layer which is the component aimed to handle transaction resources. Refactored intended to follow the single responsibility principle and provide loose coupling for testing.
  • Added tests to the command logic

    • Tests for command handler functionality and for the command business logic inside TransactionService.

This PR is related to #270

@Tiago-Salles Tiago-Salles force-pushed the Tiago-Salles/issues/270-add-tests-retry-transaction branch 2 times, most recently from d756938 to 5172d5b Compare November 5, 2024 17:12
- Removed the command functionality from the command layer
- Added tests to the command logic
@Tiago-Salles Tiago-Salles force-pushed the Tiago-Salles/issues/270-add-tests-retry-transaction branch from 5172d5b to 3a40f3f Compare November 6, 2024 11:55
@Tiago-Salles Tiago-Salles marked this pull request as ready for review November 6, 2024 12:13
Copy link
Member

@igobranco igobranco left a comment

Choose a reason for hiding this comment

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

LGTM ✔️

@igobranco igobranco merged commit 82e7a9c into main Dec 13, 2024
4 checks passed
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.

2 participants