Skip to content

SRVKP-12328: Add date range filter for PipelineRuns list page#1133

Open
adityavshinde wants to merge 1 commit into
openshift-pipelines:masterfrom
adityavshinde:feat/pipeline-run-date-filter
Open

SRVKP-12328: Add date range filter for PipelineRuns list page#1133
adityavshinde wants to merge 1 commit into
openshift-pipelines:masterfrom
adityavshinde:feat/pipeline-run-date-filter

Conversation

@adityavshinde

Copy link
Copy Markdown
Contributor

Summary

Adds a time range filter to the PipelineRuns list page, allowing users to filter pipeline runs
by time period (Last day, Last week, Last month, Last quarter, Last year).

  • Integrated as a standard filter category in the existing filter toolbar (alongside Status, Data Source)
  • Uses single-select dropdown behavior (picks one time range at a time)
  • Server-side filtering via CEL expression for Tekton Results data
  • Client-side filtering for Kubernetes/etcd data
  • User's selection is persisted across sessions via useUserPreference (stored in ConfigMap per user)
  • Added singleSelect support to CheckboxFilterConfig for reuse by other filters

Changed files

  • useDateRangeFilter.ts (new) - Hook managing time range state, persistence, and CEL generation
  • useTektonResult.ts - Accepts and forwards filter parameter to downstream hooks
  • useTaskRuns.ts - Threads filter to Tekton Results API, combines with name filter via AND()
  • DataViewFilterToolbar.tsx - Added singleSelect mode to CheckboxFilterInput
  • PipelineRunsList.tsx - Integrates time range into the filter system

Screen Recording

RFE-7628.mp4

Test plan

  • Selecting a time range filters PipelineRuns correctly
  • Runs without startTime (pending) are preserved
  • Filter persists across tab switches and page refreshes
  • Works with both Tekton Results enabled and disabled
  • Clear filters resets time range to default (Last day)
  • No flash of default value on navigation
  • Unit tests pass (11 tests)
  • TypeScript compiles clean

@qodo-code-review

qodo-code-review Bot commented Jun 16, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (4) 📘 Rule violations (2) 📜 Skill insights (0)

Grey Divider


Action required

1. Missing i18n key Time range 📘 Rule violation ⚙ Maintainability
Description
The new user-facing string t('Time range') was added without a corresponding entry in the
committed i18n artifacts, so it will not be translated in non-default locales. This violates the
requirement to regenerate/commit translations after adding or modifying strings.
Code

src/components/pipelineRuns-list/PipelineRunsList.tsx[R113-115]

+        id: 'timeRange',
+        title: t('Time range'),
+        singleSelect: true,
Evidence
PipelineRunsList.tsx introduces a new translated label t('Time range'), but the committed
English locale file contains Time Range (different casing) and does not provide an entry for `Time
range`, indicating i18n artifacts were not regenerated/updated for the new string.

AGENTS.md: Follow i18n namespace and regenerate translations after string changes: AGENTS.md: Follow i18n namespace and regenerate translations after string changes
src/components/pipelineRuns-list/PipelineRunsList.tsx[113-115]
locales/en/plugin__pipelines-console-plugin.json[576-580]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A new translatable UI string `t('Time range')` was introduced, but the locale JSON artifacts do not include this key, meaning translations are missing.
## Issue Context
The plugin uses the `plugin__pipelines-console-plugin` i18n namespace and requires regenerating/committing translation artifacts whenever user-facing strings change.
## Fix Focus Areas
- src/components/pipelineRuns-list/PipelineRunsList.tsx[113-115]
- locales/en/plugin__pipelines-console-plugin.json[576-580]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Unprefixed pf-v6-u-ml-sm class 📘 Rule violation ⚙ Maintainability ⭐ New
Description
The updated JSX adds/retains a className value (pf-v6-u-ml-sm) that does not use the required
pipelines-console-plugin- prefix. This can violate the PR styling convention and increase risk of
CSS collisions/non-standard theming usage.
Code

src/components/common/DataViewFilterToolbar.tsx[R149-154]

