RUN-3223: Refactor plugin to improve performance#8
Merged
Conversation
cache working, but not fetching new data unless cache is 8hrs old move call to worker replace endpoint fix parameter to dates Update joblist.js fix time range drama fix internal graphic parallel processing for roimetrics add cache cleanup Update table.html add concurrencypool add lock logic to prevent multiple workers for starting small cleanup kill roiworker upon navigating to other pages ensure that the graph per job will fetch data if needed cleanup add events save all executions in the indexed db, but filter locally the ones that actually have roi data and are successful remove the -1 from begin time fix for no roi data found message fix for html disable todays flag upon changing data range track executions total
There was a problem hiding this comment.
Pull Request Overview
This PR refactors UI templates to enhance performance and visibility control in the RD plugin.
- Animates the loading spinner icon.
- Switches the "No ROI data" message trigger to use
sortedJobs()instead ofjobs(). - Hides chart and related info until data is ready.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/main/rdplugin/resources/html/table.html | Added fa-spin to spinner icon; updated empty-state binding. |
| src/main/rdplugin/resources/html/job-roi.html | Wrapped chart and info sections with visibility bindings. |
Comments suppressed due to low confidence (2)
src/main/rdplugin/resources/html/table.html:160
- Changing the empty-state check from
jobs()tosortedJobs()may hide the message when the sort filter yields no items but there are jobs present. Confirm this logic aligns with intended behavior or add a comment to clarify.
<div class="alert alert-info" data-bind="visible: jobroilist.sortedJobs().length === 0">
src/main/rdplugin/resources/html/job-roi.html:111
- [nitpick] Embedding raw
<div>tags with visibility bindings can lead to unbalanced markup if sections change. Verify the closing tags remain matched or consider adding a comment to document the structure.
<div data-bind="visible: !loading() && hasRoiData()">
|
|
||
| <!-- Chart container with more controlled size --> | ||
| <div class="roi-chart-container"> | ||
| <div class="roi-chart-container" data-bind="visible: !loading() && hasRoiData()"> |
There was a problem hiding this comment.
[nitpick] The visible: !loading() && hasRoiData() binding is duplicated on the chart and info containers. Consider wrapping both sections in a single parent element with this binding to reduce repetition.
smartinellibenedetti
changed the title
edbaltra
approved these changes
May 30, 2025
edbaltra
left a comment
edbaltra
left a comment
There was a problem hiding this comment.
Everything looks great, I don’t have any comments
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implemented data flow:
- Fetch a single execution for each visible job
- Check if execution has ROI metrics
- For jobs with ROI metrics, fetch executions for a configurable period (default: 10 days)
- Fetch the roi metrics for each successful execution until it hits a 404 (to avoid retrieving data from executions that never had roi metrics enabled)
- Save data to IndexedDB
- Only fetch data for the missing time periods when users increase date range
- Leverage cached data when users decrease date range
- When data is already cached, fetch only today's data for fresh metrics
- Optimize network requests by batching and concurrent processing
- Concurrency pool for limiting parallel API requests (only allow 10 requests in flight);
- Jobs are processed in batches of 10; executions are processed in batches of 500;
- Background caching to maintain UI responsiveness
- Small delays (50ms) between batches to allow UI thread to breathe
Known constraints: