Skip to content

Conversation

@mshriver
Copy link
Contributor

@mshriver mshriver commented Sep 23, 2025

Things that popped up when working on this branch:
#716
#717

Replace the user hostile API filter field on widget configuration with the appropriate filter components and active filter processing

image image

Summary by Sourcery

Integrate reusable filter components and hooks into the widget configuration flows, replace legacy filter parameters with a unified "additional_filters" API field, and migrate existing widget configs accordingly

New Features:

  • Add useWidgetFilters hook and WidgetFilterComponent to render filter UIs in new and edit widget wizards
  • Introduce WidgetParameterFields component to centralize rendering of widget parameter inputs

Bug Fixes:

  • Add database upgrade (version 9) to migrate legacy filters, remove deprecated chart_type, and remove invalid widget parameters

Enhancements:

  • Refactor filter utility functions to unify API formatting and parsing, and update WIDGET_TYPES constants to use "additional_filters"
  • Update backend controllers to validate and clean widget parameters against schema and enforce additional_filters consistently
  • Replace react-heatmap-grid defaultProps usage with HeatMapWrapper to suppress warnings

Build:

  • Upgrade backend to use psycopg2-binary and add Hatch scripts for testing, coverage, and linting
  • Update frontend ESLint config and bump dependency versions in package.json

Tests:

  • Add tests for widget config controller logging of invalid params, enhanced import controller, and application initialization

Copilot AI review requested due to automatic review settings September 23, 2025 06:57
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR replaces the user-hostile API filter field in widget configuration with dedicated filter components and proper active filter processing. The change improves the user experience by providing appropriate UI components for different widget types instead of requiring users to manually construct filter strings.

  • Replaces manual filter string input with dedicated filter components (RunFilter/ResultFilter)
  • Adds automatic conversion from active filters to API parameters
  • Implements widget type detection to determine appropriate filter components

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes and they look great!

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `frontend/src/components/edit-widget-modal.js:131-149` </location>
<code_context>
   }, [data.widget]);

+  // Fetch runs for ResultFilter if needed
+  useEffect(() => {
+    if (componentLoaded && isResultBasedWidget) {
+      const fetchRuns = async () => {
+        try {
+          const response = await HttpClient.get([Settings.serverUrl, 'run'], {
+            pageSize: 100,
+            estimate: true,
+          });
+          const data = await HttpClient.handleResponse(response);
+          const runIds = data.runs.map((run) => run.id);
+          setRuns(runIds);
+        } catch (error) {
+          console.error('Error fetching runs:', error);
+        }
+      };
+      fetchRuns();
+    }
+  }, [componentLoaded, isResultBasedWidget]);
+
+  // Determine widget characteristics
</code_context>

<issue_to_address>
**suggestion:** Consider paginating or limiting run fetches for large datasets.

Making the page size configurable or adding pagination will help accommodate varying dataset sizes and improve scalability.

```suggestion
  // Configurable page size for run fetching
  const RUNS_PAGE_SIZE = 50; // You can adjust or make this a prop

  // Fetch runs for ResultFilter if needed, with pagination
  useEffect(() => {
    if (componentLoaded && isResultBasedWidget) {
      const fetchRuns = async () => {
        let allRunIds = [];
        let page = 1;
        let hasMore = true;
        try {
          while (hasMore && page <= 20) { // Limit to 20 pages max for safety
            const response = await HttpClient.get([Settings.serverUrl, 'run'], {
              pageSize: RUNS_PAGE_SIZE,
              page: page,
              estimate: true,
            });
            const data = await HttpClient.handleResponse(response);
            const runIds = data.runs.map((run) => run.id);
            allRunIds = allRunIds.concat(runIds);
            // If less than requested page size, we've reached the end
            hasMore = runIds.length === RUNS_PAGE_SIZE;
            page += 1;
          }
          setRuns(allRunIds);
        } catch (error) {
          console.error('Error fetching runs:', error);
        }
      };
      fetchRuns();
    }
  }, [componentLoaded, isResultBasedWidget]);
```
</issue_to_address>