+                    {!config.singleSelect && (
+                      <Badge isRead className="pf-v6-u-ml-sm">
+                        {option.totalCount !== undefined
+                          ? `${option.count}/${option.totalCount}`
+                          : option.count}
+                      </Badge>
Evidence
PR Compliance ID 1 requires newly added/modified CSS classes to be prefixed with
pipelines-console-plugin-. The code adds/uses className="pf-v6-u-ml-sm" in the modified block,
which does not meet the prefix requirement.

AGENTS.md: CSS Classes Must Use pipelines-console-plugin- Prefix and Avoid Hex Colors
src/components/common/DataViewFilterToolbar.tsx[149-154]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
New/modified JSX uses `className="pf-v6-u-ml-sm"`, which is not prefixed with `pipelines-console-plugin-` as required by the styling compliance rule.

## Issue Context
The compliance checklist requires newly introduced/modified CSS classes to use the `pipelines-console-plugin-` prefix and avoid hardcoded colors.

## Fix Focus Areas
- src/components/common/DataViewFilterToolbar.tsx[149-155]

## Suggested approach
- Option A (no class): remove `className` and use `style={{ marginLeft: 'var(--pf-v6-global--spacer--sm)' }}` (PatternFly token, no new class).
- Option B (prefixed class): change to a prefixed class (e.g., `pipelines-console-plugin-badgeSpacer`) and define it in the appropriate SCSS using PatternFly spacing tokens.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Blank singleSelect toggle label 🐞 Bug ≡ Correctness ⭐ New
Description
CheckboxFilterInput renders the single-select toggle label using an option lookup that can return
undefined when the selected value is not present in config.options, resulting in a blank filter
button. This can happen in PipelineRunsList when a persisted timespan formats to 52w but Tekton
Results is disabled (so TimeRangeOptionsK8s omits 52w).
Code

src/components/common/DataViewFilterToolbar.tsx[R126-128]

+            {config.singleSelect && selected.length > 0
+              ? config.options.find((o) => o.value === selected[0])?.label
+              : config.placeholder || config.title}
Evidence
TimeRangeOptionsK8s omits the '52w' option, while PipelineRunsList derives the selected key from
persisted timespan via formatPrometheusDuration(timespan) and always passes it as the selected
value. The single-select toggle label rendering does not handle the "selected value not in options"
case, so the label can become blank.

src/components/pipelines-overview/utils.ts[46-65]
src/components/pipelineRuns-list/PipelineRunsList.tsx[99-125]
src/components/common/DataViewFilterToolbar.tsx[114-129]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
In `CheckboxFilterInput`, the single-select button label is derived from `config.options.find(... )?.label`. If the selected value is not present in `config.options`, React renders `undefined` (blank label).

This can occur for the new `timeRange` filter in `PipelineRunsList` when the persisted preference corresponds to a key that is not offered in the current mode (e.g., user previously selected `52w` while Tekton Results was enabled, then later Tekton Results is disabled and `TimeRangeOptionsK8s()` no longer includes `52w`).

### Issue Context
- The time range options differ depending on `isTektonResultEnabled`, but the persisted `timespan` can still produce a key not present in the currently chosen options.
- The UI should not render a blank toggle label for a selected value.

### Fix Focus Areas
- src/components/common/DataViewFilterToolbar.tsx[126-128]
- src/components/pipelineRuns-list/PipelineRunsList.tsx[99-125]
- src/components/pipelines-overview/utils.ts[46-65]

### Suggested fix
1) In `CheckboxFilterInput`, compute the display label with a safe fallback, e.g.:
- if option is found: use `option.label`
- else: fall back to `config.placeholder || config.title || selected[0]`

2) (Optional but stronger) In `PipelineRunsList`, when building `currentKey`, if `currentKey` is not in `timeRangeOptions`, reset to a supported default (e.g., `1d`) and/or include the missing key as a disabled option so the UI remains consistent with persisted state.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. Stale date cutoff 🐞 Bug ≡ Correctness
Description
useDateRangeFilter memoizes startDate using Date.now() and only recomputes when timespan
changes, so the "Last day/week/..." window stops moving forward while the page stays open. This
causes both Tekton Results server-side filtering and client-side filtering to drift and include runs
older than the selected range over time.
Code

src/components/hooks/useDateRangeFilter.ts[R28-40]

