-
Notifications
You must be signed in to change notification settings - Fork 39
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
21537 - Shortname Refunds - PUT and GET methods #1758
base: main
Are you sure you want to change the base?
Conversation
@@ -137,7 +137,7 @@ def create_shortname_refund(history: EFTShortnameHistory, **kwargs) -> EFTShortn | |||
is_processing=history.is_processing, | |||
short_name_id=history.short_name_id, | |||
eft_refund_id=history.eft_refund_id, | |||
transaction_date=EFTShortnameHistorical.transaction_date_now(), | |||
transaction_date=EFTShortNameHistorical.transaction_date_now(), |
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.
Yikes, I'll put this back to EFTShortnameHistorical
Looks good so far! Thanks! |
refund = EFTRefundModel.find_pending_refund(refund_id) | ||
refund.comment = data.comment | ||
refund.decline_reason = data.declined_reason | ||
refund.status = data.status |
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.
We will need some additional logic when declining.
- The funds removed from the credits will have to be put back.
- History record status needs to be updated - amount can stay the same for history purposes and the UI will automatically display what is relevant for history status.
Feel free to just add # TODO and I can follow up on this
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.
whoops forgot, I'll add it in
@@ -71,6 +71,7 @@ notes=FIXME,XXX,TODO | |||
ignored-modules=flask_sqlalchemy,sqlalchemy,SQLAlchemy,alembic,scoped_session | |||
ignored-classes=scoped_session | |||
min-similarity-lines=15 | |||
max-attributes=15 |
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 think 7 was a bit too low, 15 is reasonable.. for some of our larger search dataclasses
status=EFTShortnameRefundStatus.APPROVED.value | ||
).to_dict(), 'valid_full_patch_refund', Role.EFT_REFUND_APPROVER.value), | ||
({}, 'unauthorized', Role.EFT_REFUND.value), | ||
(EFTShortNameRefundPatchRequest( |
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.
a bit of a new style of test, using objects that are serialized..
Issue #:
bcgov/entity#21537
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).