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.

Sign up for GitHub

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

21979 - On Test, The EFT linkage Modal Take a long time to load #1647

Merged
merged 14 commits into from
Jul 30, 2024
Merged

21979 - On Test, The EFT linkage Modal Take a long time to load #1647

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
713257e
Changes to make the query a bit faster
seeker25 Jul 26, 2024
3388d2c
add in accountIdList
seeker25 Jul 26, 2024
ab06afc
remove unused
seeker25 Jul 26, 2024
abef1b4
fix formatting
seeker25 Jul 26, 2024
c62571b
lint fixes
seeker25 Jul 26, 2024
3b1b103
Add in some additional unit tests for search, improve search performance
seeker25 Jul 26, 2024
884bb24
remove account id list
seeker25 Jul 26, 2024
f8a1305
wiring in allow_partial_account_id
seeker25 Jul 26, 2024
cc7a125
Refactor function
seeker25 Jul 30, 2024
4c28d93
small tweak
seeker25 Jul 30, 2024
8347aa2
Merge branch 'main' of https://github.com/bcgov/sbc-pay into short_na…
seeker25 Jul 30, 2024
59963c6
Small fix
seeker25 Jul 30, 2024
58d0be0
lint fixes
seeker25 Jul 30, 2024
2afee79
rename to links_subquery
seeker25 Jul 30, 2024
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
119 changes: 78 additions & 41 deletions pay-api/src/pay_api/services/eft_short_names.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -526,10 +526,10 @@ def get_statement_summary_query():
StatementModel.payment_account_id,
func.max(StatementModel.id).label('latest_statement_id'),
func.coalesce(func.sum(InvoiceModel.total - InvoiceModel.paid), 0).label('total_owing')
).outerjoin(
).join(
StatementInvoicesModel,
StatementInvoicesModel.statement_id == StatementModel.id
).outerjoin(
).join(
InvoiceModel,
InvoiceModel.id == StatementInvoicesModel.invoice_id
).group_by(StatementModel.payment_account_id)
Expand Down Expand Up @@ -572,66 +572,103 @@ def get_search_query(cls, search_criteria: EFTShortnamesSearch, is_count: bool =
# Case statement is to check for and remove the branch name from the name, so they can be filtered on separately
# The branch name was added to facilitate a better short name search experience and the existing
# name is preserved as it was with '-' concatenated with the branch name for reporting purposes
query = (db.session.query(EFTShortnameModel.id,
EFTShortnameModel.short_name,
EFTShortnameModel.created_on,
EFTShortnameLinksModel.status_code,
EFTShortnameLinksModel.auth_account_id,
case(
(EFTShortnameLinksModel.auth_account_id.is_(None),
EFTShortnameStatus.UNLINKED.value
),
else_=EFTShortnameLinksModel.status_code
).label('status_code'),
CfsAccountModel.status.label('cfs_account_status'))
.outerjoin(EFTShortnameLinksModel, EFTShortnameLinksModel.eft_short_name_id == EFTShortnameModel.id)
.outerjoin(PaymentAccountModel,
PaymentAccountModel.auth_account_id == EFTShortnameLinksModel.auth_account_id)
.outerjoin(CfsAccountModel,
CfsAccountModel.account_id == PaymentAccountModel.id))
subquery = db.session.query(
EFTShortnameLinksModel.eft_short_name_id,
EFTShortnameLinksModel.status_code,
EFTShortnameLinksModel.auth_account_id,
case(
(EFTShortnameLinksModel.auth_account_id.is_(None),
EFTShortnameStatus.UNLINKED.value
),
else_=EFTShortnameLinksModel.status_code
).label('link_status_code'),
CfsAccountModel.status.label('cfs_account_status'),
PaymentAccountModel.name,
PaymentAccountModel.branch_name) \
.outerjoin(PaymentAccountModel,
PaymentAccountModel.auth_account_id == EFTShortnameLinksModel.auth_account_id) \
.outerjoin(CfsAccountModel,
CfsAccountModel.account_id == PaymentAccountModel.id)

query = db.session.query(EFTShortnameModel.id,
EFTShortnameModel.short_name,
EFTShortnameModel.created_on)

# Join payment information if this is NOT the count query
if not is_count:
query = cls.add_payment_account_name_columns(query)
query = (query.add_columns(
subquery = cls.add_payment_account_name_columns(subquery)

subquery = (subquery.add_columns(
statement_summary_query.c.total_owing,
statement_summary_query.c.latest_statement_id
).outerjoin(
statement_summary_query,
statement_summary_query.c.payment_account_id == PaymentAccountModel.id
))

# Short name link filters
query = query.filter_conditionally(search_criteria.id, EFTShortnameModel.id)
query = query.filter_conditionally(search_criteria.account_id,
EFTShortnameLinksModel.auth_account_id,
is_like=True)
# Payment account filters
query = query.filter_conditionally(search_criteria.account_name, PaymentAccountModel.name, is_like=True)
query = query.filter_conditionally(search_criteria.account_branch, PaymentAccountModel.branch_name,
is_like=True)
subquery = subquery.filter(or_(CfsAccountModel.payment_method == PaymentMethod.EFT.value,
CfsAccountModel.id.is_(None)))
subquery = subquery.filter(
or_(PaymentAccountModel.payment_method == PaymentMethod.EFT.value, PaymentAccountModel.id.is_(None))
)

subquery = subquery.subquery()
query = query.outerjoin(subquery, subquery.c.eft_short_name_id == EFTShortnameModel.id)

if not is_count:
# Statement summary filters
query = query.filter_conditionally(search_criteria.statement_id,
statement_summary_query.c.latest_statement_id)
subquery.c.latest_statement_id)
if search_criteria.amount_owing == 0:
query = query.filter(or_(statement_summary_query.c.total_owing == 0,
statement_summary_query.c.total_owing.is_(None)))
query = query.filter(or_(subquery.c.total_owing == 0,
subquery.c.total_owing.is_(None)))
else:
query = query.filter_conditionally(search_criteria.amount_owing, statement_summary_query.c.total_owing)
query = query.filter_conditionally(
search_criteria.amount_owing, subquery.c.total_owing)

query = cls.get_link_state_filters(search_criteria, query)
query = query.filter(or_(CfsAccountModel.payment_method == PaymentMethod.EFT.value,
CfsAccountModel.id.is_(None)))
query = query.filter(
or_(PaymentAccountModel.payment_method == PaymentMethod.EFT.value, PaymentAccountModel.id.is_(None))
)
# Short name link filters
query = query.filter_conditionally(search_criteria.account_id,
subquery.c.auth_account_id,
is_like=search_criteria.allow_partial_account_id)
seeker25 marked this conversation as resolved.
Show resolved Hide resolved
# Payment account filters
query = query.filter_conditionally(
search_criteria.account_name, subquery.c.account_name, is_like=True)
query = query.filter_conditionally(search_criteria.account_branch, subquery.c.branch_name,
is_like=True)

query = query.add_columns(
subquery.c.eft_short_name_id,
subquery.c.status_code,
subquery.c.auth_account_id,
subquery.c.link_status_code,
subquery.c.cfs_account_status,
subquery.c.account_name,
subquery.c.account_branch,
subquery.c.total_owing,
subquery.c.latest_statement_id
)

if search_criteria.state:
if EFTShortnameStatus.UNLINKED.value in search_criteria.state:
seeker25 marked this conversation as resolved.
Show resolved Hide resolved
# There can be multiple links to a short name, look for any links that don't have an UNLINKED status
# if they don't exist return the short name.
query = query.filter(
~exists()
.where(subquery.c.status_code != EFTShortnameStatus.UNLINKED.value)
.where(subquery.c.eft_short_name_id == EFTShortnameModel.id)
.correlate(EFTShortnameModel)
)
if EFTShortnameStatus.LINKED.value in search_criteria.state:
query = query.filter(
subquery.c.status_code.in_([EFTShortnameStatus.PENDING.value,
EFTShortnameStatus.LINKED.value])
)

# Short name filters
query = query.filter_conditionally(search_criteria.id, EFTShortnameModel.id)
query = query.filter_conditionally(search_criteria.short_name, EFTShortnameModel.short_name, is_like=True)
if not is_count:
query = query.order_by(EFTShortnameModel.short_name.asc(), EFTShortnameLinksModel.auth_account_id.asc())
query = query.order_by(EFTShortnameModel.short_name.asc(), subquery.c.auth_account_id.asc())
return query

@classmethod
Expand Down
42 changes: 39 additions & 3 deletions pay-api/tests/unit/api/test_eft_short_names.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -661,6 +661,42 @@ def test_search_eft_short_names(session, client, jwt, app):
# create test data
data_dict = create_eft_search_data()

# Assert statement id
target_statement_id = data_dict['single-linked']['statement_summary'][0]['statement_id']
rv = client.get(f'/api/v1/eft-shortnames?statementId={target_statement_id}', headers=headers)
assert rv.status_code == 200

result_dict = rv.json
assert result_dict is not None
assert result_dict['page'] == 1
assert result_dict['stateTotal'] == 3
assert result_dict['total'] == 1
assert result_dict['limit'] == 10
assert result_dict['items'] is not None
assert_short_name(result_dict['items'][0],
data_dict['single-linked']['short_name'],
data_dict['single-linked']['accounts'][0],
data_dict['single-linked']['statement_summary'][0])

# Assert amount owing
rv = client.get('/api/v1/eft-shortnames?amountOwing=33.33', headers=headers)
assert rv.status_code == 200

result_dict = rv.json
assert result_dict is not None
assert result_dict['page'] == 1
assert result_dict['stateTotal'] == 3
assert result_dict['total'] == 1
assert result_dict['limit'] == 10
assert result_dict['items'] is not None
assert len(result_dict['items']) == 1
assert result_dict['items'][0]['shortName'] == 'TESTSHORTNAME3'
assert_short_name(result_dict['items'][0],
data_dict['multi-linked']['short_name'],
data_dict['multi-linked']['accounts'][1],
data_dict['multi-linked']['statement_summary'][1]
)

# Assert search returns unlinked short names
rv = client.get('/api/v1/eft-shortnames?state=UNLINKED', headers=headers)
assert rv.status_code == 200
Expand Down Expand Up @@ -695,7 +731,7 @@ def test_search_eft_short_names(session, client, jwt, app):
assert_short_name(result_dict['items'][1],
data_dict['multi-linked']['short_name'],
data_dict['multi-linked']['accounts'][0],
data_dict['multi-linked']['statement_summary'][0])
None) # None because we don't return a statement id if there are no invoices associated.
assert_short_name(result_dict['items'][2],
data_dict['multi-linked']['short_name'],
data_dict['multi-linked']['accounts'][1],
Expand Down Expand Up @@ -733,7 +769,7 @@ def test_search_eft_short_names(session, client, jwt, app):
assert_short_name(result_dict['items'][0],
data_dict['multi-linked']['short_name'],
data_dict['multi-linked']['accounts'][0],
data_dict['multi-linked']['statement_summary'][0])
None)

# Assert search query by no state will return all records
rv = client.get('/api/v1/eft-shortnames', headers=headers)
Expand All @@ -756,7 +792,7 @@ def test_search_eft_short_names(session, client, jwt, app):
assert_short_name(result_dict['items'][2],
data_dict['multi-linked']['short_name'],
data_dict['multi-linked']['accounts'][0],
data_dict['multi-linked']['statement_summary'][0])
None)
assert_short_name(result_dict['items'][3],
data_dict['multi-linked']['short_name'],
data_dict['multi-linked']['accounts'][1],
Expand Down
Loading