+  const ts = timespan ?? parsePrometheusDuration('1d');
+
+  const startDate = useMemo(() => {
+    if (!ts) return undefined;
+    return Date.now() - ts;
+  }, [ts]);
+
+  const dateFilterCEL = useMemo(() => {
+    if (!startDate) return '';
+    return `data.status.startTime > timestamp("${new Date(
+      startDate,
+    ).toISOString()}")`;
+  }, [startDate]);
Evidence
startDate is derived from Date.now() but only re-evaluated when ts changes, and the resulting
startDate drives both server-side and client-side filtering in the list page.

src/components/hooks/useDateRangeFilter.ts[28-40]
src/components/pipelineRuns-list/PipelineRunsList.tsx[69-83]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`startDate` is computed with `Date.now()` inside a `useMemo` keyed only by `ts`, so it becomes stale and the time window no longer represents “last X” after the page has been open for a while.
## Issue Context
This `startDate` is used both to generate the Tekton Results CEL filter and to client-side filter the list.
## Fix Focus Areas
- src/components/hooks/useDateRangeFilter.ts[28-40]
## Suggested fix
- Introduce a “now” value that updates periodically (e.g., every 30–60s) and compute `startDate = now - ts`.
- Example approach: `const [now, setNow] = useState(Date.now()); useEffect(() => { const id = setInterval(() => setNow(Date.now()), 60000); return () => clearInterval(id); }, []);`
- Then compute `startDate` from `now` and `ts`.
- Regenerate `dateFilterCEL` from the updated `startDate` so Tekton Results queries also remain aligned with the rolling window.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

5. Zero timespan disables filter 🐞 Bug ☼ Reliability
Description
If the persisted preference contains 0, useDateRangeFilter treats it as falsy and returns
startDate as undefined and an empty CEL filter, silently disabling date filtering. This can also
yield an empty dropdown key (formatPrometheusDuration(0) returns an empty string), leaving the UI
without a valid selected label.
Code

src/components/hooks/useDateRangeFilter.ts[R22-33]

+  const [timespan, setTimespan, preferenceLoaded] = useUserPreference<number>(
+    SETTINGS_KEY,
+    parsePrometheusDuration('1d'),
+    true,
+  );
+
+  const ts = timespan ?? parsePrometheusDuration('1d');
+
+  const startDate = useMemo(() => {
+    if (!ts) return undefined;
+    return Date.now() - ts;
+  }, [ts]);
Evidence
The duration parser returns 0 on invalid formats; useDateRangeFilter treats falsy ts
(including 0) as “no cutoff”, and formatting 0ms yields an empty string key.

src/components/pipelines-overview/dateTime.ts[16-29]
src/components/hooks/useDateRangeFilter.ts[28-33]
src/components/pipelines-overview/dateTime.ts[40-54]
src/components/pipelineRuns-list/PipelineRunsList.tsx[99-107]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A stored `timespan` value of `0` is treated as “no timespan” (`if (!ts) return undefined`), which disables filtering and produces an empty time-range key.
## Issue Context
`parsePrometheusDuration` returns `0` on invalid input, and user preferences can also be corrupted or manually edited.
## Fix Focus Areas
- src/components/hooks/useDateRangeFilter.ts[22-33]
- src/components/pipelines-overview/dateTime.ts[16-29]
- src/components/pipelines-overview/dateTime.ts[40-54]
## Suggested fix
- Treat non-positive values as invalid and fall back to the default:
- `const defaultTs = parsePrometheusDuration('1d');`
- `const ts = timespan && timespan > 0 ? timespan : defaultTs;`
- Optionally, once `preferenceLoaded` is true, write back the default when `timespan <= 0` to self-heal the stored preference (guard to avoid loops).
- Ensure the dropdown key never becomes `''` by guaranteeing `ts > 0` before calling `formatPrometheusDuration(ts)`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


6. Boundary comparator mismatch 🐞 Bug ≡ Correctness
Description
Server-side Tekton Results filtering uses a strict > comparison while client-side filtering uses
>= for the same cutoff, so runs exactly on the boundary can be included from K8s/etcd but excluded
from Tekton Results. This creates inconsistent results when switching data sources.
Code

