-
Notifications
You must be signed in to change notification settings - Fork 40
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
19759 - Partial refunds PAYBC implementation #1414
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1414 +/- ##
==========================================
+ Coverage 91.45% 91.96% +0.51%
==========================================
Files 186 193 +7
Lines 11319 12936 +1617
==========================================
+ Hits 10352 11897 +1545
- Misses 967 1039 +72
Flags with carried forward coverage won't be shown. Click here to find out more.
|
@@ -71,7 +71,7 @@ notes=FIXME,XXX,TODO | |||
ignored-modules=flask_sqlalchemy,sqlalchemy,SQLAlchemy,alembic,scoped_session | |||
ignored-classes=scoped_session | |||
min-similarity-lines=15 | |||
disable=C0301,W0511 | |||
disable=C0301,W0511,R0903 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Too few public methods warning, it's fairly useless for dataclasses etc ATTRS and CATTRS don't use pylint anymore they use Flake8 I believe
Creating another ticket to do refactoring of PAYBC job |
|
||
refund_lines, total_refund = DirectPayService._build_refund_revenue_lines(refund_partial) | ||
paybc_invoice = DirectPayService._query_order_status(invoice) | ||
refund_payload.update({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I probably could have used a dataclass
for this, but I think i'll leave it for now
Haven't tested with PAYBC yet, standby |
direct_pay_service = DirectPayService() | ||
with pytest.raises(BusinessException) as excinfo: | ||
direct_pay_service.build_automated_refund_payload(invoice, refund_partial) | ||
assert excinfo.value.code == Error.INVALID_REQUEST.name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to return real errors, not sure your thoughts about returning INVALID REQUEST, on the plus side at least I wrote some logging to handle the cases
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works for me
Quality Gate passedIssues Measures |
Issue #:
bcgov/entity#19759
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the sbc-pay license (Apache 2.0).