bcgov / bcgov/entity

Entity Emailer: SonarCloud has identified code duplication

Open
#15,865 0 comments 0 reactions 0 assignees View on GitHub
ENTITY - DO NOT USE techdebt
Dominant language
JavaScript
Stars
23
Forks
62
Avg merge
24m
Merged PRs (30d)
1

Description

## What is the problem?
SonarCloud has identified some code duplication in the entity_emailer. For example, each filing has a notification handler that has a method, "_get_pdfs()" which invokes the legal-api and returns the requested documents. Similarly, each notification handler has a "process()" method which builds and populates the Jinja email template. Across notification handlers, the _get_pdfs() and process() methods, contain similar -- but not identical -- code.

## What is the impact?
Duplicating code is generally not considered good practice since it increases maintenance costs and means that a bug has to be fixed and tested in multiple places. Also, it makes it more time-consuming to onboard new team members since there's more code to learn.

## Proposed solution
Write functionally into pure functions that take a hash map and return a tuple like this:
```python
def get_receipt_pdf(**kwargs) -> tuple:
filing = kwargs.get('filing')
config = kwargs.get('config')
pdfs = kwargs.get('pdfs', [])
receipt = requests.post(
f'{config.get("PAY_API_URL")}/{filing.payment_token}/receipts',
json={
'corpName': kwargs.get('corp_name'),
'filingDateTime': kwargs.get('filing_date_time'),
'effectiveDateTime': kwargs.get('effective_date'),
'filingIdentifier': str(filing.id),
'businessNumber': kwargs.get('business_number')
},
headers=kwargs.get('legal_api_headers')
)
if receipt.status_code != HTTPStatus.CREATED:
kwargs['response'] = receipt
return False, kwargs
else:
receipt_encoded = base64.b64encode(receipt.content)
kwargs['pdfs'].append(
{
'fileName': 'Receipt.pdf',
'fileBytes': receipt_encoded.decode('utf-8'),
}
)
return True, kwargs
```

Then use a pipeline runner that can invoke a nested array of our functions. Using small, pure functions that all share the same method signature and return value makes code reuse easy and very flexible.
```python
run_pipeline([
{"try": get_receipt_pdf, "fail": [
{"try": log_failed_to_get_pdf, "fail": []},
]},
{"try": get_restoration_application_pdf, "fail": [
{"try": log_failed_to_get_pdf, "fail": []},
]},
{"try": build_restoration_email_template, "fail": []},
{"try": send_email, "fail": [
{"try": log_failed_email_send, "fail": []},
]},
], config=Config, event=event)
```

Where `run_pipeline` looks like this:
```python
def run_pipeline(functions: list, **kwargs):
"""
Recursive function that calls each node in the list.
Each node has a "try" function that is executed first. If the try
function returns True, the next node in the list is returned. If the
try function returns False, the node's "fail" list is executed in the
same way.
"""
if functions:
try_fail_node = functions.pop(0)
flag, kwargs = try_fail_node['try'](**kwargs)
if flag:
kwargs = run_pipeline(functions, **kwargs)
else:
kwargs = run_pipeline(try_fail_node['fail'], **kwargs)
return kwargs
```

Contributor guide

No contributing guide indexed for this repository

Research direction

Start by locating the entity_emailer notification handlers and comparing their _get_pdfs() and process() methods. Review the proposed run_pipeline entry point and confirm the desired function signatures; done means the duplicated handler logic is consolidated without changing email, PDF, or failure behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.