Skip to content

Conversation

@Maria-Lordwill
Copy link
Collaborator

@Maria-Lordwill Maria-Lordwill commented Dec 10, 2025

Add a transition to the corresponding filter by switching workflows filtering from step IDs to task API names and expose new /templates/titles-by-workflows and /templates/titles-by-tasks endpoints across backend and frontend

Replace step-based filters with task API name filters in workflows and tasks, rename template titles endpoints and serializers, add performer and starter filters, and update Redux to RTK slices with new selectors and actions; key review points include query changes in TemplateTitlesByWorkflowsQuery, the new TemplateTitlesByTasksQuery, and UI wiring for TaskColumn and filter selects.

📍Where to Start

Start with TemplateViewSet actions in backend/src/processes/views/template.py, then review TemplateTitlesByWorkflowsQuery and TemplateTitlesByTasksQuery in backend/src/processes/queries.py, and the workflows slice in frontend/src/public/redux/workflows/slice.ts.


📊 Macroscope summarized 7e9cd01. 53 files reviewed, 38 issues evaluated, 37 issues filtered, 1 comment posted

🗂️ Filtered Issues

backend/src/processes/enums.py — 0 comments posted, 1 evaluated, 1 filtered
  • line 414: The TaskStatus.LITERALS type uses class attributes (PENDING, ACTIVE, etc.) inside Literal[]. While Python evaluates these to their string values at class definition time making this work, this pattern is fragile - if the class attributes are modified or if the evaluation order changes, the Literal type could contain unexpected values. Using explicit string literals like Literal['pending', 'active', 'completed', 'snoozed', 'skipped'] would be more robust. [ Low confidence ]
backend/src/processes/queries.py — 0 comments posted, 3 evaluated, 3 filtered
  • line 883: The removal of tt.id AS template_task_id from the SELECT clause is a breaking change. Any code that consumes the results of this query and accesses the template_task_id column will fail at runtime with a KeyError or similar error when the column is no longer present in the result set. [ Low confidence ]
  • line 1031: Removing pt.id as template_task_id from the SELECT clause breaks any code that accesses template_task_id from the query results. Since get_sql() uses SELECT * from this inner query, any consumer expecting template_task_id in the result set will fail with a KeyError or AttributeError at runtime. [ Low confidence ]
  • line 1839: The _get_where method only handles TaskStatus.ACTIVE, TaskStatus.DELAYED, and an implicit completed case in the else branch. If status is TaskStatus.PENDING or TaskStatus.SKIPPED (both valid values per TaskStatus.LITERALS), the else branch executes with is_completed IS TRUE, which is semantically incorrect for those statuses and will return wrong query results. [ Already posted ]
backend/src/processes/views/template.py — 0 comments posted, 3 evaluated, 3 filtered
  • line 118: The action_serializer_classes was changed to rename 'titles' to 'titles_by_workflows', but the get_permissions method at line 154 still references 'titles' instead of 'titles_by_workflows'. This means the titles_by_workflows action will not match the permission check and will fall through to the default permissions at lines 180-186, which incorrectly requires UserIsAdminOrAccountOwner(). Users who should have access (any authenticated user with a valid subscription) will be denied if they're not admins or account owners. [ Already posted ]
  • line 154: The get_permissions method references the action name 'titles' on line 154, but the action was renamed to 'titles_by_workflows' (as shown in code_object_to_review id 1). This means titles_by_workflows action will not match the condition on line 152-158 and will fall through to the default permissions on lines 180-186, which incorrectly requires UserIsAdminOrAccountOwner() and UsersOverlimitedPermission() permissions. This is more restrictive than intended and will deny access to non-admin/non-owner users who should have access. [ Already posted ]
  • line 154: The action titles_by_workflows is missing from the permissions check at line 154. The action was renamed from titles to titles_by_workflows (as shown in the diff), but the string 'titles' in get_permissions() was not updated to 'titles_by_workflows'. This causes the titles_by_workflows action to fall through to the default permissions at line 180-186, which requires UserIsAdminOrAccountOwner() - a stricter permission than intended. Regular authenticated users will be denied access to this endpoint. [ Already posted ]