### Comment 2
<location> `frontend/src/components/edit-widget-modal.js:151-157` </location>
<code_context>
+
+  // Determine widget characteristics
+  const hasFilterParam = useMemo(() => {
+    return widgetType?.params?.some(
+      (param) =>
+        param.name === 'filters' || param.name === 'additional_filters',
</code_context>

<issue_to_address>
**suggestion:** Widget filter param detection may miss edge cases.

This approach requires manual updates for new filter param names. Refactor to support extensibility for future filter parameters.

```suggestion
  // List of filter-related param names for extensibility
  const FILTER_PARAM_NAMES = ['filters', 'additional_filters'];

  // Determine widget characteristics
  const hasFilterParam = useMemo(() => {
    return widgetType?.params?.some(
      (param) => FILTER_PARAM_NAMES.includes(param.name)
    );
  }, [widgetType]);
```
</issue_to_address>

### Comment 3
<location> `frontend/src/components/edit-widget-modal.js:37` </location>
<code_context>
+import RunFilter from './filtering/run-filter';
+import ResultFilter from './filtering/result-filter';

 const EditWidgetModal = ({ onSave, onClose, isOpen, data }) => {
   const [widgetType, setWidgetType] = useState({});
</code_context>

<issue_to_address>
**issue (complexity):** Consider extracting run-fetching and filter logic into dedicated hooks and components to simplify EditWidgetModal.

```markdown
I agree this component has grown to handle too many responsibilities (run‐fetching, filter UI, filter context capture, params formatting, etc.). Pulling those into focussed hooks/components will flatten the modal and make it more maintainable. For example:

1. Extract run fetching into a `useRuns` hook:

   ```js
   // hooks/useRuns.js
   import { useState, useEffect } from 'react';
   import { HttpClient } from '../services/http';
   import { Settings } from '../settings';

   export function useRuns(componentLoaded, isResultBased) {
     const [runs, setRuns] = useState([]);
     useEffect(() => {
       if (!componentLoaded || !isResultBased) return;
       let cancelled = false;
       (async () => {
         try {
           const resp = await HttpClient.get([Settings.serverUrl, 'run'], {
             pageSize: 100,
             estimate: true,
           });
           const { runs: dataRuns } = await HttpClient.handleResponse(resp);
           if (!cancelled) setRuns(dataRuns.map(r => r.id));
         } catch (e) {
           console.error('Error fetching runs', e);
         }
       })();
       return () => { cancelled = true; };
     }, [componentLoaded, isResultBased]);
     return runs;
   }
   ```

2. Extract all filter‐related logic/UI into a `FilterSection`:

   ```jsx
   // components/FilterSection.js
   import React, { useMemo } from 'react';
   import { FormGroup, FormHelperText, HelperText, HelperTextItem } from '@patternfly/react-core';
   import FilterProvider from '../contexts/filterContext';
   import RunFilter from '../filtering/run-filter';
   import ResultFilter from '../filtering/result-filter';
   import { RESULT_FIELDS, RUN_FIELDS } from '../constants';

   export default function FilterSection({
     widgetType, widgetKey, componentLoaded, runs, onContextCapture
   }) {
     const hasFilterParam = useMemo(
       () => widgetType?.params?.some(p => p.name.includes('filters')),
       [widgetType]
     );
     const isResultBased = useMemo(
       () => ['result-summary','result-aggregator','importance-component']
         .includes(widgetKey),
       [widgetKey]
     );
     if (!hasFilterParam || !componentLoaded) return null;

     const FieldOptions = isResultBased ? RESULT_FIELDS : RUN_FIELDS;
     const FilterComp = isResultBased ? ResultFilter : RunFilter;

     return (
       <FormGroup label="Filters" fieldId="widget-filters">
         <FilterProvider
           fieldOptions={FieldOptions}
           hideFilters={['project_id']}
           onContextChange={onContextCapture}
         >
           <FilterComp runs={isResultBased ? runs : undefined} />
         </FilterProvider>
         <FormHelperText>
           <HelperText>
             <HelperTextItem variant="default">
               Configure filters to limit the data shown in this widget
             </HelperTextItem>
           </HelperText>
         </FormHelperText>
       </FormGroup>
     );
   }
   ```

3. Then in `EditWidgetModal`:

   ```jsx
   import { useRuns } from '../hooks/useRuns';
   import FilterSection from './components/FilterSection';

   const runs = useRuns(componentLoaded, isResultBasedWidget);

   // ...
   <Form>
     {widgetParams}
     <FilterSection
       widgetType={widgetType}
       widgetKey={data.widget}
       componentLoaded={componentLoaded}
       runs={runs}
       onContextCapture={setFilterContextRef}
     />
   </Form>
   ```

This removes ~100 lines of nested hooks/logic from the modal, keeps all behavior intact, and leaves `EditWidgetModal` focused on orchestration.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@mshriver mshriver added enhancement New feature or request frontend labels Sep 29, 2025
@mshriver mshriver force-pushed the widget-filter-components branch from 8455b6d to ce04b1e Compare September 29, 2025 12:45
@mshriver mshriver changed the title DRAFT: Sonnet iteration on using filters in widgets Use Filter Components and hooks in widget configuration Sep 30, 2025
@mshriver
Copy link
Contributor Author

@sourcery-ai review

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes - here's some feedback:

  • The run-fetching useEffect in both NewWidgetWizard and EditWidgetModal is nearly identical—consider moving it into useWidgetFilters to DRY up the data loading for result-based widgets.
  • Double-check the version bump in db/upgrades.py (and the presence of both upgrade_6 and upgrade_7) to ensure migrations run in the correct order without skipping any versions.
  • It would be helpful to add focused unit tests for useWidgetFilters (especially filter serialization and reset logic) to guard against regressions in filter context behavior.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The run-fetching useEffect in both NewWidgetWizard and EditWidgetModal is nearly identical—consider moving it into useWidgetFilters to DRY up the data loading for result-based widgets.
- Double-check the __version__ bump in db/upgrades.py (and the presence of both upgrade_6 and upgrade_7) to ensure migrations run in the correct order without skipping any versions.
- It would be helpful to add focused unit tests for useWidgetFilters (especially filter serialization and reset logic) to guard against regressions in filter context behavior.

## Individual Comments

### Comment 1
<location> `frontend/src/utilities/widget.js:86` </location>
<code_context>
+/**
+ * Get the appropriate field options for a widget type
+ */
+export const getFieldOptionsForWidget = (widgetId) => {
+  // This would need to import RESULT_FIELDS, RUN_FIELDS from constants
+  // but avoiding circular imports for now
</code_context>

<issue_to_address>
**issue (bug_risk):** getFieldOptionsForWidget returns a string instead of the actual field options array.

Currently, the function returns a string instead of the expected array, which could lead to runtime errors for consumers. Please import and return the actual arrays.
</issue_to_address>

### Comment 2
<location> `frontend/src/utilities/filters.js:48` </location>
<code_context>
-  // Take UI style filter object with field/op/val keys and generate an array of filter strings for the API
-  // TODO rework for array of filters instead of keyed object
-  let filter_strings = [];
-  for (const key in filters) {
-    if (
-      Object.prototype.hasOwnProperty.call(filters, key) &&
</code_context>

<issue_to_address>
**suggestion:** toAPIFilter expects filters as a keyed object, but other functions use arrays.

Standardizing the filter data structure across utility functions will help prevent confusion and potential bugs.
</issue_to_address>

### Comment 3
<location> `frontend/src/components/WidgetParameterFields.js:56` </location>
<code_context>
+                id={param.name}
+                aria-describedby={`${param.name}-helper`}
+                name={param.name}
+                onChange={(event, value) => onChange(event, value)}
+                isRequired={param.required}
+                validated={
</code_context>

<issue_to_address>
**suggestion (bug_risk):** WidgetParameterFields passes both event and value to onChange, but some usages expect only value.

This mismatch may cause issues in parent components. Please standardize the onChange signature or clarify the expected arguments.

Suggested implementation:

```javascript
                onChange={(value) => onChange(value)}

```

If the parent component or any usages of `onChange` expect both `(event, value)`, you will need to update those handlers to accept only `value`. 
Also, if you are using other input components (e.g., Select, Checkbox) in this file or elsewhere that use a similar pattern, you should standardize their `onChange` handlers as well.
</issue_to_address>

### Comment 4
<location> `AGENTS.md:13` </location>
<code_context>
+# frontend
+- use yarn and yarn lint from the frontend directory as the working directory
+- prefer the use of patternfly components
+- use functional components and prefer patterns that levaage all available react hook patterns
+- use strict react-hooks rules for exhaustive deps
+- React imports are necessary for JSX
</code_context>

<issue_to_address>
**issue (typo):** Typo: 'levaage' should be 'leverage'.

```suggestion
- use functional components and prefer patterns that leverage all available react hook patterns
```
</issue_to_address>

### Comment 5
<location> `backend/ibutsu_server/controllers/widget_controller.py:122` </location>
<code_context>
     params = _typecast_params(id_, params)
-    return WIDGET_METHODS[id_](**params)
+
+    # Filter out parameters that are not accepted by the widget function
+    widget_function = WIDGET_METHODS[id_]
+    import inspect
</code_context>

<issue_to_address>
**issue (complexity):** Consider replacing runtime introspection with a centralized parameter validation function for widget parameters.

```suggestion
# In your get_widget handler, drop the inspect.signature logic and delegate to _validate_widget_params
     params = _pre_process_params(params, id_)
     params = _typecast_params(id_, params)

-    # Filter out parameters that are not accepted by the widget function
-    widget_function = WIDGET_METHODS[id_]
-    import inspect
-    valid_params = inspect.signature(widget_function).parameters.keys()
-    filtered_params = {k: v for k, v in params.items() if k in valid_params}
-
-    # Check if any required parameters are missing or invalid parameters were provided
-    invalid_params = set(params.keys()) - set(valid_params)
-    if invalid_params:
-        return (
-            f"Invalid parameters for widget '{id_}': {', '.join(invalid_params)}",
-            HTTPStatus.BAD_REQUEST,
-        )
-
-    try:
-        return widget_function(**filtered_params)
-    except TypeError as e:
-        return f"Parameter error for widget '{id_}': {e}", HTTPStatus.BAD_REQUEST

+    # Validate against the central schema in WIDGET_TYPES[_]["params"]
+    errors = _validate_widget_params(id_, params)
+    if errors:
+        return f"Invalid parameters for widget '{id_}': {', '.join(errors)}", HTTPStatus.BAD_REQUEST
+
+    # All params are known, so just call the widget
+    return WIDGET_METHODS[id_](**params)
```

This removes the runtime introspection and leverages your existing `_validate_widget_params` (which already knows required vs. optional vs. types), keeping behavior and error messages intact.
</issue_to_address>

### Comment 6
<location> `frontend/src/utilities/filters.js:16` </location>
<code_context>
-  });
-};
-
-export const filtersToAPIParams = (filters = []) => {
-  if (filters?.length) {
-    return filters.map((f) => {
</code_context>

<issue_to_address>
**issue (complexity):** Consider refactoring filter string construction and parsing by extracting a reusable function and using a single regex for operator matching.

```suggestion
// 1) DRY up filters → “fieldOpCharValue” logic
// --------------------------------------------------------------------
const formatFilter = ({ field, operator, value }) =>
  `${field}${OPERATIONS[operator].opChar}${value}`;

export const filtersToAPIParams = (filters = []) =>
  filters.length
    ? filters.map(formatFilter)
    : [];

// old toAPIFilter can be rewritten as:
export const toAPIFilter = (filterObj) =>
  Object.entries(filterObj)
    .filter(([key, cfg]) => key !== 'id' && cfg)
    .map(([key, { operator, val }]) =>
      formatFilter({ field: key, operator, value: val })
    );

// 2) Collapse multi/single‐char op loops into one regex pass
// --------------------------------------------------------------------
const opCharToOperator = Object.entries(OPERATIONS)
  .reduce((acc, [op, { opChar }]) => ({ ...acc, [opChar]: op }), {});

// build a regex that matches the longest ops first (>=, <=, !=, then =, >, <, etc)
const opCharsPattern = Object.keys(opCharToOperator)
  .map((c) => c.replace(/([.?*+^$[\]\\(){}|-])/g, '\\$1'))
  .sort((a, b) => b.length - a.length)
  .join('|');
const FILTER_RE = new RegExp(`^(.+)(${opCharsPattern})(.+)$`);

export const apiParamsToFilters = (filterString = '') => {
  if (typeof filterString !== 'string' || !filterString.trim()) return [];

  return filterString
    .split(',')
    .map((s) => s.trim())
    .filter(Boolean)
    .map((str) => {
      const m = str.match(FILTER_RE);
      if (m) {
        return {
          field: m[1],
          operator: opCharToOperator[m[2]],
          value: m[3],
        };
      }
      // fallback to eq if no match
      const [field, value] = str.split('=');
      return { field, operator: 'eq', value };
    });
};
```

• Extracted `formatFilter()` to unify stringification in both `filtersToAPIParams` and `toAPIFilter`.  
• Precomputed `opCharToOperator` and built a single regex `FILTER_RE` that matches multi‐char ops first, removing the need for separate loops.  
• This preserves all existing behavior but cuts ~30–40 lines of specialized looping and branching.
</issue_to_address>

### Comment 7
<location> `frontend/src/utilities/filters.js:141` </location>
<code_context>
        const opChar = config.opChar;

</code_context>

<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/use-object-destructuring))

```suggestion
        const {opChar} = config;
```

<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.

From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>

### Comment 8
<location> `frontend/src/utilities/string.js:6` </location>
<code_context>

</code_context>

<issue_to_address>
**issue (code-quality):** Don't reassign parameter - str ([`dont-reassign-parameters`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/dont-reassign-parameters))

<details><summary>Explanation</summary>Reassigning parameters can lead to unexpected behavior, especially when accessing the arguments object.
It can also cause optimization issues, especially in V8.

From the [Airbnb JavaScript Style Guide](https://airbnb.io/javascript/#functions--reassign-params)
</details>
</issue_to_address>

### Comment 9
<location> `frontend/src/utilities/string.js:15` </location>
<code_context>

</code_context>

<issue_to_address>
**issue (code-quality):** Don't reassign parameter - path ([`dont-reassign-parameters`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/dont-reassign-parameters))

<details><summary>Explanation</summary>Reassigning parameters can lead to unexpected behavior, especially when accessing the arguments object.
It can also cause optimization issues, especially in V8.

From the [Airbnb JavaScript Style Guide](https://airbnb.io/javascript/#functions--reassign-params)
</details>
</issue_to_address>

### Comment 10
<location> `frontend/src/utilities/string.js:33` </location>
<code_context>
    s = s - days * (24 * 60 * 60);

</code_context>

<issue_to_address>
**suggestion (code-quality):** Replace assignment with assignment operator ([`assignment-operator`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/assignment-operator))

```suggestion
    s -= days * (24 * 60 * 60);
```

<br/><details><summary>Explanation</summary>The augmented assignment operators, such as `+=` are a more streamlined and easy to understand way
to add values. Using them removes a little bit of repetition and makes the code slightly easier to
understand.
</details>
</issue_to_address>

### Comment 11
<location> `frontend/src/utilities/string.js:33` </location>
<code_context>

</code_context>

<issue_to_address>
**issue (code-quality):** Don't reassign parameter - s ([`dont-reassign-parameters`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/dont-reassign-parameters))

<details><summary>Explanation</summary>Reassigning parameters can lead to unexpected behavior, especially when accessing the arguments object.
It can also cause optimization issues, especially in V8.

From the [Airbnb JavaScript Style Guide](https://airbnb.io/javascript/#functions--reassign-params)
</details>
</issue_to_address>

### Comment 12
<location> `backend/ibutsu_server/controllers/widget_config_controller.py:25-28` </location>
<code_context>
def _validate_widget_params(widget_type, params):
    """Validate and clean widget parameters against widget type specification"""
    if not params:
        return {}

    # Get valid parameter names for this widget type
    valid_param_names = {p["name"] for p in WIDGET_TYPES.get(widget_type, {}).get("params", [])}

    # Filter out invalid parameters
    cleaned_params = {k: v for k, v in params.items() if k in valid_param_names}

    return cleaned_params

</code_context>

<issue_to_address>
**suggestion (code-quality):** Inline variable that is immediately returned ([`inline-immediately-returned-variable`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/inline-immediately-returned-variable/))

```suggestion
    return {k: v for k, v in params.items() if k in valid_param_names}
```
</issue_to_address>

### Comment 13
<location> `backend/ibutsu_server/widgets/compare_runs_view.py:63-64` </location>
<code_context>
def get_comparison_data(additional_filters=None):
    data = _get_comparison_data(additional_filters=additional_filters)
    return data

</code_context>

<issue_to_address>
**suggestion (code-quality):** Inline variable that is immediately returned ([`inline-immediately-returned-variable`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/inline-immediately-returned-variable/))

```suggestion
    return _get_comparison_data(additional_filters=additional_filters)
```
</issue_to_address>

### Comment 14
<location> `backend/ibutsu_server/widgets/jenkins_job_view.py:116-117` </location>
<code_context>
def get_jenkins_job_view(
    additional_filters=None, project=None, page=1, page_size=25, run_limit=None
):
    filters = []

    if additional_filters:
        if isinstance(additional_filters, str):
            # Handle string format (comma-separated)
            for filter_string in additional_filters.split(","):
                filters.append(filter_string)
        elif isinstance(additional_filters, list):
            # Handle list format
            filters = additional_filters

    jenkins_jobs = _get_jenkins_aggregation(filters, project, page, page_size, run_limit)
    total_items = jenkins_jobs["pagination"]["totalItems"]
    total_pages = (total_items // page_size) + (1 if total_items % page_size > 0 else 0)
    jenkins_jobs["pagination"].update({"totalPages": total_pages})

    return jenkins_jobs

</code_context>

<issue_to_address>
**suggestion (code-quality):** We've found these issues:

- Replace a for append loop with list extend ([`for-append-to-extend`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/for-append-to-extend/))
- Simplify generator expression ([`simplify-generator`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/simplify-generator/))

```suggestion
            filters.extend(iter(additional_filters.split(",")))
```
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@mshriver mshriver force-pushed the widget-filter-components branch from df683d8 to e84271a Compare September 30, 2025 13:15
@mshriver mshriver force-pushed the widget-filter-components branch from e84271a to 6a52c21 Compare October 1, 2025 09:19
@mshriver mshriver added bug Something isn't working backend labels Oct 1, 2025
@ibutsu ibutsu deleted a comment from sourcery-ai bot Oct 1, 2025
@mshriver mshriver requested a review from Copilot October 1, 2025 09:35
@mshriver
Copy link
Contributor Author

mshriver commented Oct 1, 2025

@sourcery-ai summarize, add reviewer's guide, and review

@sourcery-ai
Copy link
Contributor

sourcery-ai bot commented Oct 1, 2025

Sure! I'm generating a new review now.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 102 out of 117 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (2)

scripts/ibutsu-pod.sh:1

  • Missing closing brace - there's an extra closing brace that doesn't match any opening brace. Should be \"additional_filters\": \"component=${RUN_COMPONENT}\"}}
#!/bin/bash

frontend/src/widgets/RunAggregateApex.js:1

  • String comparison with 'undefined' will not catch actual undefined values. Should use runId.current !== undefined to properly check for undefined values.
import React, { useEffect, useMemo, useRef, useState } from 'react';

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

We encountered an error and are unable to review this PR. We have been notified and are working to fix it.

You can try again by commenting this pull request with @sourcery-ai review, or contact us for help.

@sourcery-ai
Copy link
Contributor

sourcery-ai bot commented Oct 1, 2025

Hey @mshriver, I've posted a new review for you!

@mshriver mshriver force-pushed the widget-filter-components branch 3 times, most recently from 1725bfa to 46d1497 Compare October 1, 2025 16:08
@mshriver mshriver marked this pull request as draft October 2, 2025 07:08
@mshriver mshriver force-pushed the widget-filter-components branch 3 times, most recently from 15d0d5a to 5bab1f8 Compare October 3, 2025 14:33
@mshriver mshriver requested a review from Copilot October 3, 2025 14:34
@mshriver
Copy link
Contributor Author

mshriver commented Oct 3, 2025

@sourcery-ai review

@sourcery-ai
Copy link
Contributor

sourcery-ai bot commented Oct 3, 2025

Sorry @mshriver, your pull request is larger than the review limit of 150000 diff characters

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 88 out of 94 changed files in this pull request and generated 16 comments.

Comments suppressed due to low confidence (1)

scripts/ibutsu-pod.sh:1

  • Extra closing brace before the double quotes - should be ...stage\"}}
#!/bin/bash

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@mshriver mshriver requested a review from Copilot October 6, 2025 15:50
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 81 out of 83 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

scripts/ibutsu-pod.sh:1

  • Extra closing brace - should be single } instead of }}
#!/bin/bash

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@mshriver mshriver force-pushed the widget-filter-components branch from 3d1ce29 to dea2cca Compare October 7, 2025 07:13
@mshriver
Copy link
Contributor Author

mshriver commented Oct 7, 2025

@sourcery-ai summary

Edit and new widget wizard should now include RunFilter/ResultFilter and
ActiveFilter component use instead of plain text input.

The user no longer has to specify an api compatible string for
filtering.

Conversion to the api compatible string is done when making the backend
request automatically, and then destructured from the string stored in
the DB for with widgetconfig for rendering on edit.

Updates for consistent widget-config filters

additional_filters kwarg uses consistently across the backend for
widget-configs.

jenkins job views and widgets
update dev deploy script to set the param

Handle ref/forwardRef in runaggregate

handle uuid processing without 500s
extend hatch config and add a readme for hatch usage for those new to
it.

grid wrapper due to console warnings
avoid dumping warnings into the console due to the react-heatmap-grid
library

Fix logic for wizard widget steps
enable next button when required fields are filled

Add logging for invalid param configs

Expand upgrade_9 to log and cover more fields

the production data for widget-configs was used to devise the upgrade
function coverage

Drop chart_type from aggregation widgets
The map was creating multiple Tbody, and unique keys weren't being
applied to the Td elements.
@mshriver mshriver force-pushed the widget-filter-components branch from dea2cca to a028a19 Compare October 8, 2025 09:11
@mshriver mshriver marked this pull request as ready for review October 8, 2025 09:25
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Sorry @mshriver, your pull request is larger than the review limit of 150000 diff characters

@mshriver mshriver requested a review from Copilot October 8, 2025 09:25
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 87 out of 90 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

frontend/src/widgets/result-aggregate-apex.js:1

  • The chart_type parameter was removed from the params object but is still being used in this line. This should be removed to match the API changes.
import { useEffect, useMemo, useRef, useState } from 'react';

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@mshriver mshriver force-pushed the widget-filter-components branch from 0c782d5 to d0112a7 Compare October 8, 2025 10:02
@mshriver mshriver merged commit b64e82c into ibutsu:main Oct 8, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend bug Something isn't working enhancement New feature or request frontend

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant