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/
13 stars 2 forks source link

Improve Home screen personalization API call efficiency #8143

Closed TKDickson closed 3 months ago

TKDickson commented 6 months ago

What happened?

General "why this matters" Personalization of the home screen involves calling a lot of APIs that weren't previously called, very early in a user's session. Initial analytics look like we're increasing full load times for home screen by ~5-6 seconds, which is a LOT. Therefore, we should be doing work now before HSP launch to make sure that we're as efficient as possible with our API calls, and that the relationship between the API calls & what's displaying on the screen maximizes "useful screen content showing" even when APIs are still not fully finished.

When we loop back around to do the work of implementing/enforcing a refresh behavior (ex: calling APIs when reloading the home screen vs the 'cheaty way' of keeping track of veteran activity during the session), we should take the opportunity to clean up when/how the various HSP-necessary APIs are being called.

Things I've found:

Acceptance Criteria

TKDickson commented 6 months ago

This ticket may/will end up needing to be split into several different tickets. Will definitely need to go through a grooming session. I will ping Ameet for the next one.

TKDickson commented 5 months ago
TKDickson commented 3 months ago

Wanted to get a record of what the behavior is for initial login (with onboarding), subsequent login UN/PW (no onboarding), and subsequent login with biometrics.

Initial login

Between webview handoff & FaceID popup:

Between 'allow FaceID' to first onboarding screen:

Between 'skip' in Onboarding and Home screen load

TKDickson commented 3 months ago

Biometric login - alllllll of the things fire in very rapid succession (sorted in reverse chronological order by start time column, so the newest is on the top):

image.png
TKDickson commented 3 months ago

UN/PW subsequent login (not the same user as above) - they sent off in a sequenced, but tightly/efficiently, way - and the home screen was fully rendered even though the past claims was still going (win):

image.png
TKDickson commented 3 months ago

Also important - none of these logins had any of the JWT token errors :tada:

TKDickson commented 3 months ago

OK so - on initial login, military-service-history fires twice. Once after you answer the biometrics prompt while the onboarding screen is being pulled up (aka at the same time as basically all of the homescreen calls), and then AGAIN after you get through onboarding/the home screen is loading. That's only if I'm going slowly (ex: reading the various screens in between) and when I just tap tap tap tap tap through things, it doesn't - so.... maybe it's because I'm hitting the 'more than 5 seconds' cache timing with it?

Next set of cases - waiting more than 5 minutes at each pause point during initial login (to be over the max cache timer for all things) and seeing if that handles as I'd expect.

TKDickson commented 3 months ago

Super interesting. If I wait for 5 minutes at all pause points (biometrics prompt, and then at onboarding), the APIs that I'd expect to be called again, are - for instance, v2/user gets called each time. However - if I sit on onboarding for 5+ minutes - I also get a UI thing I don't expect - where the home screen shows a null state briefly, before switching into the loading state. ( @htcollier said that she saw this with users during research, so it could very well not be caused by this branch. I haven't looked into that yet)

If I move quickly-but-reasonably through the screens (ex: reading onboarding) then typically I see the fully-loaded home screen with correct data when it's revealed. If I RUSH through screens, then I get the loading state (also expected) when I see the home screen the first time.

TKDickson commented 3 months ago

v2/user/authorized-services is the key "we wait for that response" before transitioning from the sync screen, to the home screen, on subsequent login. Makes sense to me. All of the other "wait for X to do Y" is the same as the initial ACs for home screen implementation.

We don't currently have an error state/loading state for "we don't know if you have cerner or not" so I'm going to ask Holly about that one.

TKDickson commented 3 months ago

OK confirmed that "waiting at onboarding" screen happens pre-this-development (and I even think I have a guess why!) so will be writing that up separately - #8811

So @theodur ignore the wall(s) of text. All you need to know is:

theodur commented 3 months ago

From the work in prior tickets, there weren't many unnecessary/inefficient API calls being made. Primarily the v2/user endpoint was being called twice because the query stale time was 5 seconds, so it would be called again once login completes on the Sync screen, and another time on the home screen after loading the Nametag component (which needs to wait for the other API calls to finish required for the About you section). I increased the v2/user query stale time to 5 minutes for now, but the final stale time for that will be decided in https://github.com/department-of-veterans-affairs/va-mobile-app/issues/8638.

The main optimization I made was starting the home screen API calls on the Sync screen whenever sign in is completed and the downtime windows API call is finished. This ensures we're making the API calls as soon as possible, so that we don't need to wait for the Home screen to mount before the API calls are started. This seemed to shave off a little bit of time

Also FYI, this was supposed to be posted last week when I finished my work on this (before you started QA'ing Therese). Guess the comment didn't go through 😅

theodur commented 3 months ago

@TKDickson Sounds like military service history is in the same boat that the v2/user endpoint was in. That makes sense because both of those queries are in the Nametag component, and from the time it takes after the first initial service history API call to the time that the About you section is finished loading often exceeds 5 seconds. The Nametag component references the military service query, but it doesn't get rendered until the About you section API calls are finished loading, so it basically recalls the API because the cache stale time is up by the time Nametag is rendered and tries to reference the query. Service history and the v2/user are the only queries referenced in Nametag, so they are the only ones that would have this issue.

For now I'll also go ahead and increase the cache stale time for service history to 5 minutes, but the final stale time for that will probably be decided in the cache time ticket (#8638).

theodur commented 3 months ago

Super interesting. If I wait for 5 minutes at all pause points (biometrics prompt, and then at onboarding), the APIs that I'd expect to be called again, are - for instance, v2/user gets called each time. However - if I sit on onboarding for 5+ minutes - I also get a UI thing I don't expect - where the home screen shows a null state briefly, before switching into the loading state. ( @htcollier said that she saw this with users during research, so it could very well not be caused by this branch. I haven't looked into that yet)

Yeah, I've also noticed this issue on the base feature XL branch. I fixed it in the loading animation glitch ticket (#8746)

TKDickson commented 3 months ago

Aha, very helpful notes - that's what I kind of figured/was testing with those types of assumptions.

The increase in military history timer worked, and generally this is all WAY more efficient/neatly ordered, so I'm good with merging this to the feature branch.

TKDickson commented 3 months ago

Merged to feature branch ; closing.