frontend/src/public/api/getTemplatesTitlesByTasks.ts — 0 comments posted, 1 evaluated, 1 filtered
  • line 12: In getTemplatesTitlesByTasks, urls.templatesTitlesByTasks is accessed on line 12 but there is no guarantee this property exists in the urls configuration object. If templatesTitlesByTasks is not defined in the API config, baseUrl will be undefined, and the constructed URL on line 14 will become "undefined?status=active" or "undefined?status=completed", causing the API request to fail or hit an incorrect endpoint. [ Low confidence ]
frontend/src/public/api/getUserTasks.ts — 0 comments posted, 1 evaluated, 1 filtered
  • line 87: In getUserTasksQueryString, when IS_STATUS_COMPLETED is false, the code accesses QS_BY_SORTING[sorting] on line 87. However, the sorting parameter type is ETaskListSorting | ETaskListCompleteSorting. While ETaskListCompleteSorting values overlap with ETaskListSorting, the type system doesn't guarantee this at runtime if the enum values diverge in the future, potentially returning undefined for the sorting query parameter. [ Low confidence ]
frontend/src/public/components/Dashboard/Breakdowns/TaskItem.tsx — 0 comments posted, 1 evaluated, 1 filtered
  • line 74: The useCallback for getRoute at line 26 has an incomplete dependency array [mode]. The callback uses task.apiName, task.id, and templateId, but none of these are in the dependency array. This will cause stale closures where the routes generated will use outdated values of task.apiName, task.id, or templateId when those props change, leading to incorrect navigation links. [ Out of scope ]
frontend/src/public/components/TuneViewModal/TuneViewModal.tsx — 0 comments posted, 1 evaluated, 1 filtered
  • line 53: The useEffect dependency array on line 53 is missing savedFields. The effect uses savedFields to initialize selectedFields state, but if savedFields changes in Redux while isOpen, templateId, and templateTasks remain the same, the effect won't re-run and the component will display stale data. [ Out of scope ]
frontend/src/public/components/UI/Select/FilterSelect.tsx — 0 comments posted, 2 evaluated, 2 filtered
  • line 284: The condition selectedOption !== null on line 284 incorrectly renders the reset option when isMultiple is true. When isMultiple is true, the interface defines selectedOption?: never, meaning selectedOption is undefined. Since undefined !== null evaluates to true, renderResetOption() will always be called in multi-select mode, which is unintended. The original code used props.selectedOption && renderResetOption() which correctly returned falsy for undefined. The fix should check !isMultiple && selectedOption !== null. [ Already posted ]
  • line 352: The OutsideClickHandler wrapper (lines 351-421) combined with reactstrap's Dropdown component may cause conflicting outside-click handling behavior. Both components may attempt to handle outside clicks, potentially causing double-toggling or unexpected dropdown state. The Dropdown component's internal toggle mechanism via the toggle prop combined with OutsideClickHandler's onOutsideClick calling the same handleToggleDropdown could lead to race conditions. [ Low confidence ]
frontend/src/public/components/UI/Select/SelectMenu.tsx — 0 comments posted, 1 evaluated, 1 filtered
  • line 97: The CSS class radioStyles['select-menu__radio--checked'] on line 97 appears to reference a class that likely doesn't exist in RadioButton.css. The naming convention select-menu__radio--checked is inconsistent with the other radio style classes used (radio, radio__box, radio__title) which follow standard BEM naming. This would result in the checked state styling not being applied when withRadio is true and an item is selected. [ Low confidence ]
frontend/src/public/components/Workflows/WorkflowsGridPage/WorkflowsGridPage.tsx — 0 comments posted, 4 evaluated, 4 filtered
  • line 81: The removal of loadTemplatesTitles() call from the component means templatesFilter may never be populated if it was previously loaded by this component. If templatesFilter relies on this call being made, handleRunNewWorkflow will always fall through to openSelectTemplateModal() on line 82 when templatesFilter is empty. [ Low confidence ]
  • line 147: In the skeleton loader loop, item is used as the key prop, but since Array(N) creates empty slots, item will be undefined for all elements (if the array iteration worked). This would cause React key warnings. The key should use the index: .map((_, index) => <WorkflowCardLoader key={index} />). [ Out of scope ]
  • line 147: Array(INIT_SKELETION_QUANTITY).map((item) => ...) creates an array with empty slots, but .map() skips empty slots entirely. This results in an empty array being rendered instead of 16 skeleton loaders. Should use Array.from({ length: INIT_SKELETION_QUANTITY }).map(...) or Array(INIT_SKELETION_QUANTITY).fill(null).map(...). [ Out of scope ]
  • line 155: Array(SCROLL_LOADERS_QUANTITY).map((item) => ...) has the same empty slots bug - .map() will not iterate over empty array slots, resulting in an empty loader array passed to InfiniteScroll. Should use Array.from({ length: SCROLL_LOADERS_QUANTITY }).map(...). [ Out of scope ]
