Entity Emailer: SonarCloud has identified code duplication
- 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