department-of-veterans-affairs / va-mobile-app

"If VA were a company, it would have a flagship mobile app."
https://department-of-veterans-affairs.github.io/va-mobile-app/
11 stars 2 forks source link

BUG - sev-2 - All - Vaccines: groupname null causes vaccines page to error out to user without throwing an error #7335

Closed TKDickson closed 8 months ago

TKDickson commented 9 months ago

What happened?

+119, one of our standard vaccine staging test users, is now displaying an error in vaccines instead of information, because some of their vaccines have null for groupname (which is what the row in teh vaccine list is labelled with).

Looks like (just guessing from the mock test data) that we're expecting an empty string, rather than null. So maybe a backend system change that breaks our assumptions?

But the API is returning real data, so 1) we shouldn't be showing an error and 2) this error is happening "invisibly" (not showing up in analytics) so we have no idea how frequently it's happening in the wild.

Specs:

Steps to Reproduce

See relevant JSON in the collapsible Charles section

Desired behavior

Should have a fallback for the vaccine row label, I'm guessing?

Also we should probably figure out how to log an error for something like this in the future :sigh:

Acceptance Criteria

Bug Severity - BE SURE TO ADD THE SEVERITY LABEL

See [Bug Tracking](https://department-of-veterans-affairs.github.io/va-mobile-app/docs/QA#issue-severity) for details on severity levels

Linked to Story

Screen shot(s) and additional information

Full JSON response for services related to issue (expand/collapse) HTTP/1.1 200 OK Date: Fri, 17 Nov 2023 21:57:26 GMT Content-Type: application/json; charset=utf-8 Etag: W/"298c5d2a3b590af365fbf50999168cca" Referrer-Policy: strict-origin-when-cross-origin Vary: Accept, Origin X-Content-Type-Options: nosniff X-Custom-Response-Header: eks-staging X-Download-Options: noopen X-Frame-Options: SAMEORIGIN X-Git-Sha: d381ead70a7d9894bc6f738b89402583cf383604 X-Github-Repository: https://github.com/department-of-veterans-affairs/vets-api X-Permitted-Cross-Domain-Policies: none X-Request-Id: 80f2a314-0e4f-48b6-8fec-ea088595dd28 X-Runtime: 1.191146 X-Xss-Protection: 1; mode=block Strict-Transport-Security: max-age=31536000; includeSubDomains; preload X-Frame-Options: SAMEORIGIN Cache-Control: no-cache, no-store Pragma: no-cache Content-Encoding: gzip Transfer-Encoding: chunked Connection: keep-alive `{"data":[{"id":"I2-D7GGP4JKRYXLCMUE3H6DSOWSMQ000000","type":"immunization","attributes":{"cvxCode":139,"date":"2023-01-02T01:54:57Z","doseNumber":"Series 1","doseSeries":null,"groupName":null,"manufacturer":null,"note":"Sample Immunization Note.","reaction":"Other","shortDescription":"Td (adult) preservative free"},"relationships":{"location":{"data":{"id":"I2-WJWPBHQZCQI6I6PA5BYIWZ7EDM000000","type":"location"},"links":{"related":"staging-api.va.gov/mobile/v0/health/locations/I2-WJWPBHQZCQI6I6PA5BYIWZ7EDM000000"}}}},{"id":"I2-FK4I5QJIWM26H5FTOPSLMGTCQA000000","type":"immunization","attributes":{"cvxCode":88,"date":"2022-05-30T01:54:57Z","doseNumber":"Series 1","doseSeries":null,"groupName":null,"manufacturer":null,"note":"Sample Immunization Note.","reaction":"Other","shortDescription":"Influenza, seasonal, injectable, preservative free"},"relationships":{"location":{"data":{"id":"I2-DIA2RBFDB7J2IVNOP6WTENLNGA000000","type":"location"},"links":{"related":"staging-api.va.gov/mobile/v0/health/locations/I2-DIA2RBFDB7J2IVNOP6WTENLNGA000000"}}}},{"id":"I2-EOWF2PZHM5ONA4RLH3LIQEADMU000000","type":"immunization","attributes":{"cvxCode":213,"date":"2021-03-01T01:54:57Z","doseNumber":"Series 1","doseSeries":null,"groupName":null,"manufacturer":null,"note":"Sample Immunization Note.","reaction":"Other","shortDescription":"SARS-COV-2 (COVID-19) vaccine, mRNA, spike protein, LNP, preservative free, 100 mcg/0.5mL dose"},"relationships":{"location":{"data":{"id":"I2-Y24GUB4YYUJ2H5FESXPGTYQKEE000000","type":"location"},"links":{"related":"staging-api.va.gov/mobile/v0/health/locations/I2-Y24GUB4YYUJ2H5FESXPGTYQKEE000000"}}}},{"id":"I2-DQH47R2YFRKSJUBTTYAT5K2LNE000000","type":"immunization","attributes":{"cvxCode":213,"date":"2021-03-29T01:54:57Z","doseNumber":"Series 1","doseSeries":null,"groupName":null,"manufacturer":null,"note":"Sample Immunization Note.","reaction":"Other","shortDescription":"SARS-COV-2 (COVID-19) vaccine, mRNA, spike protein, LNP, preservative free, 100 mcg/0.5mL dose"},"relationships":{"location":{"data":{"id":"I2-Y24GUB4YYUJ2H5FESXPGTYQKEE000000","type":"location"},"links":{"related":"staging-api.va.gov/mobile/v0/health/locations/I2-Y24GUB4YYUJ2H5FESXPGTYQKEE000000"}}}},{"id":"I2-JZQ6CBLBUUB6BT7BRK6EARA6TM000000","type":"immunization","attributes":{"cvxCode":88,"date":"2021-12-27T01:54:57Z","doseNumber":"Series 1","doseSeries":null,"groupName":null,"manufacturer":null,"note":"Sample Immunization Note.","reaction":"Other","shortDescription":"Influenza, seasonal, injectable, preservative free"},"relationships":{"location":{"data":{"id":"I2-WJWPBHQZCQI6I6PA5BYIWZ7EDM000000","type":"location"},"links":{"related":"staging-api.va.gov/mobile/v0/health/locations/I2-WJWPBHQZCQI6I6PA5BYIWZ7EDM000000"}}}},{"id":"I2-EHZOHANICONL3KOGVDEPUUG3K4000000","type":"immunization","attributes":{"cvxCode":88,"date":"2020-12-21T01:54:57Z","doseNumber":"Series 1","doseSeries":null,"groupName":null,"manufacturer":null,"note":"Sample Immunization Note.","reaction":"Other","shortDescription":"Influenza, seasonal, injectable, preservative free"},"relationships":{"location":{"data":{"id":"I2-WJWPBHQZCQI6I6PA5BYIWZ7EDM000000","type":"location"},"links":{"related":"staging-api.va.gov/mobile/v0/health/locations/I2-WJWPBHQZCQI6I6PA5BYIWZ7EDM000000"}}}},{"id":"I2-6NNWZQ5TJWSI4PORSPZ5ITABOI000000","type":"immunization","attributes":{"cvxCode":88,"date":"2018-12-10T01:54:57Z","doseNumber":"Series 1","doseSeries":null,"groupName":null,"manufacturer":null,"note":"Sample Immunization Note.","reaction":"Other","shortDescription":"Influenza, seasonal, injectable, preservative free"},"relationships":{"location":{"data":{"id":"I2-WJWPBHQZCQI6I6PA5BYIWZ7EDM000000","type":"location"},"links":{"related":"staging-api.va.gov/mobile/v0/health/locations/I2-WJWPBHQZCQI6I6PA5BYIWZ7EDM000000"}}}},{"id":"I2-E7DEV4VXFBF5R3KTKI2SG26AM4000000","type":"immunization","attributes":{"cvxCode":88,"date":"2017-12-04T01:54:57Z","doseNumber":"Series 1","doseSeries":null,"groupName":null,"manufacturer":null,"note":"Sample Immunization Note.","reaction":"Other","shortDescription":"Influenza, seasonal, injectable, preservative free"},"relationships":{"location":{"data":{"id":"I2-WJWPBHQZCQI6I6PA5BYIWZ7EDM000000","type":"location"},"links":{"related":"staging-api.va.gov/mobile/v0/health/locations/I2-WJWPBHQZCQI6I6PA5BYIWZ7EDM000000"}}}},{"id":"I2-25EWJWOEARZWHQDLL4FLMGU544000000","type":"immunization","attributes":{"cvxCode":88,"date":"2016-11-28T01:54:57Z","doseNumber":"Series 1","doseSeries":null,"groupName":null,"manufacturer":null,"note":"Sample Immunization Note.","reaction":"Other","shortDescription":"Influenza, seasonal, injectable, preservative free"},"relationships":{"location":{"data":{"id":"I2-WJWPBHQZCQI6I6PA5BYIWZ7EDM000000","type":"location"},"links":{"related":"staging-api.va.gov/mobile/v0/health/locations/I2-WJWPBHQZCQI6I6PA5BYIWZ7EDM000000"}}}},{"id":"I2-QNXXY74AFW4RIUFDTTG3WWAFGE000000","type":"immunization","attributes":{"cvxCode":88,"date":"2015-05-11T01:54:57Z","doseNumber":"Series 1","doseSeries":null,"groupName":null,"manufacturer":null,"note":"Sample Immunization Note.","reaction":"Other","shortDescription":"Influenza, seasonal, injectable, preservative free"},"relationships":{"location":{"data":{"id":"I2-Y24GUB4YYUJ2H5FESXPGTYQKEE000000","type":"location"},"links":{"related":"staging-api.va.gov/mobile/v0/health/locations/I2-Y24GUB4YYUJ2H5FESXPGTYQKEE000000"}}}}],"meta":{"pagination":{"currentPage":1,"perPage":10,"totalPages":2,"totalEntries":18}}}`

Ticket Checklist

TKDickson commented 9 months ago

Adding backend label - FYI @jperk51 and @kellylein - since it looks like we were expecting an empty string rather than null. Keeping the frontend label - FYI @timwright12 - because idk how resilient our FE should be.

alexandec commented 9 months ago

@TKDickson we have a check we run after fetching data from the /v1/health/immunizations endpoint which is:

// Ensure all required fields(date, groupName, and type and dosage) exist; otherwise throw API Error
if (!hasRequiredFields) {
  throw { status: 500, json: { errors: [], text: '{vaccineSlice:{Missing required fields(date, groupName, and type)}}' } } as APIError
}

Specifically the required fields check is:

const hasRequiredFields = vaccinesData?.data.every((v) => {
  return !!v.attributes?.date && !!v.attributes?.groupName && !!v.attributes?.shortDescription
})

So if date, groupName, or shortDescription is falsy (null, undefined, empty string, false, zero) then we throw an error. Do we want to remove any/all of those three checks?

TKDickson commented 9 months ago

Interesting.... @kpethtel chimed in on Friday that apparently the documentation says groupName can be empty (this slack thread, after the lonnnnng charles stuff). IDK if that's true for the other two as well, should check on that

If we've got those checks currently, then we probably would need to check in with UX about how to handle the list/details display for when whichever of those fields can be falsy, are falsy, as well before tackling this.

TKDickson commented 9 months ago

bah. @alexandec ^

alexandec commented 9 months ago

@TKDickson Here's how Vaccine Details looks if I remove the checks and set groupName to null:

Simulator Screenshot - iPhone 13 - 2023-12-11 at 13.36.50.png

And if I disable the checks, then set date, groupName, and shortDescription to null:

Simulator Screenshot - iPhone 13 - 2023-12-11 at 13.39.44.png

The list view looks like this with checks disabled and groupName set to null:

Simulator Screenshot - iPhone 13 - 2023-12-11 at 13.43.30.png

And with all three fields set to null:

Simulator Screenshot - iPhone 13 - 2023-12-11 at 13.41.05.png

Probably not perfect, but certainly better than the whole feature breaking which is what happens currently. I would lean towards removing the checks to get the feature working, and then following up later to improve the UX. What do you think?

TKDickson commented 9 months ago

@alexandec sadly all of those are broken URLs for me: Non-Image content-type returned

alexandec commented 9 months ago

@TKDickson ZenHub strikes again, haha. I re-uploaded them using GitHub, see if they work for you now

TKDickson commented 9 months ago

Success! Screenshots are better than 'entire activity broken', it's true. So @alexandec sounds good to me. I will spin up a ticket for the UX portion and drop a link in here.

TKDickson commented 8 months ago

UX improvement ticket is #7508

DJUltraTom commented 8 months ago

I wasn't assigned on this ticket so it was not on my radar until I got pinged about it.

Initial look is promising as its actually loading now, but the RX list view is nowhere close to being informative or close to the original design spec. I'll dig in with charles sometimes tomorrow and see what's being sent vs what we're sending.

DJUltraTom commented 8 months ago

okay, Grabbed the charles logs and it looks like we are displaying what we are getting from the service correctly. I'm guessing there has been some upstream change as I'm not seeing ANY groupname being returned on any of the test data/users we have. Our fix is good. QA approves this fix

DJUltraTom commented 8 months ago

verified changes look as expected on the overnight Dev build, Closing as deployed