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

17534 - NSF Implementation #1377

Merged
merged 6 commits into from
Jan 22, 2024
Merged

17534 - NSF Implementation #1377

merged 6 commits into from
Jan 22, 2024

Conversation

rodrigo-barraza
Copy link
Collaborator

@rodrigo-barraza rodrigo-barraza commented Jan 19, 2024

*Issue #:17534 *
bcgov/entity#17534

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).

commit 91785fa
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Tue Jan 16 07:59:01 2024 -0800

    revision update

commit 19b8a90
Merge: 915cc0d 676d202
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Fri Jan 12 08:15:08 2024 -0800

    Merge branch 'main' into feature/17534

commit 915cc0d
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Fri Jan 12 06:38:47 2024 -0800

    Updating NSF query

commit 89d80bc
Merge: 2b50ab4 7ca3f7d
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Tue Jan 2 09:55:10 2024 -0800

    Merge branch 'main' into feature/17534

commit 2b50ab4
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Fri Dec 15 07:26:47 2023 -0800

    Unit test fix

commit ae9c1de
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Fri Dec 15 07:17:50 2023 -0800

    Expanding API test

commit b034d16
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Fri Dec 15 07:02:58 2023 -0800

    Flake8 fix

commit 521b252
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Fri Dec 15 06:56:30 2023 -0800

    Unit test fix

commit 9005132
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Fri Dec 15 06:46:42 2023 -0800

    Expanding tests

commit 5d75820
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Fri Dec 15 06:33:01 2023 -0800

    Checking auth, getting latest CFS account

commit 4aab599
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Thu Dec 14 12:30:13 2023 -0800

    Cleanup, new CFS method

commit 23c4099
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Wed Dec 13 18:21:20 2023 -0800

    Expanding unit test

commit fad3b66
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Wed Dec 13 14:00:40 2023 -0800

    Lint fix

commit 848ec10
Merge: f2cebe8 dbb11ad
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Wed Dec 13 13:55:09 2023 -0800

    Merge branch 'main' into feature/17534

commit f2cebe8
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Wed Dec 13 13:55:01 2023 -0800

    Unit test fixes

commit a6fbaa4
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Tue Dec 12 13:40:35 2023 -0800

    NSF query refactoring

    NSF query refactoring and accommodating template structure

commit e2946ad
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Tue Dec 12 09:08:41 2023 -0800

    Moving logic to query, updating template

commit 13a4eb7
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Mon Dec 11 13:17:15 2023 -0800

    Feedback updates

commit 08d62ac
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Mon Dec 11 13:13:13 2023 -0800

    New NSF query and cleanup for NSF PDF template

commit dff9b98
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Sun Dec 10 22:18:56 2023 -0800

    Test fixes

commit 5063764
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Sun Dec 10 21:10:11 2023 -0800

    Version bump

commit baa28b4
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Sun Dec 10 21:03:08 2023 -0800

    flake8 fix

commit 5d24d0d
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Sun Dec 10 20:55:22 2023 -0800

    Model and service changes

commit e08b591
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Sun Dec 10 19:50:21 2023 -0800

    asdict

commit be2d5e2
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Sun Dec 10 19:06:27 2023 -0800

    Cleanup

commit 6cc9bdc
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Sun Dec 10 18:33:27 2023 -0800

    Lint fixes

commit 31aa3da
Merge: 456eb65 dafa812
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Sun Dec 10 18:20:21 2023 -0800

    Merge branch 'main' into feature/17534

commit 456eb65
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Sun Dec 10 18:20:07 2023 -0800

    PDF fixes

commit f6c676f
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Fri Dec 8 13:29:14 2023 -0800

    Query fix

commit 91e3ed5
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Thu Dec 7 08:50:50 2023 -0800

    Ignoring lint on NonSufficientFundsSearchModel

commit a9fa502
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Thu Dec 7 08:46:10 2023 -0800

    Lint cleanup

commit b7c6a93
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Thu Dec 7 08:36:33 2023 -0800

    New NSF template, adding search model, migration script, cleanup

commit e7cc660
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Sun Dec 3 19:42:32 2023 -0800

    import fix

commit 04742b8
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Sun Dec 3 19:33:34 2023 -0800

    isort fix

commit 7f663da
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Sun Dec 3 19:26:14 2023 -0800

    Fixes

commit df55123
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Sun Dec 3 19:07:58 2023 -0800

    More flake8 test fixes

commit 40a4b47
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Sun Dec 3 18:51:54 2023 -0800

    Flake8 fixes

commit 01aa77e
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Sun Dec 3 18:40:15 2023 -0800

    Lint and test fixes

commit bb0ccdd
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Sun Dec 3 18:26:56 2023 -0800

    Linting and test fix

commit 6c7e6c0
Merge: 93390b7 2235d3d
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Sun Dec 3 18:08:59 2023 -0800

    Merge branch 'main' into feature/17534

commit 93390b7
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Sun Dec 3 18:08:51 2023 -0800

    Non sufficient funds implementation
commit b78db55
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Tue Jan 16 08:58:32 2024 -0800

    Linting fixes

commit 29e2180
Merge: 0e691bb 676d202
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Tue Jan 16 08:37:23 2024 -0800

    Merge branch 'main' into feature/17507

commit 0e691bb
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Mon Dec 11 09:40:53 2023 -0800

    reminder

commit 6ad3242
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Mon Dec 11 00:20:31 2023 -0800

    Payload reformatting

commit ea27395
Merge: a130ae2 dafa812
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Sun Dec 10 23:31:06 2023 -0800

    Merge branch 'main' into feature/17507

