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

21536 - Passing short Name Id parameter to find refunds #1783

Merged
merged 13 commits into from
Oct 18, 2024
8 changes: 6 additions & 2 deletions pay-api/src/pay_api/dtos/eft_shortname.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,14 @@ class EFTShortNameRefundGetRequest:
"""EFT Short name refund DTO."""

statuses: List[str]
short_name_id: int = None

@classmethod
def from_dict(cls, data: dict):
"""Convert from request json to EFTShortNameRefundDTO."""
input_string = data.get("status", "")
input_string = data.get("statuses", "")
short_name_id = None
ochiu marked this conversation as resolved.
Show resolved Hide resolved
if data.get("shortNameId", None) is not None:
Copy link
Collaborator

@seeker25 seeker25 Oct 18, 2024

Choose a reason for hiding this comment

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

This defeats the purpose of using the serializer or converter.. please use Serializable super().from_dict(data) instead of doing this, the statuses were a work around because the serialization needed a bit more looking at

Copy link
Collaborator

Choose a reason for hiding this comment

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

EG
image

Copy link
Collaborator

@seeker25 seeker25 Oct 18, 2024

Choose a reason for hiding this comment

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

we eventually want to get rid of dto.state = dto.state.split(",") etc somehow in the converter
same with the account_id_list shouldn't need to do this by hand.. eventually we'd like to do this all in the serializer in a generic way - so were not repeating a bunch of boilerplate code over and over again

Copy link
Collaborator

@seeker25 seeker25 Oct 18, 2024

Choose a reason for hiding this comment

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

I'll see if we can put in:

typical register structure unstructure hook - new type DelimitedString - so we wont need to do above hopefully and it's done in one spot in code

short_name_id = int(data.get("shortNameId", None))
statuses = input_string.split(",") if input_string else []
return EFTShortNameRefundGetRequest(statuses=statuses)
return EFTShortNameRefundGetRequest(statuses=statuses, short_name_id=short_name_id)
4 changes: 3 additions & 1 deletion pay-api/src/pay_api/models/eft_refund.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,11 @@ class EFTRefund(Audit):
status = db.Column(db.String(25), nullable=True)

@classmethod
def find_refunds(cls, statuses: List[str]):
def find_refunds(cls, statuses: List[str], short_name_id: int = None):
"""Return all refunds by status."""
query = cls.query
if statuses:
query = cls.query.filter(EFTRefund.status.in_(statuses))
if short_name_id:
query = cls.query.filter(EFTRefund.short_name_id == short_name_id)
return query.all()
2 changes: 1 addition & 1 deletion pay-api/src/pay_api/services/eft_refund.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ def create_shortname_refund(request: dict, **kwargs):
@staticmethod
def get_shortname_refunds(data: EFTShortNameRefundGetRequest):
"""Get all refunds."""
refunds = EFTRefundModel.find_refunds(data.statuses)
refunds = EFTRefundModel.find_refunds(data.statuses, data.short_name_id)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Update tests for short_name_id search please

return [refund.to_dict() for refund in refunds]

@staticmethod
Expand Down
4 changes: 2 additions & 2 deletions pay-api/tests/unit/api/test_eft_short_names.py
Original file line number Diff line number Diff line change
Expand Up @@ -1071,12 +1071,12 @@ def test_post_shortname_refund_invalid_request(client, mocker, jwt):
[
("", "get_all", 3),
(
f"?status={EFTShortnameRefundStatus.APPROVED.value},{EFTShortnameRefundStatus.PENDING_APPROVAL.value}",
f"?statuses={EFTShortnameRefundStatus.APPROVED.value},{EFTShortnameRefundStatus.PENDING_APPROVAL.value}",
seeker25 marked this conversation as resolved.
Show resolved Hide resolved
"status_filter_multiple",
2,
),
(
f"?status={EFTShortnameRefundStatus.DECLINED.value}",
f"?statuses={EFTShortnameRefundStatus.DECLINED.value}",
Copy link
Collaborator

Choose a reason for hiding this comment

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

additional parameters for short_name_id search?

"status_filter_rejected",
1,
),
Expand Down
Loading