42381 frontend [ workflows ] When cleaning and changing filters, the results does not match the filter#98
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)
This comment was marked as spam.
This comment was marked as spam.
…frontend/workflows/43180__add_a_transition_to_the_corresponding_filter
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
Removed default empty arrays from getTemplatesTasks and getTemplatesVariables selectors, which caused Maximum update depth exceeded error for non-existent templateId
Maria-Lordwill
added
bug
…sponding_filter' into frontend/workflows/42381__when_cleaning_and_changing_filters_the_results_does_not_match_the_filter
…terRequests action
| placeholder, | ||
| type = 'text', | ||
| icon, | ||
| ref, |
There was a problem hiding this comment.
If a caller passes an id prop, it overwrites the generated id on the input (via ...props), but htmlFor still uses the generated inputId, breaking label-input association. Consider destructuring id from props and using id || inputId for both attributes.
🚀 Want me to fix this? Reply ex: "fix it for me".
| user: User, | ||
| with_tasks_in_progress: Optional[bool] = None, | ||
| workflows_status: Optional[WorkflowStatus] = None, | ||
| status: Optional[WorkflowStatus] = None, |
There was a problem hiding this comment.
status is typed as WorkflowStatus (integers like 0, 1, 3), but WorkflowApiStatus.MAP expects WorkflowApiStatus string keys ('running', 'done', 'snoozed'). This will raise KeyError. Consider fixing the type hint to Optional[WorkflowApiStatus] or changing the lookup logic.
| status: Optional[WorkflowStatus] = None, | |
| status: Optional[WorkflowApiStatus] = None, |
🚀 Want me to fix this? Reply ex: "fix it for me".
| export type TGetTemplatesTitlesByTasksResponse = ITemplateTitleBaseWithCount[]; | ||
|
|
||
| export function getTemplatesTitlesByTasks(completionStatus: ETaskListCompletionStatus) { | ||
| const { |
There was a problem hiding this comment.
Possible runtime error: code assumes config/data always exist. Suggest guarding before destructuring/iterating (e.g., ?., default values) so missing values don’t throw.
🚀 Want me to fix this? Reply ex: "fix it for me".
…ormersCounters and cancelTemplateTasksCounters actions
| return; | ||
| } | ||
|
|
||
| if (result) { |
There was a problem hiding this comment.
I think the result check is redundant here.
…eformer id' in the URL is unknown
…y selecting all options
…frontend/workflows/42381__when_cleaning_and_changing_filters_the_results_does_not_match_the_filter
Maria-Lordwill
dismissed
pneumatic-ilyacedrik’s stale review
The merge-base changed after approval.
5 participants
Fix workflows filters to use task API names and add cancelable filter requests, updating endpoints to
GET /templates/titles-by-workflowsandGET /templates/titles-by-taskswithcountresultsReplace step-id filtering with task API name filtering across backend serializers, queries, and frontend sagas/slices; add cancelable filter/preset/steps requests; introduce task- and workflow-based template titles endpoints and UI filters; update tables to a clickable Task column and propagate
countin titles responses.📍Where to Start
Start with the templates titles endpoints in
processes.views.template.TemplateViewSetat [file:backend/src/processes/views/template.py], then review query changes inTemplateTitlesByWorkflowsQueryandTemplateTitlesByTasksQueryat [file:backend/src/processes/queries.py], followed by the workflows slice and saga at [file:frontend/src/public/redux/workflows/slice.ts] and [file:frontend/src/public/redux/workflows/saga.ts].📊 Macroscope summarized ecb1efa. 36 files reviewed, 22 issues evaluated, 11 issues filtered, 0 comments posted
🗂️ Filtered Issues
backend/src/processes/queries.py — 0 comments posted, 4 evaluated, 2 filtered
statusparameter is typed asOptional[WorkflowStatus](an integer enum with values 0, 1, 3), but the code attempts to use it as a key inWorkflowApiStatus.MAPwhich has string keys ('running','done','snoozed'). This will raise aKeyErrorat runtime when a validWorkflowStatusinteger is passed. The lookupWorkflowApiStatus.MAP[status]expects aWorkflowApiStatusstring, not aWorkflowStatusinteger. [ Already posted ]_get_wheremethod only explicitly handlesTaskStatus.ACTIVEandTaskStatus.DELAYED, butTaskStatus.LITERALSincludesPENDING,COMPLETED, andSKIPPED. If a caller passesTaskStatus.PENDINGorTaskStatus.SKIPPED, the code falls into theelsebranch which filters fordereferenced_performers.is_completed IS TRUE. This is semantically incorrect for pending and skipped tasks, which are not completed. [ Already posted ]backend/src/processes/views/template.py — 0 comments posted, 3 evaluated, 3 filtered
'titles'inget_permissions()at line 154 was not updated to'titles_by_workflows'when the action was renamed. This causes thetitles_by_workflowsaction to fall through to the default permissions at lines 180-186, which incorrectly requireUserIsAdminOrAccountOwner()permission instead of the intended basic permissions (UserIsAuthenticated,ExpiredSubscriptionPermission,BillingPlanPermission). [ Already posted ]get_permissions()method still checks for'titles'at line 154, but the action was renamed fromtitlestotitles_by_workflows. This means when thetitles_by_workflowsendpoint is accessed,self.actionwill be'titles_by_workflows'which won't match'titles'in the tuple, causing the permission check to fall through to the default case (lines 180-186) which incorrectly requiresUserIsAdminOrAccountOwner()permission instead of the intended less restrictive permissions. [ Already posted ]titlestotitles_by_workflows, but the permissions check inget_permissions()at line 154 still references'titles'instead of'titles_by_workflows'. This means thetitles_by_workflowsendpoint will fall through to the default permissions (lines 180-186) which requireUserIsAdminOrAccountOwner(), potentially blocking regular users who should have access to this endpoint. [ Already posted ]frontend/src/public/api/getTemplatesTitlesByTasks.ts — 0 comments posted, 1 evaluated, 1 filtered
getTemplatesTitlesByTasksrelies onurls.templatesTitlesByTasks, but there is no evidence in the provided code (specificallyfrontend/src/public/utils/getConfig.tsorcommonRequest.ts'sgetBrowserConfigEnv) that this new key exists in the configuration object returned bygetBrowserConfigEnv(). Accessing a property on an undefined configuration object or accessing a missing key that results inundefinedbeing passed tocommonRequestcould lead to constructing an invalid URL (e.g.,undefined?status=active). [ Already posted ]frontend/src/public/components/Highlights/TemplatesFilter.tsx — 0 comments posted, 2 evaluated, 1 filtered
truncatedTemplatesuseMemohook triggers a state update (handleShowAllcallssetShowAllVisibleState) at line 77 during the component's render phase. Triggering side-effects (like state updates) directly within the render body oruseMemoviolates React's purity contract. This can cause 'Too many re-renders' errors or inconsistent state if the conditional logicisSelectedWorkflowsHidden(which was modified in this diff) fails to stabilize immediately. [ Out of scope ]frontend/src/public/components/Tasks/container.ts — 0 comments posted, 2 evaluated, 2 filtered
taskApiNameFilterproperty and theSyncedTasksconfiguration. ThegetQueryParamByPropfortaskApiNameFilteris set to theStringconstructor. SincetaskApiNameFilteris nullable (defaulting tonull),String(null)produces the string"null", which is then serialized to the URL (e.g.,&template-task=null). When reading this back, thecreateActionlogic checksif (taskApiNAme)which evaluates the string"null"as truthy, resulting insetFilterStep("null")being called. This causes the task list to filter by the literal string API name"null"instead of applying no filter. This contrasts withtemplateIdFilterwhereNumber("null")results inNaNand is handled safely. [ Already posted ]container.ts, thewithSyncedQueryStringconfiguration fortaskApiNameFilterusesStringas the serializer ingetQueryParamByProp. WhentaskApiNameFilterisnull(indicating no filter),String(null)returns"null", causing?template-task=nullto be written to the URL. [ Already posted ]frontend/src/public/components/TuneViewModal/TuneViewModal.tsx — 0 comments posted, 1 evaluated, 1 filtered
TuneViewModal,templateTaskscan be undefined ifgetTemplatesTasksreturns undefined for the giventemplateId(e.g., during loading or if invalid). While optional chaining (?.) is used in theuseEffect, it is NOT used when accessingtemplateTasksinsidehandleApplyChanges. IfhandleApplyChangesis triggered (e.g. by user clicking 'Apply') whiletemplateTasksis still undefined,templateTasks.forEachwill throwTypeError: Cannot read properties of undefined (reading 'forEach'). [ Out of scope ]frontend/src/public/components/Workflows/WorkflowModal/WorkflowModal.tsx — 0 comments posted, 1 evaluated, 1 filtered
componentDidUpdate, the code previously accessedthis.props.workflow?.kickoff.idusing optional chaining, guarding against cases wherekickoffmight be undefined on the workflow object (a case explicitly handled inrenderKickoffwithif (!initialKickoff ...)). The new implementation destructureskickofffromworkflowand accesseskickoff.iddirectly without a check. If a workflow is loaded that is missing thekickoffobject (e.g. legacy data or partial API response), accessingkickoff.idwill throw a runtime error ('Cannot read property id of undefined'). [ Already posted ]