search

ytgov / travel-authorization

0 stars 3 forks source link

Optimize Pre-approved travel #98

Open dpdavids opened 3 months ago

dpdavids commented 3 months ago

Describe the bug When I select the preapproved travel from the global nav. for the first time the page/grid takes about 10seconds to load. I vaguely recall Max suggesting that the full AD list is loaded at this point which is why it takes so long.

To Reproduce Steps to reproduce the behavior:

  1. Go to 'Pre-approved travel' on the Global navigation dropdown
  2. See the duration of time for the first load

Expected behavior I would expect that the load time would be much less then 10 sec. In the 1second range.

Screenshots If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

klondikemarlen commented 3 months ago

I can confirm this; the page does indeed do a bunch of lookups to https://api.gov.yk.ca/ that are quite expensive, and then caches them after first lookup, at least until the app restarts. See api/src/routes/lookup-router.ts.

The lookup/cache system is implemented in several different ways throughout the app. A solution to this would be to:

  1. standardize the lookup system from the YG api.
  2. store the data locally in the database as it done in other apps (such as Internal Data Portal)
  3. run the sync once a day via a background job
  4. provide Admin's the ability to trigger directory sync manually if they want it to happen faster.

    Note that this system is independent of the the User profile sync feature.

dpdavids commented 3 months ago

hey @klondikemarlen I am not sure the best way to approach this. If #1 is the best solution I am happy to follow up with the YG directory API owner to make it more useful. Particularly if it will help further development using this api. 2 or 3 are fine as well if this is already how things are managed in other systems. I think #4 is a no go as no admin is going to remember to do this.

I think what max suggested is that the caching didn't need to happen before the grid loaded. The grid is not using the AD information it is the underlying form that would use this information. Can the grid draw and then we cache AD in the background? OR when it is being used for a new form?

klondikemarlen commented 3 months ago

@dpdavids Bit of confusion here, I wasn't proposing a list of options, but rather a list of tickets required for a complete solution.

By "standardize the lookup system from the YG api." I mean that the Travel Authorization project code needs some cleanup, not that the YG API needs to change. The code in this part of the app is doing a lot of things that are both confusing and really inefficient. So performing some cleanup and using best practices will remove most of the speed problems.

However, there is no getting around the issue of needing to keep the local "cache" (local clone of YG directory info) in sync with the remote YG directory info. So the cache will need to be synced nightly (or manually if Admins want an earlier sync).

dpdavids commented 3 months ago

Ahh @klondikemarlen that makes more sense. Does it make sense to strip these into separate tickets? Were we make #1 update early and add #2-4 for a later iteration?

klondikemarlen commented 3 months ago

@dpdavids Yeah, that would be perfect, its always easier to make forward progress when the ticket is as small and targeted as possible.