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.

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

703 Added RECAP Fetch webhook event #2453

Merged
merged 9 commits into from Jan 10, 2023
Merged

Conversation

albertisfu
Copy link
Contributor

@albertisfu albertisfu commented Jan 9, 2023

This PR adds support for RECAP Fetch webhook events as commented in #703.

The webhook event has the following structure:

{
   "payload":{
      "id":803712,
      "court":null,
      "docket":null,
      "recap_document":221797284,
      "date_created":"2023-01-06T09:06:23.854208-08:00",
      "date_modified":"2023-01-06T09:06:24.760189-08:00",
      "date_completed":"2023-01-06T09:06:24.760078-08:00",
      "status":2,
      "request_type":2,
      "message":"Successfully completed fetch and save.",
      "pacer_case_id":"",
      "docket_number":"",
      "de_date_start":null,
      "de_date_end":null,
      "de_number_start":null,
      "de_number_end":null,
      "show_parties_and_counsel":true,
      "show_terminated_parties":true,
      "show_list_of_member_cases":false
   },
   "webhook":{
      "version":1,
      "event_type":3,
      "date_created":"2022-12-28T14:21:40.855097-07:00",
      "deprecation_date":null
   }
}

Is triggered when the fetch task is marked as completed when one of the following statuses:

SUCCESSFUL, FAILED, INVALID_CONTENT, or NEEDS_INFO

Webhook events are not triggered on the following statuses:
ENQUEUED, IN_PROGRESS, or QUEUED_FOR_RETRY

  • Added documentation for this new webhook event.
  • Added dummy webhook data to send webhook test events.

Working on this I detected some bugs and changes that were needed:

  • Removed mark_fq_status (SUCCESSFUL) at the end of fetch_docket to avoid duplicated webhook events since there is a mark_fq_successful task within do_pacer_fetch for fetch docket chain.
  • I found a bug within fetch_docket task, when cookies expired the fq object was marked as failed but the task execution and following tasks in the chain continue. When the task and consecutive tasks in the chain should be aborted, I solved it by returning None if there are no cookies. The reason tests were not catching this problem is that within a test all the tasks in a chain are executed so the mark_fq_successful task was being executed even though the fetch failed.
  • Fixed tests for RecapDocketFetchApiTest adding a mock for cookies and adding an additional assert that checks if a RECAPDocument is created when fetching the docket instead of only relying on the FQ SUCCESSFUL status.
  • I found what seems to be another bug, when fetching a docket only by pacer_case_id and court it was not working since the pacer_case_id provided in the requests was not being used, so I added it in the first place here: tasks.py#L1695, so that if the pacer_case_id is passed in the fetch request it'll be used. The test for this test_fetch_docket_by_pacer_case_id was not failing for the same reason described above related to mark_fq_successful task. So I also added an additional assert to confirm the docket is properly fetched.

Let me know what you think.

Copy link
Member

@mlissner mlissner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple small tweaks, please. I also pushed some updates to the docs that you'll see.

cl/recap/tasks.py Outdated Show resolved Hide resolved
cl/recap/utils.py Outdated Show resolved Hide resolved
@albertisfu albertisfu force-pushed the 703-add-recap-fetch-wehook-event branch from 3a1e80d to 1c53e9c Compare January 10, 2023 00:18
@albertisfu
Copy link
Contributor Author

Thank you! I've applied the requested changes.

The one related to moving send_recap_fetch_webhook_event is applied on #2456 that I set to be merged into this one.

cl/recap/tasks.py Outdated Show resolved Hide resolved
@mlissner
Copy link
Member

Just merged the refactor into this one. Let me know when you feel good about this one if you don't already.

@albertisfu albertisfu force-pushed the 703-add-recap-fetch-wehook-event branch from 543cbbc to f2e0ba1 Compare January 10, 2023 19:30
@albertisfu albertisfu force-pushed the 703-add-recap-fetch-wehook-event branch from f2e0ba1 to e654bc9 Compare January 10, 2023 20:12
@albertisfu
Copy link
Contributor Author

Thanks for your comments, I have applied the suggestion and this seems ready!

@mlissner mlissner merged commit 6d0b76c into main Jan 10, 2023
@mlissner mlissner deleted the 703-add-recap-fetch-wehook-event branch January 10, 2023 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants