-
-
Notifications
You must be signed in to change notification settings - Fork 182
feat(recap): Adds support for ACMS attachment page purchases #5971
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
feat(recap): Adds support for ACMS attachment page purchases #5971
Conversation
Update `get_att_report_by_rd` to handle ACMS documents by selecting the `ACMSAttachmentPage` class when appropriate.
This commit updates attachment page handling logic to accommodate ACMS documents by skipping HTML parsing and using structured data from the response. The appropriate document number is extracted from the ACMS payload, and the data is passed to `merge_attachment_page_data` with an `is_acms_attachment` flag. This enables seamless processing of ACMS attachment pages alongside existing district and appellate workflows.
This test verifies that ACMS attachment pages are correctly processed using the `ACMSAttachmentPage` class and that no district or appellate parsers are called. It also ensures the fetch is marked successful and the expected number of RECAP documents are created.
809b2e2
to
8f2912a
Compare
Great. Thank you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ERosendo this looks great.
Just a couple of minor comments and suggestions.
cl/recap/tests/tests.py
Outdated
"cl.recap.tasks.get_pacer_cookie_from_cache", | ||
return_value=None, | ||
) | ||
def test_fetch_att_page_no_cookies( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason to remove this test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry about that — I accidentally removed it while deleting the other test related to ACMS purchases
document_number = None | ||
else: | ||
document_number = att_data["document_number"] | ||
|
||
try: | ||
async_to_sync(merge_attachment_page_data)( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be a good idea to update the PacerHtmlFiles
storage for the attachment data within merge_attachment_page_data
, so the attachment data is saved as JSON, similar to what you did for the docket data?
docket_case_id = rd.docket_entry.docket.pacer_case_id | ||
rd_entry_id = rd.pacer_doc_id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we start ingesting ACMS email notifications that don't include a pacer_case_id
, we'll retrieve it using the AcmsCaseSearch
API. However, the pacer_doc_id
will be empty for documents merged from ACMS email notifications.
So, I think it would be a good idea to check these values before querying the report as a safeguard. If either is missing, we could raise an error and store it in the FQ, so users understand why the fetch can't complete.
It might be better to perform this check in fetch_attachment_page
, since that’s where these values are initially retrieved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, I think it would be a good idea to check these values before querying the report as a safeguard.
Great idea — thanks! I'll update the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @ERosendo this now looks great. Set to auto-merge.
Key changes:
Removes the early return in
fetch_attachment_page
for ACMS requests, allowing ACMS attachments to be properly processed.Updates
fetch_attachment_page
logic to support ACMS documents by skipping HTML parsing and instead using structured data from the ACMS response. The appropriate document number is extracted from the ACMS payload and passed tomerge_attachment_page_data
along with theis_acms_attachment
flag.Refines get_att_report_by_rd to introduce logic to detect ACMS documents and select the appropriate
ACMSAttachmentPage
class. ACMS attachments are queried using bothpacer_case_id
andpacer_doc_id
.Depends on #5960 and freelawproject/juriscraper#1495
Fixes #4938