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

19936 - EFT - PAY-API changes to handle CFS when switching payment methods #1445

Merged
merged 5 commits into from
Mar 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions pay-api/src/pay_api/services/eft_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
from pay_api.models import Payment as PaymentModel
from pay_api.models import PaymentAccount as PaymentAccountModel
from pay_api.models import Receipt as ReceiptModel
from pay_api.utils.enums import CfsAccountStatus, InvoiceReferenceStatus, PaymentMethod, PaymentStatus
from pay_api.utils.enums import CfsAccountStatus, InvoiceReferenceStatus, PaymentMethod, PaymentStatus, PaymentSystem

from .deposit_service import DepositService
from .invoice import Invoice
from .invoice_reference import InvoiceReference
Expand All @@ -35,9 +36,13 @@ class EftService(DepositService):
"""Service to manage electronic fund transfers."""

def get_payment_method_code(self):
"""Return EFT as the system code."""
"""Return EFT as the payment method code."""
return PaymentMethod.EFT.value

def get_payment_system_code(self):
"""Return PAYBC as the system code."""
return PaymentSystem.PAYBC.value

def create_account(self, identifier: str, contact_info: Dict[str, Any], payment_info: Dict[str, Any],
**kwargs) -> CfsAccountModel:
"""Create an account for the EFT transactions."""
Expand All @@ -47,6 +52,17 @@ def create_account(self, identifier: str, contact_info: Dict[str, Any], payment_
cfs_account.status = CfsAccountStatus.PENDING.value
return cfs_account

def update_account(self, name: str, cfs_account: CfsAccountModel, payment_info: Dict[str, Any]) -> CfsAccountModel:
"""Update pad account."""
if str(payment_info.get('bankInstitutionNumber')) != cfs_account.bank_number or \
str(payment_info.get('bankTransitNumber')) != cfs_account.bank_branch_number or \
str(payment_info.get('bankAccountNumber')) != cfs_account.bank_account_number:
# This means, the current cfs_account is for PAD, not EFT
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is good, intent of why you wrote the code

# Make the current CFS Account as INACTIVE in DB
Copy link
Collaborator

Choose a reason for hiding this comment

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

redundant, read the two lines below

cfs_account.status = CfsAccountStatus.INACTIVE.value
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Inactive current PAD cfs_account

Copy link
Collaborator

@seeker25 seeker25 Mar 19, 2024

Choose a reason for hiding this comment

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

Above is better than "# Make the current CFS Account as INACTIVE in DB"

cfs_account.flush()
return cfs_account

def create_invoice(self, payment_account: PaymentAccount, line_items: List[PaymentLineItem], invoice: Invoice,
**kwargs) -> InvoiceReference:
"""Do nothing here, we create invoice references on the create CFS_INVOICES job."""
Expand Down
5 changes: 4 additions & 1 deletion pay-api/src/pay_api/services/payment_account.py
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,10 @@ def _handle_payment_details(cls, account_request, is_sandbox, pay_system, paymen
cfs_account: CfsAccountModel = CfsAccountModel.find_effective_by_account_id(payment_account.id) \
if payment_account.id else None
if pay_system.get_payment_system_code() == PaymentSystem.PAYBC.value:
if cfs_account is None:
if cfs_account is None or (payment_account.payment_method == PaymentMethod.EFT and cfs_account):
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

when Switching from PAD to EFT, we always create a new cfs account, cause the previous EFT cfs account suppose to be inactivated when it switch to PAD, and also I can't just remove the payment details use cfs service, it returns 404.

Copy link
Collaborator

@seeker25 seeker25 Mar 19, 2024

Choose a reason for hiding this comment

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

Wouldn't the above technically be something good for a comment? EG.

# when Switching from PAD to EFT, we always create a new cfs account, cause the previous EFT cfs account  # 
 # suppose to be inactivated when it switch to PAD,

if payment_account.payment_method == PaymentMethod.EFT and cfs_account:
pay_system.update_account(name=payment_account.name, cfs_account=cfs_account,
payment_info=payment_info)
cfs_account = pay_system.create_account( # pylint:disable=assignment-from-none
identifier=payment_account.auth_account_id,
contact_info=account_request.get('contactInfo'),
Expand Down
15 changes: 15 additions & 0 deletions pay-api/tests/unit/api/test_account.py
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,21 @@ def test_update_pad_account_when_cfs_up(session, client, jwt, app):
assert rv.status_code == 200


def test_switch_eft_account_when_cfs_up(session, client, jwt, app, admin_users_mock):
"""Assert that the payment records are created with 202."""
token = jwt.create_jwt(get_claims(role=Role.SYSTEM.value), token_header)
headers = {'Authorization': f'Bearer {token}', 'content-type': 'application/json'}
rv = client.post('/api/v1/accounts',
data=json.dumps(get_basic_account_payload(payment_method=PaymentMethod.PAD.value)),
headers=headers)
auth_account_id = rv.json.get('accountId')
rv = client.put(f'/api/v1/accounts/{auth_account_id}',
data=json.dumps(get_basic_account_payload(payment_method=PaymentMethod.EFT.value)),
headers=headers)

assert rv.status_code == 200


def test_update_online_banking_account_when_cfs_down(session, client, jwt, app):
"""Assert that the payment records are created with 200, as there is no CFS update."""
token = jwt.create_jwt(get_claims(role=Role.SYSTEM.value), token_header)
Expand Down
Loading