frontend/src/public/components/Workflows/WorkflowsTablePage/WorkflowsTable/Columns/Cells/TaskColumn/TaskColumn.tsx — 0 comments posted, 2 evaluated, 2 filtered
  • line 23: Object.keys(multipleTasksNamesByApiNames)[0] at line 23 returns undefined when multipleTasksNamesByApiNames is an empty object. This undefined value is then dispatched to setFilterTemplateTasks at line 26 when the button is clicked, which could cause unexpected filter behavior or errors in the reducer. [ Already posted ]
  • line 27: When template is null or undefined, template?.id ?? 0 at line 27 dispatches 0 as the template filter. Filtering by template ID 0 may not match any actual template and could produce unexpected empty results or behave inconsistently with user expectations. [ Low confidence ]
frontend/src/public/components/Workflows/WorkflowsTablePage/WorkflowsTable/Columns/Cells/TemplateNameColumn/TemplateNameColumn.tsx — 0 comments posted, 1 evaluated, 1 filtered
  • line 17: When value.template is null or undefined, clicking the button will dispatch setWorkflowsFilterTemplate([undefined]). This passes an array containing undefined to the filter action, which likely expects valid template IDs. This could cause incorrect filtering behavior or errors in the reducer/selector that processes the filter. The button should either be disabled when there's no template, or the click handler should guard against dispatching with an undefined ID. [ Already posted ]
frontend/src/public/components/Workflows/WorkflowsTablePage/WorkflowsTable/WorkflowsTable.tsx — 0 comments posted, 1 evaluated, 1 filtered
  • line 126: The hook useCheckDevice() is called twice in WorkflowsTable: once at line 98 to get isDesktop and again at line 126 to get isMobile. While not a crash bug, this is wasteful and could cause subtle issues if the hook has side effects or triggers re-renders. These should be combined into a single destructuring call like const { isMobile, isDesktop } = useCheckDevice();. [ Already posted ]
frontend/src/public/components/icons/PerformerFilterIcon.tsx — 0 comments posted, 1 evaluated, 1 filtered
  • line 4: The component declares React.SVGAttributes<SVGElement> as its props type, which allows callers to pass any SVG attribute (like onClick, aria-label, style, etc.), but only className is destructured and passed to the <svg> element. All other props are silently ignored, which could lead to unexpected behavior if callers expect those props to be applied. [ Already posted ]
frontend/src/public/components/icons/StartRoundIcon.tsx — 0 comments posted, 1 evaluated, 1 filtered
  • line 6: The component now only extracts fill and className from props, discarding all other props. Previously, the component used ...rest spread to forward additional props (like onClick, aria-label, style, id, tabIndex, etc.) to the underlying <svg> element. Any code that passes additional props to StartRoundIcon will now silently ignore those props, breaking event handlers, accessibility attributes, and styling. [ Already posted ]
frontend/src/public/components/icons/TableViewIcon.tsx — 0 comments posted, 1 evaluated, 1 filtered
  • line 4: The component accepts React.SVGAttributes<SVGElement> as props but only destructures fill without spreading remaining props to the <svg> element. If callers pass other standard SVG attributes like className, style, onClick, or aria-label, they will be silently ignored, causing unexpected behavior. [ Low confidence ]
frontend/src/public/layout/TasksLayout/TasksLayout.tsx — 0 comments posted, 2 evaluated, 2 filtered
  • line 199: The FilterSelect for step filtering has optionIdKey="id" (line 199) but selectedOption={taskApiNameFilter} (line 197) where taskApiNameFilter is a string (apiName). The component will attempt to match a string apiName against numeric id values, causing the selected step to never be recognized as selected by the FilterSelect internal logic. The optionIdKey should be "apiName" to match the selectedOption type. [ Already posted ]
  • line 201: The onChange prop for the step FilterSelect (line 201) is set to an empty function () => {}. While step selection is handled via customClickHandler in the options, if FilterSelect internally depends on onChange being called for state updates or if any consuming code expects onChange to fire, this could cause the step filter to not properly update through the standard component interface. [ Low confidence ]
