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

Optimize Pre-approved travel #98

Open
dpdavids opened this issue Jun 13, 2024 · 5 comments
Open

Optimize Pre-approved travel #98

dpdavids opened this issue Jun 13, 2024 · 5 comments
Milestone

Comments

@dpdavids
Copy link
Collaborator

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):

  • Browser: chrome
@klondikemarlen
Copy link
Contributor

klondikemarlen commented Jun 17, 2024

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
Copy link
Collaborator Author

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
Copy link
Contributor

@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
Copy link
Collaborator Author

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
Copy link
Contributor

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

@dpdavids dpdavids added this to the PreApproved milestone Jul 15, 2024
@klondikemarlen klondikemarlen moved this to 📋 Backlog in Travel Authorization Jul 19, 2024
@klondikemarlen klondikemarlen moved this from 📋 Backlog to 🔖 Ready in Travel Authorization Jul 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants