feat(recap): Enables appellate attachment page purchases

[ERosendo]

Key changes.

• Introduces a new function, is_appellate_court, which determines whether a given court ID belongs to an appellate court. This helper improves code readability and maintainability by adhering to the DRY principle.

• Updates the fetch_attachment_page task and the get_att_report_by_rd method to support appellate attachment pages.

fixes #4862

[mlissner]

Looks pretty OK to me, though I might have expected another test or two, but that's not based on any knowledge of what tests we already have, just a thought.

I appreciate the addition of the helper function and the small tweaks to use it too. We should do more of this as we see these kinds of things. (It's OK to have their own PR too, if they're bigger and worth it.)

Onward to full review. :)

Thanks!

[albertisfu]

While testing this with a real Fetch request, I noticed it was failing at this part:

It worked after applying this tweak:

Suggested change

	att_report = AttachmentPage(pacer_court_id, s)
	att_report.query(rd.pacer_doc_id)
	if is_appellate_court(pacer_court_id):
	att_report = AppellateAttachmentPage(pacer_court_id, s)
	else:
	att_report = AttachmentPage(pacer_court_id, s)
	if is_appellate_court(pacer_court_id):
	att_report = AppellateAttachmentPage(pacer_court_id, s)
	else:
	att_report = AttachmentPage(pacer_court_id, s)
	att_report.query(rd.pacer_doc_id)

[ERosendo]

You're correct. Thanks!

[albertisfu]

Thanks @ERosendo this looks about right! In addition to my comments above:

• I think this is a good opportunity to remove recap_docs.json from the RecapPdfFetchApiTest class and use factories instead.
• I would recommend adding a couple of additional test cases to RecapPdfFetchApiTest to cover the district and appellate tweaks made in this PR: one to fetch a district attachment page and another to fetch an appellate attachment page. We can leverage mocks for AppellateAttachmentPage or AttachmentPage to confirm the correct report class is being called based on the court to which the request belongs. Additionally, we can use a couple of mocks for the parser methods, such as get_data_from_appellate_att_report or get_data_from_att_report, to return different data based on the court type being tested. A DictFactory, like AppellateAttachmentPageFactory, could be utilized in this process.

[albertisfu]

One last comment, as we discussed on Slack:

• In this PR we're not adding support for fetching ACMS attachment pages. Currently, such requests will fail. In the meantime, while we determine whether it will be possible to support them, would it be a good idea to add a custom error message for the failed FQ, such as: "ACMS attachment pages are not currently supported"?

[ERosendo]

Thanks for your review @albertisfu

would it be a good idea to add a custom error message for the failed FQ, such as: "ACMS attachment pages are not currently supported"?

This makes perfect sense. I checked Juriscraper and discovered that the ACMSAttachmentPage class lacks an implementation for the query method.

Furthermore, I recalled that the extension currently does not send HTML data for ACMS cases. Instead, it sends JSON. Therefore, we need to determine if it's possible to query ACMS court data and receive a JSON response. If this direct JSON retrieval is not possible, we should also implement a method to parse the necessary data from the HTML response.

[mlissner]

Can I suggest that we put the error message in place and make an issue for ACMS support for the next sprint?

[ERosendo]

Can I suggest that we put the error message in place and make an issue for ACMS support for the next sprint?

@mlissner The error message was introduced in commit 05550e8(with tests) and here's the issue: #4938

[albertisfu]

Thanks @ERosendo changes look great!