-
Notifications
You must be signed in to change notification settings - Fork 15
43180 frontend [ workflows ] add a transition to the corresponding filter #90
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
43180 frontend [ workflows ] add a transition to the corresponding filter #90
Conversation
…plate_task_api_name
…ces and update related queries and serializers to use template_task_api_name instead
…s to use task_id instead of task_api_name
…eter to templateTaskApiName
…to frontend/tech_debt/36989__moving_template_task_id_to_template_task_api_name
…988__delete_template_task_id
… 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
…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)
…) and clean up code
…to frontend/workflows/43180__add_a_transition_to_the_corresponding_filter
…thub.com:pneumaticapp/pneumaticworkflow into frontend/workflows/43180__add_a_transition_to_the_corresponding_filter
…'performer icon' in the table
|
|
||
| return ( | ||
| <button | ||
| onClick={() => dispatch(setWorkflowsFilterTemplate([value.template?.id]))} |
There was a problem hiding this comment.
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.
| 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; |
There was a problem hiding this comment.
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.
| 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); |
There was a problem hiding this comment.
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.
| 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.
| 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; |
There was a problem hiding this comment.
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()} |
There was a problem hiding this comment.
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.
| {selectedOption !== null && renderResetOption()} | |
| {!isMultiple && selectedOption != null && renderResetOption()} |
🚀 Reply to ask Macroscope to explain or update this suggestion.
👍 Helpful? React to give us feedback.
…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, | ||
| { |
There was a problem hiding this comment.
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.
| {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) : () => {}} |
There was a problem hiding this comment.
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]); |
There was a problem hiding this comment.
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.
| }, [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(','), |
There was a problem hiding this comment.
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(','); |
There was a problem hiding this comment.
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'; | |||
There was a problem hiding this comment.
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".
| 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} | ||
| > |
There was a problem hiding this comment.
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.
| 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".
…'task' in the table
| const [colWidths, setColWidths] = useState<Record<string, number>>({}); | ||
| const [isСhangeTemplateId, setIsСhangeTemplateId] = useState(false); | ||
|
|
||
| const { isMobile } = useCheckDevice(); |
There was a problem hiding this comment.
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]); | |||
There was a problem hiding this comment.
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]))} |
There was a problem hiding this comment.
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].
| 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}`, |
There was a problem hiding this comment.
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.
| 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" |
There was a problem hiding this comment.
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.
| 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; |
There was a problem hiding this comment.
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.
| const prevKickoffId = prevProps.workflow?.kickoff.id; | |
| const prevKickoffId = prevProps.workflow?.kickoff?.id; |
🚀 Want me to fix this? Reply ex: "fix it for me".
| if (taskApiNAme) { | ||
| return setFilterStep(taskApiNAme); | ||
| } |
There was a problem hiding this comment.
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.
| 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); |
There was a problem hiding this comment.
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.
| return urlWithProtocolRegex.test(str); | |
| urlWithProtocolRegex.lastIndex = 0; | |
| return urlWithProtocolRegex.test(str); |
🚀 Want me to fix this? Reply ex: "fix it for me".
Add a transition to the corresponding filter by switching workflows filtering from step IDs to task API names and expose new
/templates/titles-by-workflowsand/templates/titles-by-tasksendpoints across backend and frontendReplace 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 newTemplateTitlesByTasksQuery, and UI wiring forTaskColumnand filter selects.📍Where to Start
Start with
TemplateViewSetactions in backend/src/processes/views/template.py, then reviewTemplateTitlesByWorkflowsQueryandTemplateTitlesByTasksQueryin 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
TaskStatus.LITERALStype uses class attributes (PENDING,ACTIVE, etc.) insideLiteral[]. 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, theLiteraltype could contain unexpected values. Using explicit string literals likeLiteral['pending', 'active', 'completed', 'snoozed', 'skipped']would be more robust. [ Low confidence ]backend/src/processes/queries.py — 0 comments posted, 3 evaluated, 3 filtered
tt.id AS template_task_idfrom the SELECT clause is a breaking change. Any code that consumes the results of this query and accesses thetemplate_task_idcolumn will fail at runtime with a KeyError or similar error when the column is no longer present in the result set. [ Low confidence ]pt.id as template_task_idfrom the SELECT clause breaks any code that accessestemplate_task_idfrom the query results. Sinceget_sql()usesSELECT *from this inner query, any consumer expectingtemplate_task_idin the result set will fail with a KeyError or AttributeError at runtime. [ Low confidence ]_get_wheremethod only handlesTaskStatus.ACTIVE,TaskStatus.DELAYED, and an implicit completed case in the else branch. IfstatusisTaskStatus.PENDINGorTaskStatus.SKIPPED(both valid values perTaskStatus.LITERALS), the else branch executes withis_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
action_serializer_classeswas changed to rename'titles'to'titles_by_workflows', but theget_permissionsmethod at line 154 still references'titles'instead of'titles_by_workflows'. This means thetitles_by_workflowsaction will not match the permission check and will fall through to the default permissions at lines 180-186, which incorrectly requiresUserIsAdminOrAccountOwner(). 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 ]get_permissionsmethod 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 meanstitles_by_workflowsaction will not match the condition on line 152-158 and will fall through to the default permissions on lines 180-186, which incorrectly requiresUserIsAdminOrAccountOwner()andUsersOverlimitedPermission()permissions. This is more restrictive than intended and will deny access to non-admin/non-owner users who should have access. [ Already posted ]titles_by_workflowsis missing from the permissions check at line 154. The action was renamed fromtitlestotitles_by_workflows(as shown in the diff), but the string'titles'inget_permissions()was not updated to'titles_by_workflows'. This causes thetitles_by_workflowsaction to fall through to the default permissions at line 180-186, which requiresUserIsAdminOrAccountOwner()- 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
getTemplatesTitlesByTasks,urls.templatesTitlesByTasksis accessed on line 12 but there is no guarantee this property exists in theurlsconfiguration object. IftemplatesTitlesByTasksis not defined in the API config,baseUrlwill beundefined, 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
getUserTasksQueryString, whenIS_STATUS_COMPLETEDis false, the code accessesQS_BY_SORTING[sorting]on line 87. However, thesortingparameter type isETaskListSorting | ETaskListCompleteSorting. WhileETaskListCompleteSortingvalues overlap withETaskListSorting, the type system doesn't guarantee this at runtime if the enum values diverge in the future, potentially returningundefinedfor the sorting query parameter. [ Low confidence ]frontend/src/public/components/Dashboard/Breakdowns/TaskItem.tsx — 0 comments posted, 1 evaluated, 1 filtered
useCallbackforgetRouteat line 26 has an incomplete dependency array[mode]. The callback usestask.apiName,task.id, andtemplateId, but none of these are in the dependency array. This will cause stale closures where the routes generated will use outdated values oftask.apiName,task.id, ortemplateIdwhen 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
useEffectdependency array on line 53 is missingsavedFields. The effect usessavedFieldsto initializeselectedFieldsstate, but ifsavedFieldschanges in Redux whileisOpen,templateId, andtemplateTasksremain 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
selectedOption !== nullon line 284 incorrectly renders the reset option whenisMultipleis true. WhenisMultipleis true, the interface definesselectedOption?: never, meaningselectedOptionisundefined. Sinceundefined !== nullevaluates totrue,renderResetOption()will always be called in multi-select mode, which is unintended. The original code usedprops.selectedOption && renderResetOption()which correctly returned falsy forundefined. The fix should check!isMultiple && selectedOption !== null. [ Already posted ]OutsideClickHandlerwrapper (lines 351-421) combined with reactstrap'sDropdowncomponent may cause conflicting outside-click handling behavior. Both components may attempt to handle outside clicks, potentially causing double-toggling or unexpected dropdown state. TheDropdowncomponent's internal toggle mechanism via thetoggleprop combined withOutsideClickHandler'sonOutsideClickcalling the samehandleToggleDropdowncould lead to race conditions. [ Low confidence ]frontend/src/public/components/UI/Select/SelectMenu.tsx — 0 comments posted, 1 evaluated, 1 filtered
radioStyles['select-menu__radio--checked']on line 97 appears to reference a class that likely doesn't exist inRadioButton.css. The naming conventionselect-menu__radio--checkedis 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 whenwithRadiois true and an item is selected. [ Low confidence ]frontend/src/public/components/Workflows/WorkflowsGridPage/WorkflowsGridPage.tsx — 0 comments posted, 4 evaluated, 4 filtered
loadTemplatesTitles()call from the component meanstemplatesFiltermay never be populated if it was previously loaded by this component. IftemplatesFilterrelies on this call being made,handleRunNewWorkflowwill always fall through toopenSelectTemplateModal()on line 82 whentemplatesFilteris empty. [ Low confidence ]itemis used as thekeyprop, but sinceArray(N)creates empty slots,itemwill beundefinedfor 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 ]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 useArray.from({ length: INIT_SKELETION_QUANTITY }).map(...)orArray(INIT_SKELETION_QUANTITY).fill(null).map(...). [ Out of scope ]Array(SCROLL_LOADERS_QUANTITY).map((item) => ...)has the same empty slots bug -.map()will not iterate over empty array slots, resulting in an emptyloaderarray passed toInfiniteScroll. Should useArray.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
Object.keys(multipleTasksNamesByApiNames)[0]at line 23 returnsundefinedwhenmultipleTasksNamesByApiNamesis an empty object. This undefined value is then dispatched tosetFilterTemplateTasksat line 26 when the button is clicked, which could cause unexpected filter behavior or errors in the reducer. [ Already posted ]templateis null or undefined,template?.id ?? 0at line 27 dispatches0as the template filter. Filtering by template ID0may 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
value.templateisnullorundefined, clicking the button will dispatchsetWorkflowsFilterTemplate([undefined]). This passes an array containingundefinedto 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
useCheckDevice()is called twice inWorkflowsTable: once at line 98 to getisDesktopand again at line 126 to getisMobile. 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 likeconst { isMobile, isDesktop } = useCheckDevice();. [ Already posted ]frontend/src/public/components/icons/PerformerFilterIcon.tsx — 0 comments posted, 1 evaluated, 1 filtered
React.SVGAttributes<SVGElement>as its props type, which allows callers to pass any SVG attribute (likeonClick,aria-label,style, etc.), but onlyclassNameis 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
fillandclassNamefrom props, discarding all other props. Previously, the component used...restspread to forward additional props (likeonClick,aria-label,style,id,tabIndex, etc.) to the underlying<svg>element. Any code that passes additional props toStartRoundIconwill 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
React.SVGAttributes<SVGElement>as props but only destructuresfillwithout spreading remaining props to the<svg>element. If callers pass other standard SVG attributes likeclassName,style,onClick, oraria-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
FilterSelectfor step filtering hasoptionIdKey="id"(line 199) butselectedOption={taskApiNameFilter}(line 197) wheretaskApiNameFilteris astring(apiName). The component will attempt to match a string apiName against numericidvalues, causing the selected step to never be recognized as selected by theFilterSelectinternal logic. TheoptionIdKeyshould be"apiName"to match theselectedOptiontype. [ Already posted ]onChangeprop for the stepFilterSelect(line 201) is set to an empty function() => {}. While step selection is handled viacustomClickHandlerin the options, ifFilterSelectinternally depends ononChangebeing called for state updates or if any consuming code expectsonChangeto 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
StarterFilterSelect,getActiveUsers(users)is called on line 21 with the result ofuseSelector(getAccountsUsers). Ifstate.accounts.usersisundefinedornull(e.g., before data loads or if the state shape is incorrect),getActiveUserswill call.filter()onundefined, 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
optionsobjects created inTaskFilterSelect(lines 46-52) destructure{ name, number, count, ...rest }fromTTemplateStepFilterand spread...rest. TheFilterSelectcomponent usesoptionIdKey="apiName"to identify options, but there's no guarantee thatTTemplateStepFilter(which extendsITemplateStep) contains anapiNameproperty. IfapiNameis 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
TemplateFilterSelect, theuseMemoon line 18-23 maps overfilterTemplatesand accessescountproperty. If any item infilterTemplateslacks thecountproperty (i.e.,countisundefined), the expressioncount > 0evaluates tofalse, 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
prevStatusFilterReftoEWorkflowsStatus.RunningandprevSortingReftoEWorkflowsSorting.DateDesc(lines 84-85) assumes these are the actual initial prop values. If the component mounts with different values forstatusFilterorsorting, the first call tocheckFilterDependenciesChangedwill incorrectly detect a change, triggering unnecessary filter applications or counter updates. [ Low confidence ]currentTemplateTaskApiNamesSet.size === 0, the code returns all existingtasksApiNamesFiltervalues asprevTemplatesActualTaskApiNames. 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 ]currentFiltersValuesRefis read inuseLayoutEffect(line 303) but is only updated in auseEffect(lines 271-289). SinceuseLayoutEffectruns synchronously after DOM mutations but beforeuseEffect, the ref will contain stale values from the previous render cycle when the layout effect accesses it, causing incorrect filter value serialization. [ Already posted ]useLayoutEffectat line 297 useschangedFiltersRef.current.sizeas a dependency, but React does not track changes to ref properties. This means the effect will not re-run whenchangedFiltersRef.currentis modified, causing the previous filter refs (prevStatusFilterRef,prevTemplatesIdsFilterRef, etc.) to never be updated. This breaks the change detection logic incheckFilterDependenciesChanged, 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
getQueryParamByPropat line 91: the function is typed as(value: number[])buttasksApiNamesFiltercontains string API names (as evidenced by thecreateActionat 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
TaskFilterSelect, theoptionsarray created ingroupedStepshasnameset to a React element (<StepName ... />), not a string. WhengetRenderPlaceholderis called withtype === ERenderPlaceholderType.Task, it returnsselectedOption?.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 ]getRenderPlaceholder, whenselectedOptionis not found (undefined) andtypeisERenderPlaceholderType.StarterorERenderPlaceholderType.PerformerwithfilterType === 'userType', the function callsgetUserFullName(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 ]