frontend/src/public/layout/WorkflowsLayout/StarterFilterSelect.tsx — 0 comments posted, 1 evaluated, 1 filtered
  • line 21: In StarterFilterSelect, getActiveUsers(users) is called on line 21 with the result of useSelector(getAccountsUsers). If state.accounts.users is undefined or null (e.g., before data loads or if the state shape is incorrect), getActiveUsers will call .filter() on undefined, causing a runtime error: Cannot read properties of undefined (reading 'filter'). [ Low confidence ]
frontend/src/public/layout/WorkflowsLayout/TaskFilterSelect.tsx — 0 comments posted, 1 evaluated, 1 filtered
  • line 46: The options objects created in TaskFilterSelect (lines 46-52) destructure { name, number, count, ...rest } from TTemplateStepFilter and spread ...rest. The FilterSelect component uses optionIdKey="apiName" to identify options, but there's no guarantee that TTemplateStepFilter (which extends ITemplateStep) contains an apiName property. If apiName is not present in the step data, the filter selection logic will fail to match selected options. [ Low confidence ]
frontend/src/public/layout/WorkflowsLayout/TemplateFilterSelect.tsx — 0 comments posted, 1 evaluated, 1 filtered
  • line 21: In TemplateFilterSelect, the useMemo on line 18-23 maps over filterTemplates and accesses count property. If any item in filterTemplates lacks the count property (i.e., count is undefined), the expression count > 0 evaluates to false, which may hide items that should show a count if the data shape is inconsistent. [ Low confidence ]
frontend/src/public/layout/WorkflowsLayout/WorkflowsLayout.tsx — 0 comments posted, 4 evaluated, 4 filtered
  • line 84: The initialization of prevStatusFilterRef to EWorkflowsStatus.Running and prevSortingRef to EWorkflowsSorting.DateDesc (lines 84-85) assumes these are the actual initial prop values. If the component mounts with different values for statusFilter or sorting, the first call to checkFilterDependenciesChanged will incorrectly detect a change, triggering unnecessary filter applications or counter updates. [ Low confidence ]
  • line 186: At lines 186-188, when currentTemplateTaskApiNamesSet.size === 0, the code returns all existing tasksApiNamesFilter values as prevTemplatesActualTaskApiNames. This may retain stale task API names that no longer correspond to any selected template, potentially causing unexpected filter behavior when templates are deselected. [ Low confidence ]
  • line 303: The currentFiltersValuesRef is read in useLayoutEffect (line 303) but is only updated in a useEffect (lines 271-289). Since useLayoutEffect runs synchronously after DOM mutations but before useEffect, the ref will contain stale values from the previous render cycle when the layout effect accesses it, causing incorrect filter value serialization. [ Already posted ]
  • line 311: The useLayoutEffect at line 297 uses changedFiltersRef.current.size as a dependency, but React does not track changes to ref properties. This means the effect will not re-run when changedFiltersRef.current is modified, causing the previous filter refs (prevStatusFilterRef, prevTemplatesIdsFilterRef, etc.) to never be updated. This breaks the change detection logic in checkFilterDependenciesChanged, potentially causing filters to never apply or to apply repeatedly. [ Already posted ]
frontend/src/public/layout/WorkflowsLayout/container.ts — 0 comments posted, 1 evaluated, 1 filtered
  • line 91: Type mismatch in getQueryParamByProp at line 91: the function is typed as (value: number[]) but tasksApiNamesFilter contains string API names (as evidenced by the createAction at lines 88-89 which parses them as strings without converting to numbers). This inconsistency could cause type-related issues if downstream code relies on the type annotation. [ Already posted ]