src/components/pipelineRuns-list/PipelineRunsList.tsx[R76-82]

+  const dateFilteredRuns = useMemo(() => {
+    if (!timespan || !startDate || !pipelineRuns) return pipelineRuns;
+    return pipelineRuns.filter((plr) => {
+      const st = plr.status?.startTime;
+      if (!st) return true;
+      return new Date(st).getTime() >= startDate;
+    });
Evidence
The CEL string is built with > while the list’s client-side filtering compares with >=,
producing a mismatch at exact equality.

src/components/hooks/useDateRangeFilter.ts[35-40]
src/components/pipelineRuns-list/PipelineRunsList.tsx[76-82]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Server-side CEL uses `>` but client-side filtering uses `>=`, causing boundary-time runs to differ by data source.
## Issue Context
The same cutoff timestamp is used for both Tekton Results (CEL) and K8s client-side filtering.
## Fix Focus Areas
- src/components/hooks/useDateRangeFilter.ts[35-40]
- src/components/pipelineRuns-list/PipelineRunsList.tsx[76-82]
## Suggested fix
- Pick one consistent comparator and apply it to both paths:
- Either change CEL to `>=` to match the client, or change client logic to `>`.
- Update/extend tests accordingly (the hook test regex currently expects `>`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Previous review results

Review updated until commit aa972d4

Results up to commit b44e712


🐞 Bugs (3) 📘 Rule violations (1) 📜 Skill insights (0)


Action required
1. Missing i18n key Time range 📘 Rule violation ⚙ Maintainability
Description
The new user-facing string t('Time range') was added without a corresponding entry in the
committed i18n artifacts, so it will not be translated in non-default locales. This violates the
requirement to regenerate/commit translations after adding or modifying strings.
Code

src/components/pipelineRuns-list/PipelineRunsList.tsx[R113-115]

+        id: 'timeRange',
+        title: t('Time range'),
+        singleSelect: true,
Evidence
PipelineRunsList.tsx introduces a new translated label t('Time range'), but the committed
English locale file contains Time Range (different casing) and does not provide an entry for `Time
range`, indicating i18n artifacts were not regenerated/updated for the new string.

AGENTS.md: Follow i18n namespace and regenerate translations after string changes
src/components/pipelineRuns-list/PipelineRunsList.tsx[113-115]
locales/en/plugin__pipelines-console-plugin.json[576-580]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A new translatable UI string `t('Time range')` was introduced, but the locale JSON artifacts do not include this key, meaning translations are missing.

## Issue Context
The plugin uses the `plugin__pipelines-console-plugin` i18n namespace and requires regenerating/committing translation artifacts whenever user-facing strings change.

## Fix Focus Areas
- src/components/pipelineRuns-list/PipelineRunsList.tsx[113-115]
- locales/en/plugin__pipelines-console-plugin.json[576-580]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended
2. Stale date cutoff 🐞 Bug ≡ Correctness
Description
useDateRangeFilter memoizes startDate using Date.now() and only recomputes when timespan
changes, so the "Last day/week/..." window stops moving forward while the page stays open. This
causes both Tekton Results server-side filtering and client-side filtering to drift and include runs
older than the selected range over time.
Code

src/components/hooks/useDateRangeFilter.ts[R28-40]

+  const ts = timespan ?? parsePrometheusDuration('1d');
+
+  const startDate = useMemo(() => {
+    if (!ts) return undefined;
+    return Date.now() - ts;
+  }, [ts]);
+
+  const dateFilterCEL = useMemo(() => {
+    if (!startDate) return '';
+    return `data.status.startTime > timestamp("${new Date(
+      startDate,
+    ).toISOString()}")`;
+  }, [startDate]);
Evidence
startDate is derived from Date.now() but only re-evaluated when ts changes, and the resulting
startDate drives both server-side and client-side filtering in the list page.

src/components/hooks/useDateRangeFilter.ts[28-40]
src/components/pipelineRuns-list/PipelineRunsList.tsx[69-83]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`startDate` is computed with `Date.now()` inside a `useMemo` keyed only by `ts`, so it becomes stale and the time window no longer represents “last X” after the page has been open for a while.

## Issue Context
This `startDate` is used both to generate the Tekton Results CEL filter and to client-side filter the list.

## Fix Focus Areas
- src/components/hooks/useDateRangeFilter.ts[28-40]

## Suggested fix
- Introduce a “now” value that updates periodically (e.g., every 30–60s) and compute `startDate = now - ts`.
 - Example approach: `const [now, setNow] = useState(Date.now()); useEffect(() => { const id = setInterval(() => setNow(Date.now()), 60000); return () => clearInterval(id); }, []);`
 - Then compute `startDate` from `now` and `ts`.
- Regenerate `dateFilterCEL` from the updated `startDate` so Tekton Results queries also remain aligned with the rolling window.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational
3. Zero timespan disables filter 🐞 Bug ☼ Reliability
Description
If the persisted preference contains 0, useDateRangeFilter treats it as falsy and returns
startDate as undefined and an empty CEL filter, silently disabling date filtering. This can also
yield an empty dropdown key (formatPrometheusDuration(0) returns an empty string), leaving the UI
without a valid selected label.
Code

src/components/hooks/useDateRangeFilter.ts[R22-33]

+  const [timespan, setTimespan, preferenceLoaded] = useUserPreference<number>(
+    SETTINGS_KEY,
+    parsePrometheusDuration('1d'),
+    true,
+  );
+
+  const ts = timespan ?? parsePrometheusDuration('1d');
+
+  const startDate = useMemo(() => {
+    if (!ts) return undefined;
+    return Date.now() - ts;
+  }, [ts]);
Evidence
The duration parser returns 0 on invalid formats; useDateRangeFilter treats falsy ts
(including 0) as “no cutoff”, and formatting 0ms yields an empty string key.

src/components/pipelines-overview/dateTime.ts[16-29]
src/components/hooks/useDateRangeFilter.ts[28-33]
src/components/pipelines-overview/dateTime.ts[40-54]
src/components/pipelineRuns-list/PipelineRunsList.tsx[99-107]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A stored `timespan` value of `0` is treated as “no timespan” (`if (!ts) return undefined`), which disables filtering and produces an empty time-range key.

## Issue Context
`parsePrometheusDuration` returns `0` on invalid input, and user preferences can also be corrupted or manually edited.

## Fix Focus Areas
- src/components/hooks/useDateRangeFilter.ts[22-33]
- src/components/pipelines-overview/dateTime.ts[16-29]
- src/components/pipelines-overview/dateTime.ts[40-54]

## Suggested fix
- Treat non-positive values as invalid and fall back to the default:
 - `const defaultTs = parsePrometheusDuration('1d');`
 - `const ts = timespan && timespan > 0 ? timespan : defaultTs;`
- Optionally, once `preferenceLoaded` is true, write back the default when `timespan <= 0` to self-heal the stored preference (guard to avoid loops).
- Ensure the dropdown key never becomes `''` by guaranteeing `ts > 0` before calling `formatPrometheusDuration(ts)`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. Boundary comparator mismatch 🐞 Bug ≡ Correctness
Description
Server-side Tekton Results filtering uses a strict > comparison while client-side filtering uses
>= for the same cutoff, so runs exactly on the boundary can be included from K8s/etcd but excluded
from Tekton Results. This creates inconsistent results when switching data sources.
Code

src/components/pipelineRuns-list/PipelineRunsList.tsx[R76-82]

+  const dateFilteredRuns = useMemo(() => {
+    if (!timespan || !startDate || !pipelineRuns) return pipelineRuns;
+    return pipelineRuns.filter((plr) => {
+      const st = plr.status?.startTime;
+      if (!st) return true;
+      return new Date(st).getTime() >= startDate;
+    });
Evidence
The CEL string is built with > while the list’s client-side filtering compares with >=,
producing a mismatch at exact equality.

src/components/hooks/useDateRangeFilter.ts[35-40]
src/components/pipelineRuns-list/PipelineRunsList.tsx[76-82]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Server-side CEL uses `>` but client-side filtering uses `>=`, causing boundary-time runs to differ by data source.

## Issue Context
The same cutoff timestamp is used for both Tekton Results (CEL) and K8s client-side filtering.

## Fix Focus Areas
- src/components/hooks/useDateRangeFilter.ts[35-40]
- src/components/pipelineRuns-list/PipelineRunsList.tsx[76-82]

## Suggested fix
- Pick one consistent comparator and apply it to both paths:
 - Either change CEL to `>=` to match the client, or change client logic to `>`.
- Update/extend tests accordingly (the hook test regex currently expects `>`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Qodo Logo

@openshift-ci

openshift-ci Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: adityavshinde
Once this PR has been reviewed and has the lgtm label, please assign anwesha-palit-redhat for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Comment thread src/components/pipelineRuns-list/PipelineRunsList.tsx
@qodo-code-review qodo-code-review Bot added enhancement New feature or request Tests labels Jun 16, 2026
@anwesha-palit-redhat anwesha-palit-redhat requested review from arvindk-softwaredev and removed request for vdemeester June 16, 2026 09:20
@adityavshinde adityavshinde force-pushed the feat/pipeline-run-date-filter branch from b44e712 to 24a8a9b Compare June 16, 2026 09:31

@anwesha-palit-redhat anwesha-palit-redhat left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Disable tekton-results from tektonconfig and check the behave and attach the screen recording
  2. The screen recording you currently added is not clear - unable to understand the flow of what you are trying to show
  3. Add some AI generated unit test cases for this change please

@adityavshinde

Copy link
Copy Markdown
Contributor Author
  1. Disable tekton-results from tektonconfig and check the behave and attach the screen recording

    1. The screen recording you currently added is not clear - unable to understand the flow of what you are trying to show

    2. Add some AI generated unit test cases for this change please

1. Screen recording after disabling tekton-results from tektonconfig

RFE_without_tkn-results.mp4

2. Steps followed in the screen recording (description)

  1. Logged in as approver-user-1 and set the date range filter to "Last quarter".

  2. Switched to kubeadmin (cluster admin) to verify the preference was persisted -- admin access is required to list ConfigMaps in the openshift-console-user-settings namespace.

  3. Confirmed that the user-settings-<uid> ConfigMap for approver-user-1 contains the saved preference key plugin__pipelines-console-plugin.dateRangeFilter with the expected value.

3. Unit tests

Already added in useDateRangeFilter.spec.ts and PipelineRunsList.spec.tsx

@anwesha-palit-redhat

Copy link
Copy Markdown
Contributor
  1. Disable tekton-results from tektonconfig and check the behave and attach the screen recording

    1. The screen recording you currently added is not clear - unable to understand the flow of what you are trying to show
    2. Add some AI generated unit test cases for this change please

1. Screen recording after disabling tekton-results from tektonconfig

RFE_without_tkn-results.mp4

2. Steps followed in the screen recording (description)

  1. Logged in as approver-user-1 and set the date range filter to "Last quarter".
  2. Switched to kubeadmin (cluster admin) to verify the preference was persisted -- admin access is required to list ConfigMaps in the openshift-console-user-settings namespace.
  3. Confirmed that the user-settings-<uid> ConfigMap for approver-user-1 contains the saved preference key plugin__pipelines-console-plugin.dateRangeFilter with the expected value.

3. Unit tests

Already added in useDateRangeFilter.spec.ts and PipelineRunsList.spec.tsx

  1. We need to have more data, the purpose of this is to understand if the changes are working for etcd plr
  2. Same we need a lot more data to test this - user config change and time range persistance is a part of this change, we also need to ensure that we are showing the right data
  3. ACK
  4. retitle your PR with the Jira so that the label is added please

const trOptions: typeof optionsMemo = useMemo(() => {
if (optionsMemo?.name) {
const { name, ...rest } = optionsMemo;
const { name, filter, ...rest } = optionsMemo;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this needs to be rebased with master branch

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this rebased yet ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I have rebased this. Now making changes accordingly.

placeholder?: string;
options: FilterOption[];
defaultValues?: string[];
singleSelect?: boolean;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the purpose of single select

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The existing DataViewFilterToolbar only supported checkbox-style filters where users can select multiple values (e.g., selecting both "Succeeded" and "Failed" statuses). But for the date range filter, selecting multiple time ranges simultaneously doesn't make sense.

@adityavshinde adityavshinde changed the title Add date range filter for PipelineRuns list page SRVKP-12328: Add date range filter for PipelineRuns list page Jun 17, 2026
@openshift-ci-robot

openshift-ci-robot commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator

@adityavshinde: This pull request references SRVKP-12328 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set.

Details

In response to this:

Summary

Adds a time range filter to the PipelineRuns list page, allowing users to filter pipeline runs
by time period (Last day, Last week, Last month, Last quarter, Last year).

  • Integrated as a standard filter category in the existing filter toolbar (alongside Status, Data Source)
  • Uses single-select dropdown behavior (picks one time range at a time)
  • Server-side filtering via CEL expression for Tekton Results data
  • Client-side filtering for Kubernetes/etcd data
  • User's selection is persisted across sessions via useUserPreference (stored in ConfigMap per user)
  • Added singleSelect support to CheckboxFilterConfig for reuse by other filters

Changed files

  • useDateRangeFilter.ts (new) - Hook managing time range state, persistence, and CEL generation
  • useTektonResult.ts - Accepts and forwards filter parameter to downstream hooks
  • useTaskRuns.ts - Threads filter to Tekton Results API, combines with name filter via AND()
  • DataViewFilterToolbar.tsx - Added singleSelect mode to CheckboxFilterInput
  • PipelineRunsList.tsx - Integrates time range into the filter system

Screen Recording

RFE-7628.mp4

Test plan

  • Selecting a time range filters PipelineRuns correctly
  • Runs without startTime (pending) are preserved
  • Filter persists across tab switches and page refreshes
  • Works with both Tekton Results enabled and disabled
  • Clear filters resets time range to default (Last day)
  • No flash of default value on navigation
  • Unit tests pass (11 tests)
  • TypeScript compiles clean

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

limit?: number;
name?: string;
skipFetch?: boolean;
filter?: string;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Server-side filtering via CEL expression for Tekton Results data
  • Client-side filtering for Kubernetes/etcd data

can you capture screen recording with network tab to verify if server side filtering is working for tekton-results api

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Server side filtering is working for tekton-results api but for K8s api we do not have server side filtering.

Screencast.From.2026-06-19.13-39-30.mp4

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also add a single line comment for what the filter means here

@adityavshinde adityavshinde force-pushed the feat/pipeline-run-date-filter branch from 24a8a9b to e698165 Compare June 22, 2026 09:28
Signed-off-by: Aditya Shinde <adishind@redhat.com>
@adityavshinde adityavshinde force-pushed the feat/pipeline-run-date-filter branch from e698165 to aa972d4 Compare June 22, 2026 09:31
@adityavshinde adityavshinde deleted the feat/pipeline-run-date-filter branch June 22, 2026 10:28
@adityavshinde adityavshinde restored the feat/pipeline-run-date-filter branch June 22, 2026 10:32
@adityavshinde adityavshinde reopened this Jun 22, 2026
@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit aa972d4

return {
...rest,
filter: EQ('data.metadata.name', name),
filter: AND(EQ('data.metadata.name', name), filter),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if filter is null ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the expected format for the filter

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if filter is null ?

AND() is a utility function in tekton-results.ts that joins CEL expressions with &&. It skips falsy values (null, undefined, empty string). So if filter is null:

  1. AND(EQ('data.metadata.name', name), null) just returns EQ('data.metadata.name', name)
  2. It behaves exactly like the original code before the change: filter: EQ('data.metadata.name', name)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the expected format for the filter

It's a CEL string for the Tekton Results API. The format looks like:
data.status.startTime > timestamp("2026-06-22T10:00:00.000Z")

This is generated by useDateRangeFilter in dateFilterCEL. It's the same format used everywhere else in the codebase for Tekton Results queries (e.g., EQ('data.metadata.name', name) produces data.metadata.name == "xyz").

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add verification

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. This verifies that if filter is null, nothing breaks and giving output as before.
Screencast.From.2026-06-22.23-29-19.mp4
  1. In summary-api same pattern has been used.
image

() => ({
...(selector && { selector }),
...(options?.filter && { filter: options.filter }),
}),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you break this down to if-else so that it is easier to read ?
Also describe how the flow happens from pipelinerun list page to useTaskRuns.ts - try and think about the edge cases, also how it might change in future for taskruns

const isTektonResultEnabled = useFlag(FLAG_PIPELINE_TEKTON_RESULT_INSTALLED);
const [timespan, setTimespan, preferenceLoaded] = useUserPreference<number>(
SETTINGS_KEY,
parsePrometheusDuration('1d'),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are we setting 1d as default ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are we setting 1d as default ?

Yes

const currentKey = formatPrometheusDuration(timespan);
const timeRangeOptions = isTektonResultEnabled
? TimeRangeOptions()
: TimeRangeOptionsK8s();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think we need two separate options here as k8 anyway does not support server side rendering, the one in pipeline overview is added because of prometheus
@arvindk-softwaredev wdyt ?

? TimeRangeOptions()
: TimeRangeOptionsK8s();

const filterValues = useMemo(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how much are we really saving in terms of performance with this useMemo ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how much are we really saving in terms of performance with this useMemo ?

Without useMemo, React creates a new filterValues object on every render, even if nothing changed. DataViewFilterToolbar sees a "new" object each time and re-renders unnecessarily.

With useMemo, React reuses the same object until baseFilterValues or currentKey actually changes. This avoids those extra re-renders. Removing it wouldn't cause any visible performance issue as the object is small.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please explain how would this change if the values in dependency arrays were props

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it will work same because useMemo only compares current values with the previous one. If the parent passes new object every render then it will recompute again and again.

return (
<ListPageBody>
{!hideTextFilter && (
{!hideTextFilter && preferenceLoaded && (

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens when preferenceLoaded is false ? the user sees nothing in this implementation, we should add necessary error banners / loaders

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will try this case practically and update

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bubble the error state and then try please

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we set preferenceLoaded as false then it will load forever. Also error is not defined in the SDK, it only returns a tuple of 3 (user preference value, setter callback and loaded boolean). Added a screenshot of this and screen recording for the behavior.

Screencast.From.2026-06-22.22-27-06.mp4
Screenshot From 2026-06-22 22-28-56


export const useDateRangeFilter = (): DateRangeFilterResult => {
const isTektonResultEnabled = useFlag(FLAG_PIPELINE_TEKTON_RESULT_INSTALLED);
const [timespan, setTimespan, preferenceLoaded] = useUserPreference<number>(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if this api fails ?
can we bubble the error please

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if this api fails ? can we bubble the error please

If this api fails, then it falls back to the default value which is "Last day". This will not throw an error as it is defined in SDK

useUserPreference,
} from '@openshift-console/dynamic-plugin-sdk';
import { FLAG_PIPELINE_TEKTON_RESULT_INSTALLED } from '../../consts';
import { parsePrometheusDuration } from '../pipelines-overview/dateTime';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are we using this parsePrometheusDuration ?
we are going to query tektonresults and this is very unreadable
try { const parts = duration .trim() .split(/\s+/) .map((p) => p.match(/^(\d+)([wdhms])$/)); return _.sumBy(parts, (p) => parseInt(p[1], 10) * units[p[2]]); } catch (ignored) { // Invalid duration format return 0; }

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are we using this parsePrometheusDuration ? we are going to query tektonresults and this is very unreadable try { const parts = duration .trim() .split(/\s+/) .map((p) => p.match(/^(\d+)([wdhms])$/)); return _.sumBy(parts, (p) => parseInt(p[1], 10) * units[p[2]]); } catch (ignored) { // Invalid duration format return 0; }

parsePrometheusDuration simply converts a human-readable duration string like '1d' into milliseconds (86400000). It's not related to Prometheus or Tekton Results, it's just a utility for parsing duration strings. We use it because:

  • The time range dropdown options are stored as duration keys ('1d', '1w', '1m', etc.)
  • We need milliseconds for Date.now() - timespan calculation
  • The Overview page already uses this same function for the same purpose
  • It's already in the codebase, so no new dependency

The name is misleading (it's borrowed from Prometheus-style duration format), but the function itself is just string to milliseconds conversion.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adityavshinde
This is a request
We do not want AI responses to review comment, write based on what you understand and can verify else there will be no learning from this
cc : @arvindk-softwaredev

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants