CDCgov / prime-reportstream

ReportStream is a public intermediary tool for delivery of data between different parts of the healthcare ecosystem.
https://reportstream.cdc.gov
Creative Commons Zero v1.0 Universal
65 stars 39 forks source link

Bugs in delivery list, facilities and deliver details / endpoints #6252

Open jimduff-usds opened 1 year ago

jimduff-usds commented 1 year ago

Describe the bug

Issues fixed in the #6632 PR:

  1. Bug: There are two filenames listed in deliver details. One is wrong, not sure which.
  2. Bug: item_count not working
  3. Bug: topic is not working
  4. Delivery list returns the list of files sent, not the list of files waiting for delivery. This means 1) Receivers with no transport get an empty list and therefore 2) Receivers with no transport cannot manually download their data. and 3) the links to download are wrong, because they should point to files that have finished the batch step, not files sent. (Relevant code is
       var filter = ACTION.ACTION_NAME.eq(TaskAction.send)
  5. in DatabaseDeliveryAccess.kt) api/waters/report/<reportId>/delivery does not work for valid delivered reportId.
  6. Why is the fileName different from the externalName? Any filename we show should be the one that an external user would see if they went into the blobstore. So one of those names should be removed - they can't both be right.)

Other issues found

  1. Test/QA issue: Does "positives" work?
  2. Discussion with UI team not done: Get rid of expires
  3. Bug: various deliveries queries require the receiver fullname. Eg. api/waters/org/<orgname>/deliveriesdoes not work for valid orgnames. (It does work for orgname.recievername)
  4. We need smoke testing that actually tests correctness of data returned by the APIs, not just did the API respond.
  5. I'm not able to get showfailed to work. Its listed as showFailed in the openapi spec, but the code has showfailed. Neither works for me. (??? I think this applies to SubmissionHistory, not DeliveryHistory)
  6. The Facility list does not collapse down (aka "group by") like data. Instead it lists the same facility over and over. See screenshot below. Also the total-tests often does not match the Total Tests.
  7. DeliveryDetails: auth fails for "regular" users (non-primeadmin). it checks for a Sending org, when it needs to query the report_file table to get the receiving_org. (See DeliveryFacade::checkAccessAuthorization). I have partly refactored this code, but have not written the query yet to pull the right auth.1. The submission features need to be fully re-tested as well, since the code is shared between the two. We need a smoke test that includes full tests for correct data in the API response.
  8. DeliveryDetails: Basic Internal Feature Missing: The Delivery Details does not actually give any deliver details! It is currently basically the same data as the deliver list. Deliver Details should include a mapping back to the original reports submitted, and details about them. (Symmetric to have the Submission details includes a mapping back to the reports delivered, and details about them). Then you get the Facilities query "for free" with a fast query to the covid_result_metadata table, because you already have the list of submitted report_IDs and action_ids.
  9. Incomplete: No release notes
  10. Incomplete: No updated examples
  11. Feature not done: List of facilities over long-term history: never done. This is the query which may not perform well.
  12. Subtle minor bug: Reports can in theory be sent without ever being batched, with the send-immediately feature, or by having a transport but no timing setting. No such cases exist at the moment. If they do, they will be missed by the api/waters/org/ORG.RECEIVER/deliveries API call. (this was true of the old call as well)
  13. Subtle minor bug: The "batch" step sometimes actually breaks apart reports. In particular, for states that can only receive one HL7 message per file. In those cases, the call to fetchReportForActionId, when used in the delivery details query, can return erroneous results if queried using an ActionId, because one batch action creates many reports. I think the solution is to eliminate delivery queries based on actionIds. There's no strong use case for them.

Other notes

Impact

Delivery API endpoints are not usable.

Here's an example of what the API was returning prior to fixing anything:

 {
  "deliveryId" : 3155171,
  "sent" : "2022-07-27T22:23:05.791Z",
  "expires" : "2022-08-26T22:23:05.791Z",
  "receivingOrg" : "cdc",
  "receivingOrgSvc" : "dcipher-monkeypox",
  "reportId" : "f3320b16-ff31-4057-a7e1-2d6892f40d29",
  "topic" : null,
  "reportItemCount" : null,
  "fileName" : "dcipher-monkeypox-f3320b16-ff31-4057-a7e1-2d6892f40d29-20220727222305.csv",
  "fileType" : "CSV",
  "externalName" : "https://pdhstagingpartner.blob.core.windows.net/dcipher/dcipher-monkeypox-20701fba-6c89-4baa-ae0f-b1f6ed215429-20220727222259.csv"
},

Steps to Reproduce one of the above issues:

Do a list query against api/waters/org/md-phd.elr/deliveries. Then grab that reportId from above, and put it into this call: curl -H "authentication-type: okta" -H "Authorization: Bearer $TOK" "https://staging.prime.cdc.gov/api/waters/report/f3320b16-ff31-4057-a7e1-2d6892f40d29/delivery" That should work, but instead I get this error: {"error": "f3320b16-ff31-4057-a7e1-2d6892f40d29 is not a submitted report"} This is doubly wrong. I'm not making a submission query so why is the error saying anything at all about submissions?Furthermore, it is a valid delivery report, and I should get back a detailed response.

jimduff-usds commented 1 year ago

Bug 17. On a whim, I grabbed a submission reportId and sent it to the api/waters/report/????/delivery endpoint. This should give an error. It does not.

For this submission to RS Staging:

{
  "submissionId" : 3154138,
  "timestamp" : "2022-07-26T16:12:15.138Z",
  "id" : "c01d9fd4-ccf8-4249-9b97-a94012278242",
  "topic" : "monkeypox",
  "reportItemCount" : null,
  "sender" : "sonic",
  "httpStatus" : 201
} ]

If I then use that report to query the delivery endpoint:

curl -H "authentication-type: okta" -H "Authorization: Bearer $TOK" "https://staging.prime.cdc.gov/api/waters/report/c01d9fd4-ccf8-4249-9b97-a94012278242/delivery"

This should return an error, but instead I get this back, but erroneously labelled as if its a delivery!

{
  "deliveryId" : 3154138,
  "sent" : "2022-04-12T17:06:10.534Z",
  "expires" : "2022-05-12T17:06:10.534Z",
  "receivingOrg" : "sonic",
  "receivingOrgSvc" : "elr-secondary",
  "reportId" : "c3c8e304-8eff-4882-9000-3645054a30b7",
  "topic" : "covid-19",
  "reportItemCount" : 1,
  "fileName" : "covid-19-c3c8e304-8eff-4882-9000-3645054a30b7-20220412170610.hl7",
  "fileType" : "HL7_BATCH",
  "externalName" : null
}
luis-pabon-tf commented 1 year ago

Gonna split this ticket into multiple smaller ones, one per issue

luis-pabon-tf commented 1 year ago

Made related tickets for the issues I was able to replicate on my branch with #6178 changes.

jimduff-usds commented 1 year ago

Here's an example showing the same Facility over and over. This is from Staging.

image
jimduff-usds commented 1 year ago

Bug: Here's an example where the "Total tests" does not add up to the "Total Tests Reported":

image
avnieldravid commented 1 year ago

@jimduff-usds is this ticket still valid?

bishoyayoub commented 1 year ago

@arnejduranovic Please review.