Skip to content

Reuse today's ROI fetch result in incremental path#14

Open
yoshi-taka wants to merge 1 commit into
rundeck-plugins:mainfrom
yoshi-taka:fix/today-double-fetch
Open

Reuse today's ROI fetch result in incremental path#14
yoshi-taka wants to merge 1 commit into
rundeck-plugins:mainfrom
yoshi-taka:fix/today-double-fetch

Conversation

@yoshi-taka
Copy link
Copy Markdown

Summary

Avoid fetching today's executions twice for the same job in the incremental ROI update path.

This change keeps the current flow mostly intact, but reuses the result from the earlier "today" fetch when the job is already confirmed to have ROI data.

Background

getExecutionsWithRoi() has an incremental path for users who already have ROI state in the browser.

In the current flow, the same job can fetch today's executions twice:

  1. once in the earlier ROI-aware check
  2. again in the later common "today data" handling path

That creates an unnecessary extra request even though the first result is still usable.

What changed

This PR makes a small, localized change:

  • keep the earlier "today" fetch result in a local variable
  • reuse that result in the later "today data" path when available
  • fall back to the existing fetch only when no earlier result exists

This keeps the existing control flow and behavior largely unchanged, while avoiding the duplicate request.

Why this approach

An alternative would be to remove the earlier fetch and rely only on the later common path, but the current structure and comments suggest that earlier ROI-aware step is part of the intended flow.

Reusing the first result preserves that structure, keeps the diff small, and avoids changing the broader cache / discovery behavior.

Expected impact

User-facing impact:

  • reduces unnecessary requests even before the ROI tab is explicitly opened, in cases where incremental ROI refresh logic runs
  • helps projects with many jobs by avoiding duplicate "today" execution lookups for the same job
  • in a project with around 200 jobs, this can mean up to about 200 fewer background requests in the incremental ROI path
  • keeps current ROI behavior and timing largely unchanged

Technical impact:

  • one fewer fetchExecutions(jobId, todayDateRange) call for jobs where the earlier ROI-aware path already fetched today's data
  • no intended change to cache TTL, preload strategy, or ROI state management design

Risk

Risk should be low because:

  • the later path still falls back to the existing fetch when no earlier result is present
  • the broader incremental update decision logic is unchanged
  • the change is limited to reusing data that was already fetched in the same flow

The main thing to watch is whether any edge case implicitly depended on the second request being issued separately, even when the same "today" data had already been retrieved.

Testing

Not fully verified with automated tests in this repo.

Manual verification target:

  • open a jobs or job detail flow where incremental ROI refresh is active
  • confirm today's executions still appear as before
  • confirm the duplicate "today" request no longer occurs for the same job

This updates the incremental ROI path to reuse the earlier fetch of today's executions when a job is confirmed to have ROI data.

The flow still performs the initial ROI status check and keeps the existing control flow, but it now carries the first today-result forward instead of issuing the same request again later.

That keeps the diff small, stays close to the current design intent, and reduces unnecessary server calls for jobs that already need today's ROI check.
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

Successfully merging this pull request may close these issues.

1 participant