commit a130ae2
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Thu Nov 23 18:05:14 2023 -0800

    Cleanup

commit 24f771f
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Wed Nov 15 08:55:20 2023 -0800

    Argument fix

commit b7af147
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Tue Nov 14 20:09:51 2023 -0800

    Moving payload over

commit 8a0f788
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Thu Nov 9 08:54:10 2023 -0800

    Error fix

commit b9106f2
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Thu Nov 9 08:41:31 2023 -0800

    PDF payload information

commit 034cd7c
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Thu Nov 9 07:53:04 2023 -0800

    Passing payload data for PDF

commit 6110773
Author: Rodrigo Barraza <hello@rod.dev>
Date:   Mon Nov 6 05:37:41 2023 -0800

    Email update
@rodrigo-barraza rodrigo-barraza changed the title Feature/17534 nsf feature 17534 - NSF Implementation Jan 19, 2024
Copy link

codecov bot commented Jan 19, 2024

Codecov Report

Attention: 38 lines in your changes are missing coverage. Please review.

Comparison is base (79924ce) 91.45% compared to head (4ea8cf0) 87.36%.
Report is 76 commits behind head on main.

❗ Current head 4ea8cf0 differs from pull request most recent head 6615d98. Consider uploading reports for the commit 6615d98 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1377      +/-   ##
==========================================
- Coverage   91.45%   87.36%   -4.10%     
==========================================
  Files         186       65     -121     
  Lines       11319     3403    -7916     
==========================================
- Hits        10352     2973    -7379     
+ Misses        967      430     -537     
Flag Coverage Δ
bcolapi ?
eventlistenerqueue 82.00% <100.00%> (+0.18%) ⬆️
payadmin ∅ <ø> (?)
payapi ?
paymentjobs 83.35% <94.64%> (+3.13%) ⬆️
paymentreconciliationsqueue 92.30% <93.84%> (+0.86%) ⬆️
reportapi 91.09% <ø> (ø)

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

Files Coverage Δ
jobs/payment-jobs/tasks/common/dataclasses.py 100.00% <100.00%> (ø)
jobs/payment-jobs/tasks/common/enums.py 100.00% <ø> (ø)
jobs/payment-jobs/tasks/distribution_task.py 97.70% <100.00%> (-0.06%) ⬇️
...ayment-jobs/tasks/ejv_partner_distribution_task.py 99.17% <100.00%> (+0.03%) ⬆️
jobs/payment-jobs/tasks/ejv_payment_task.py 96.61% <100.00%> (+0.14%) ⬆️
.../payment-jobs/tasks/statement_notification_task.py 79.74% <100.00%> (+47.92%) ⬆️
jobs/payment-jobs/tasks/statement_task.py 91.39% <100.00%> (+10.96%) ⬆️
...ices/events-listener/src/events_listener/config.py 93.75% <100.00%> (+0.13%) ⬆️
...iations/src/reconciliations/cgi_reconciliations.py 93.43% <100.00%> (+0.07%) ⬆️
...ment-reconciliations/src/reconciliations/config.py 96.47% <100.00%> (+0.08%) ⬆️
... and 17 more

... and 131 files with indirect coverage changes

Copy link

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@rodrigo-barraza rodrigo-barraza marked this pull request as ready for review January 22, 2024 16:14
@@ -91,6 +91,12 @@ def find_effective_by_account_id(cls, account_id: str):
return CfsAccount.query.filter(CfsAccount.account_id == account_id,
CfsAccount.status != CfsAccountStatus.INACTIVE.value).one_or_none()

@classmethod
def find_latest_account_by_account_id(cls, account_id: str):
"""Return a frozen account by account_id, and return the record with the highest id."""
Copy link
Collaborator

Choose a reason for hiding this comment

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

comment doesn't pertain to the logic?

@@ -121,5 +123,5 @@ def from_row(cls, row: PaymentLineItem):

https://www.attrs.org/en/stable/init.html
"""
return cls(gst=row.gst, pst=row.pst, description=row.description,
filing_type_code=row.fee_schedule.filing_type_code)
return cls(total=row.total, gst=row.gst, pst=row.pst, service_fees=row.service_fees,
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we're changing this, need to let CSO know

try:
pdf, pdf_filename = NonSufficientFundsService.create_non_sufficient_funds_statement_pdf(account_id=account_id)
response = Response(pdf, 201)
response.headers.set('Content-Disposition', 'attachment', filename=f'{pdf_filename}.pdf')
Copy link
Collaborator

Choose a reason for hiding this comment

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

candidate for helper function, I think this code is duplicated somewhere else

op.create_table('non_sufficient_funds',
sa.Column('id', sa.Integer(), autoincrement=True, nullable=False),
sa.Column('description', sa.String(length=50), nullable=True),
sa.Column('invoice_id', sa.Integer(), nullable=False),
Copy link
Collaborator

@seeker25 seeker25 Jan 22, 2024

Choose a reason for hiding this comment

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

invoice_number as well? that's what's really tied to CAS's system

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Originally I felt that passing the invoice_number would the better choice, since the NSF state can apply to multiple invoices, and with the current logic it just fit. But at the end of the day it's tied to a specific invoice, so it made more sense for this. Passing both invoice_number and invoice_id should be fine, even though it's might seem a little redundant.

@seeker25 seeker25 merged commit 49bfe45 into main Jan 22, 2024
19 of 25 checks passed
@rodrigo-barraza rodrigo-barraza deleted the feature/17534-nsf-feature branch September 4, 2024 18:41
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