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

Remove excess data fetching in Training Module #700

Open
1 of 13 tasks
turnerrainer opened this issue Nov 7, 2024 · 5 comments · Fixed by #731
Open
1 of 13 tasks

Remove excess data fetching in Training Module #700

turnerrainer opened this issue Nov 7, 2024 · 5 comments · Fixed by #731
Assignees
Labels
enhancement New feature or request epic v2.0.2

Comments

@turnerrainer
Copy link
Contributor

turnerrainer commented Nov 7, 2024

AS AN Architect
I WANT excess data fetching in Training Module to be removed
SO THAT excess data requests, processing, logging, etc. would not overwhelm the system

Acceptance Criteria

  • There are no duplicate requests within the same page load for the same request/endpoint
  • There is no pre-fetched data in large scale to meet the functionalities of End Client's not yet taken activities

Note

For example, in case of fetching a list of intents, within the same request, there may not be any fetching of samples of intents, until the End Client has specifically chosen an intent to see intents for

Acceptance Testing

  • Dev Tools does not show any duplicate requests within the same page load
  • Ruuter logs do not show excess data fetching when logging is set to cover all requests and responses

Scope

@turnerrainer
Copy link
Contributor Author

Related issue
#729

@IgorKrupenja
Copy link
Collaborator

First PR: #731

  • Resolves most issues with excessive data fetching on intents page. Previously all intent data for all intents was fetched every time. This was very slow and not scalable. Now, I split off a separate IntentDetails component where detailed data is fetched only for the selected intent. The Intents/index.tsx itself now only fetches minimum data required to render the list.
  • Response and rule fetching is not fixed in this PR, these still fetch data for all requests. This will be implemented in later PRs.

@IgorKrupenja
Copy link
Collaborator

IgorKrupenja commented Dec 4, 2024

Notes on next steps for myself and @turnerrainer:

Remaining issues on /training/intents and /training/common-intents pages.

Description Input Estimate Issue PR
Remove excess intent fetching on /training/intents page #742 #731
Get rid of unused turn intent into service logic Vassili 1h #747 #738
Fix issues due to DataMapper rate limit Ahmed, Varmo, Alar, Rainer 4h buerokratt/DataMapper#114
Fix response fetching on /training/intents page maybe Ahmed 6h #742 #744
Fix rule fetching on /training/intents page maybe Ahmed 6h #742 #754
Fix /training/common-intents page issues using new common component 8h #743

Total: 22h

@IgorKrupenja
Copy link
Collaborator

IgorKrupenja commented Dec 11, 2024

Some statistics on work done so far:

  • /training/intents page: 3 requests now fetch data for selected intent only, not all intents.
  • /training/intents initial page load is ~85% faster. Tested when running locally, could be different when deployed due to network latency.

@IgorKrupenja
Copy link
Collaborator

IgorKrupenja commented Dec 11, 2024

Updated estimates:

Task Estimate Notes
#745 6h Infinite scroll
#746 6h Infinite scroll
#749 8h + ?* Infinite scroll + fetch stories and rules separately
#750 0 No changes necessary, task can be closed
#751 8h Infinite scroll + duplicate requests
#752 8h Infinite scroll + duplicate requests
#753 2h Duplicate requests

* - Needs further research/analysis on how to implement on Edit rules/stories pages as we discussed on call with Rasmus and @ffrose.

Total: 38h (+ yet unknown for Edit rules/stories pages)
Alternative if not implementing infinite scroll: 14h

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request epic v2.0.2
Projects
Status: In Progress
Development

Successfully merging a pull request may close this issue.

3 participants