frontend/src/public/layout/WorkflowsLayout/utils.ts — 0 comments posted, 2 evaluated, 2 filtered
  • line 47: In TaskFilterSelect, the options array created in groupedSteps has name set to a React element (<StepName ... />), not a string. When getRenderPlaceholder is called with type === ERenderPlaceholderType.Task, it returns selectedOption?.name (line 47), which would be a JSX element, not a string. This causes inconsistent return types - the function returns strings in all other cases but a React element for tasks, which may cause rendering issues or type errors depending on how the placeholder is used. [ Already posted ]
  • line 54: In getRenderPlaceholder, when selectedOption is not found (undefined) and type is ERenderPlaceholderType.Starter or ERenderPlaceholderType.Performer with filterType === 'userType', the function calls getUserFullName(selectedOption) which returns an empty string '' for undefined input. This silently returns an empty placeholder instead of showing a meaningful fallback like the unknown filter message used for other types. [ Already posted ]

Maria-Lordwill and others added 30 commits October 27, 2025 15:05
…ces and update related queries and serializers to use template_task_api_name instead
…to frontend/tech_debt/36989__moving_template_task_id_to_template_task_api_name
… from templateTaskId to templateTaskApiName and replace the stepIdFilter entity with the taskApiNameFilter entity
…component and getRenderPlaceholder into a separate function
…e table from a 'starters' filter to plain text
…maticapp/pneumaticworkflow into frontend/workflows/43469__move_filters_to_header
…GET /templates/titles-by-tasks and in the response rename the property workflows_count to count
…ET /templates/titles-by-workflows and in the response rename the property workflows_count to count
- for the 'FilterSelect' component, add the ability to disable it
- in 'workflowTable' component: replace the header in the 'performers' column in the table from a 'performers' filter to plain text
- in the performers filter at the top: added the icon and added displaying text on the badge
…m one template to multiple templates; remove the 'All options' checkbox from all filters on the workflow page
…s to the loadTemplateSteps logic (preserve selected tasks for existing templates and remove tasks for deleted templates)

return (
<button
onClick={() => dispatch(setWorkflowsFilterTemplate([value.template?.id]))}
Copy link

Choose a reason for hiding this comment

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

onClick may dispatch [undefined] when value.template is missing. Consider guarding so it only dispatches when template.id exists to avoid invalid filter values.

Suggested change
onClick={() => dispatch(setWorkflowsFilterTemplate([value.template?.id]))}
onClick={() => value.template?.id && dispatch(setWorkflowsFilterTemplate([value.template.id]))}

🚀 Reply to ask Macroscope to explain or update this suggestion.

👍 Helpful? React to give us feedback.

renderPlaceholder,
} = props;

const allOptions = flatGroupedOptions || options;
Copy link

Choose a reason for hiding this comment

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

allOptions uses options when groupedOptions is provided without flatGroupedOptions, so the placeholder, Select All, and “all selected” checks operate on the wrong set. Consider deriving allOptions by flattening groupedOptions when present.

Suggested change
const allOptions = flatGroupedOptions || options;
const allOptions = flatGroupedOptions || (groupedOptions ? Array.from(groupedOptions.values()).flatMap(g => g.options) : options);

🚀 Reply to ask Macroscope to explain or update this suggestion.

👍 Helpful? React to give us feedback.

type === ERenderPlaceholderType.Starter ||
(type === ERenderPlaceholderType.Performer && filterType === 'userType')
) {
return getUserFullName(selectedOption);
Copy link

Choose a reason for hiding this comment

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

Calling getUserFullName(selectedOption) when selectedOption is undefined yields an empty string, so the placeholder renders blank. Consider adding the same fallback used in the other branches (e.g., use the sorting.unknown-filter-value message with type) so a meaningful placeholder is shown.

Suggested change
return getUserFullName(selectedOption);
return getUserFullName(selectedOption) || `${formatMessage({ id: 'sorting.unknown-filter-value' })} ${type}`;

🚀 Reply to ask Macroscope to explain or update this suggestion.

👍 Helpful? React to give us feedback.

Comment on lines 171 to 186
useEffect(() => {
if (templatesIdsFilter.length === 0) {
setStepsFilter([]);
return;
}

if (isArrayWithItems(currentTemplate.steps)) {
if (!isArrayWithItems(stepsIdsFilter)) {
setStepsFilter(currentTemplate.steps.map((s) => s.id));
const currentTemplateTaskIdsSet = new Set(
selectedTemplates.flatMap((template) => template.steps.map((step) => step.id)),
);
const prevTemplatesActualTaskIds =
currentTemplateTaskIdsSet.size === 0
? stepsIdsFilter
: stepsIdsFilter.filter((id) => currentTemplateTaskIdsSet.has(id));

selectedTemplates.forEach((template) => {
const hasTasks = template.steps.length > 0;
Copy link

Choose a reason for hiding this comment

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

loadingTaskRef never removes IDs for deselected templates, so a removed template can be treated as "already loading" forever. Consider clearing it when no templates are selected and pruning IDs not in templatesIdsFilter before loading.

-    if (templatesIdsFilter.length === 0) {
-      setStepsFilter([]);
-      return;
-    }
+    if (templatesIdsFilter.length === 0) {
+      setStepsFilter([]);
+      loadingTaskRef.current.clear();
+      return;
+    }
+
+    loadingTaskRef.current.forEach((id) => {
+      if (!templatesIdsFilter.includes(id)) {
+        loadingTaskRef.current.delete(id);
+      }
+    });

🚀 Reply to ask Macroscope to explain or update this suggestion.

👍 Helpful? React to give us feedback.

<>
{props.selectedOption && renderResetOption()}
{rendeSelectAllOption()}
{selectedOption !== null && renderResetOption()}
Copy link

Choose a reason for hiding this comment

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

renderResetOption() now runs in multi-select because selectedOption !== null is true when selectedOption is undefined. Consider guarding by select mode (and nullish) so the reset option only shows for single-select.

Suggested change
{selectedOption !== null && renderResetOption()}
{!isMultiple && selectedOption != null && renderResetOption()}

🚀 Reply to ask Macroscope to explain or update this suggestion.

👍 Helpful? React to give us feedback.

EBirkenfeld and others added 5 commits December 16, 2025 15:24
…rkflows update when selecting a group; also removed the unused CSS class dropdown__active-value
…hub.com:pneumaticapp/pneumaticworkflow into frontend/tech_debt/36989__moving_template_task_id_to_template_task_api_name
…frontend/workflows/43180__add_a_transition_to_the_corresponding_filter
return new Map(
selectedTemplates.map((template) => [
template.id,
{
Copy link

Choose a reason for hiding this comment

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

Task placeholder may render [object Object] because option name is a JSX element. Consider reading a plain-text field (e.g., searchByText) in getRenderPlaceholder when type is Task, falling back to name if needed.

-    return selectedOption?.name || `${formatMessage({ id: 'sorting.unknown-filter-value' })} ${type}`;
+    return (selectedOption?.searchByText || selectedOption?.name) || `${formatMessage({ id: 'sorting.unknown-filter-value' })} ${type}`;

🚀 Reply to ask Macroscope to explain or update this suggestion.

👍 Helpful? React to give us feedback.

Comment on lines 286 to +307
{foundValues.map((option) => {
const label = (
<div className={styles['dropdown-item-content']}>
<div className={styles['dropdown-item-content__text']}>{option[optionLabelKey]}</div>

{typeof option.count !== 'undefined' && (
<span className={styles['dropdown-item-content__count']}>{option.count}</span>
)}
</div>
);
let label: ReactNode | null = null;

if (typeof option !== 'string') {
label = (
<div className={styles['dropdown-item-content']}>
<div className={styles['dropdown-item-content__text']}>{option[optionLabelKey]}</div>

{typeof option.count !== 'undefined' && (
<span className={styles['dropdown-item-content__count']}>{option.count}</span>
)}
</div>
);
} else {
label = <div className={styles['dropdown-item-content__title']}>{option}</div>;
}

return (
<DropdownItem
key={option[optionIdKey]}
key={typeof option !== 'string' ? option[optionIdKey] : option}
className={classnames('dropdown-item-sm', styles['value-item'])}
onClick={handleChange(option)}
toggle={!props.isMultiple}
onClick={typeof option !== 'string' ? handleChange(option) : () => {}}
Copy link

Choose a reason for hiding this comment

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

Some list items use non-unique or undefined React keys, which risks warnings/misrenders. Suggest generating stable, unique keys (e.g., include index or a stable id) instead of raw labels/sparse values.

-        {foundValues.map((option) => {
+        {foundValues.map((option, idx) => {
@@
-            <DropdownItem
-              key={typeof option !== 'string' ? option[optionIdKey] : option}
+            <DropdownItem
+              key={typeof option !== 'string' ? option[optionIdKey] : `group-title-${idx}-${option}`}

🚀 Reply to ask Macroscope to explain or update this suggestion.

👍 Helpful? React to give us feedback.

This commit completes task 36989:
- for GET /workflows/ and GET /workflows/count/by-current-performer
  switched from template_task_ids to template_task_api_names
- replaced the stepsIdsFilter entity with the tasksApiNamesFilter entity
setTasksFilter(prevTemplatesActualTaskApiNames);
}
}, [workflowsView, templatesIdsFilter[0], filterTemplates, statusFilter, isLoadSteps]);
}, [templatesIdsFilter, selectedTemplates, statusFilter]);
Copy link

Choose a reason for hiding this comment

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

Several useEffect/useCallback hooks read values missing (or wrong) in their dependency arrays, causing stale state/closures. Suggest aligning each hook’s deps with the variables it uses and the correct keys (e.g., templatesIdsFilter vs tasksApiNamesFilter) so updates fire reliably.

Suggested change
}, [templatesIdsFilter, selectedTemplates, statusFilter]);
}, [templatesIdsFilter, selectedTemplates, statusFilter, tasksApiNamesFilter]);

🚀 Want me to fix this? Reply ex: "fix it for me".

const tasks = queryParam.split(',');
return setWorkflowsFilterTasks(tasks);
},
getQueryParamByProp: (value: number[]) => value.join(','),
Copy link

Choose a reason for hiding this comment

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

getQueryParamByProp takes number[], but tasksApiNamesFilter is string[]. Consider typing it as string[] to keep types accurate and avoid missed errors.

🚀 Want me to fix this? Reply ex: "fix it for me".

}

return setWorkflowsFilterSteps([]);
const tasks = queryParam.split(',');
Copy link

Choose a reason for hiding this comment

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

tasksApiNamesFilter’s createAction treats an empty tasks query as ['']. Consider handling empty input (or filtering out empty strings) so an empty tasks= yields an empty array.

-        const tasks = queryParam.split(',');
+        const tasks = queryParam ? queryParam.split(',').filter(Boolean) : [];

🚀 Want me to fix this? Reply ex: "fix it for me".

@@ -0,0 +1,59 @@
import React, { useMemo } from 'react';
import { useIntl } from 'react-intl';
import { useDispatch, useSelector } from 'react-redux';
Copy link

Choose a reason for hiding this comment

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

ERenderPlaceholderType import breaks because utils.ts doesn’t export it. Consider exporting the enum to match the import.

-enum ERenderPlaceholderType {
+export enum ERenderPlaceholderType {

🚀 Want me to fix this? Reply ex: "fix it for me".

Comment on lines +4 to +13
export function PerformerFilterIcon({ className }: React.SVGAttributes<SVGElement>) {
return (
<svg
width="20"
height="20"
viewBox="0 0 20 20"
fill="none"
xmlns="http://www.w3.org/2000/svg"
className={className}
>
Copy link

Choose a reason for hiding this comment

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

Props are typed as React.SVGAttributes<SVGElement>, but only className is forwarded. Consider spreading the rest onto the <svg> so onClick, aria-*, etc. work.

Suggested change
export function PerformerFilterIcon({ className }: React.SVGAttributes<SVGElement>) {
return (
<svg
width="20"
height="20"
viewBox="0 0 20 20"
fill="none"
xmlns="http://www.w3.org/2000/svg"
className={className}
>
export function PerformerFilterIcon({ className, ...props }: React.SVGAttributes<SVGElement>) {
return (
<svg
width="20"
height="20"
viewBox="0 0 20 20"
fill="none"
xmlns="http://www.w3.org/2000/svg"
className={className} {...props}
>

🚀 Want me to fix this? Reply ex: "fix it for me".

const [colWidths, setColWidths] = useState<Record<string, number>>({});
const [isСhangeTemplateId, setIsСhangeTemplateId] = useState(false);

const { isMobile } = useCheckDevice();
Copy link

Choose a reason for hiding this comment

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

Duplicate useCheckDevice() calls. Consider calling it once, destructuring both isDesktop and isMobile, and removing the second call to avoid extra useMediaQuery work and possible timing inconsistencies.

🚀 Want me to fix this? Reply ex: "fix it for me".

@@ -171,67 +162,6 @@
}
}, [currentTemplateId]);
Copy link

Choose a reason for hiding this comment

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

Several hooks read values that aren’t listed in their dependency arrays, which can cause stale state/closures. Suggest adding all referenced values to each useEffect/useCallback deps so behavior stays in sync.

-  }, [currentTemplateId]);
+  }, [currentTemplateId, lastLoadedTemplateIdForTable]);

🚀 Want me to fix this? Reply ex: "fix it for me".

</Tooltip>
) : (
<button
onClick={() => dispatch(setFilterTemplateTasks([singleActiveTaskApiName]))}
Copy link

Choose a reason for hiding this comment

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

singleActiveTaskApiName can be undefined when areMultipleTasks is false. Consider guarding the click handler so we don’t dispatch [undefined].

Suggested change
onClick={() => dispatch(setFilterTemplateTasks([singleActiveTaskApiName]))}
() => {
if (!singleActiveTaskApiName) return;
dispatch(setFilterTemplateTasks([singleActiveTaskApiName]));
}

🚀 Want me to fix this? Reply ex: "fix it for me".

templateId && `template_id=${templateId}`,
templateStepId && `template_task_id=${templateStepId}`,
].filter(Boolean).join('&');
templateTaskApiName && `template_task_api_name=${templateTaskApiName}`,
Copy link

Choose a reason for hiding this comment

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

Consider URL-encoding templateTaskApiName before adding it to the query; otherwise values with &, = or spaces will break the URL.

Suggested change
templateTaskApiName && `template_task_api_name=${templateTaskApiName}`,
templateTaskApiName && `template_task_api_name=${encodeURIComponent(templateTaskApiName)}`,

🚀 Want me to fix this? Reply ex: "fix it for me".

selectedOption={stepIdFilter}
selectedOption={taskApiNameFilter}
options={getStepsFilterOptions()}
optionIdKey="id"
Copy link

Choose a reason for hiding this comment

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

FilterSelect can’t mark the selected step: selectedOption uses apiName (string | null) but optionIdKey is "id" (number). Consider switching to optionIdKey="apiName" to match taskApiNameFilter.

Suggested change
optionIdKey="id"
optionIdKey="apiName"

🚀 Want me to fix this? Reply ex: "fix it for me".

} = this.props;
const editProcess = { name, kickoff: getEditKickoff(kickoff) };
const { name, kickoff } = workflow;
const prevKickoffId = prevProps.workflow?.kickoff.id;
Copy link

Choose a reason for hiding this comment

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

prevProps.workflow?.kickoff.id can throw when kickoff is null/undefined. Consider prevProps.workflow?.kickoff?.id to guard.

Suggested change
const prevKickoffId = prevProps.workflow?.kickoff.id;
const prevKickoffId = prevProps.workflow?.kickoff?.id;

🚀 Want me to fix this? Reply ex: "fix it for me".

Comment on lines +134 to 136
if (taskApiNAme) {
return setFilterStep(taskApiNAme);
}
Copy link

Choose a reason for hiding this comment

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

taskApiNameFilter uses String in getQueryParamByProp, so null becomes 'null'. createAction treats this as truthy and applies an invalid filter. Consider ignoring the literal 'null' and mapping it to null.

Suggested change
if (taskApiNAme) {
return setFilterStep(taskApiNAme);
}
if (taskApiNAme && taskApiNAme !== 'null') {
return setFilterStep(taskApiNAme);
}

🚀 Want me to fix this? Reply ex: "fix it for me".

const timezone = useSelector(getTimezone);

const isUrl = (str: string) => {
return urlWithProtocolRegex.test(str);
Copy link

Choose a reason for hiding this comment

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

isUrl uses a g-flagged urlWithProtocolRegex, so lastIndex persists and makes test flaky. Consider resetting lastIndex before testing.

Suggested change
return urlWithProtocolRegex.test(str);
urlWithProtocolRegex.lastIndex = 0;
return urlWithProtocolRegex.test(str);

🚀 Want me to fix this? Reply ex: "fix it for me".

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

Labels

Frontend Web client changes request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants