FWF-4820: [BugFix] Reviewer journey latest

[Bonymol-aot]

User description

Issue Tracking

JIRA: https://aottech.atlassian.net/browse/FWF-4820
Issue Type: BUG/ FEATURE

Changes

• Task list issue while changing attribute filter fixed
• Added 'All Fields' option and its edit always to attribute filter dropdown
• Page limit changed from 15 --> 25
• Removed 'No filters found' text from attribute filter dropdown
• No tasks found message css change

---

PR Type

Bug fix, Enhancement

---

Description

Fix attribute filter modal effect and share options
Enhance attribute filter save and list update
Improve attribute filter dropdown items and actions
Adjust task list defaults and table behavior

• Increase page limit to 25
• Add roles column support

---

Changes walkthrough 📝

	Relevant files
Enhancement	5 files AttributeFIlterModalBody.tsx Fix filter effect dependency and include metadata                +6/-1      AttributeFilterModal.tsx Update attribute filter list after save                                    +3/-0      AttributeFilterDropdown.tsx Improve dropdown items and edit actions                                    +10/-13  TaskList.tsx Refine default filter selection and page limit                      +20/-18  TasklistTable.tsx Add roles column support in table                                                +8/-0
Bug fix	1 files SaveFilterTab.tsx Adjust save filter tab display condition                                  +1/-1
Formatting	1 files _table.scss Change last column text alignment                                                +1/-1

---

Need help?

Type /help how to ... in the comments thread for any questions about PR-Agent usage.

Check out the documentation for more information.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 8341f97)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review useEffect Dependency The dependency array of useEffect was changed to [attributeFilter], removing shareAttrFilter and selectedFilter, which may cause the effect to miss updates or use stale values. if(selectedAttributeFilter?.roles.length > 0 || (selectedAttributeFilter?.users.length == 0 && selectedAttributeFilter.roles.length == 0)){ setShareAttrFilter(FILTER_SHARE_OPTIONS.SAME_AS_TASKS); } getTaskAccess(); }, [attributeFilter]); Save Button Logic The new conditional only shows the save button for "All Tasks" when creating filters, removing the branch for other filters and potentially hiding the button for edits. if (createFilters && (selectedFilter.name === "All Tasks" && !filterToEdit)) { return ( <div className="pt-4"> <CustomButton size="md" variant={saveAndUpdateButtonVariant} onClick={handleSaveCurrentFilter} icon={ Default Filter Check The comparison defaultFilterId !== filters.find(...) compares an ID to a filter object (not its id), which is always true and forces unintended default filter resets. if (defaultFilterId !== filters.find((f) => f.id === defaultFilterId)?.id) { const newFilter = filters.find(f => f.name === "All Tasks") || filters[0]; dispatch(setDefaultFilter(newFilter.id)); updateDefaultFilter(newFilter.id); } }

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 8341f97

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Correct save-tab condition The original check prevented saving on non-"All Tasks" filters; the new AND logic is inverted. Change the condition to require a non-"All Tasks" selection when creating and no edit target. forms-flow-review/src/components/TaskFilterModal/SaveFilterTab.tsx [215] -if (createFilters && (selectedFilter.name === "All Tasks" && !filterToEdit)) { +if (createFilters && selectedFilter.name !== "All Tasks" && !filterToEdit) { return ( <div className="pt-4"> <CustomButton size="md" ... Suggestion importance[1-10]: 8 __ Why: The current condition only allows saving on "All Tasks" and blocks valid filters; adjusting the logic fixes the save button visibility for non-default filters.	Medium
	Handle missing default ID If defaultFilterId is falsy, the check against find returns false and skips setting a default. Expand the condition to also trigger when defaultFilterId is not set or not found in filters. forms-flow-review/src/components/TaskList/TaskList.tsx [83-87] -if (defaultFilterId !== filters.find((f) => f.id === defaultFilterId)?.id) { +if (!defaultFilterId || !filters.find((f) => f.id === defaultFilterId)) { const newFilter = filters.find(f => f.name === "All Tasks") || filters[0]; dispatch(setDefaultFilter(newFilter.id)); updateDefaultFilter(newFilter.id); } Suggestion importance[1-10]: 7 __ Why: If defaultFilterId is falsy or not present in filters, the block is skipped; expanding the condition ensures a fallback filter is always selected.	Medium
General	Fix effect dependencies The effect depends on both the selected attribute filter and the share option, not just the filter object. Update the dependency array to include both selectedAttributeFilter and shareAttrFilter so it re-runs correctly. forms-flow-review/src/components/AttributeFilterModal/AttributeFIlterModalBody.tsx [173-180] useEffect(() => { if (selectedAttributeFilter?.roles.length > 0 || (selectedAttributeFilter?.users.length == 0 && selectedAttributeFilter.roles.length == 0)) { setShareAttrFilter(FILTER_SHARE_OPTIONS.SAME_AS_TASKS); } getTaskAccess(); -}, [attributeFilter]); +}, [selectedAttributeFilter, shareAttrFilter]); Suggestion importance[1-10]: 7 __ Why: The effect uses selectedAttributeFilter and shareAttrFilter but only depends on attributeFilter, which may cause stale or missing updates when those change.	Medium
	Guard metadata on create When creating a new filter, attributeFilter may be undefined and introduce empty metadata fields. Conditionally spread these ID/metadata properties only if attributeFilter exists. forms-flow-review/src/components/AttributeFilterModal/AttributeFIlterModalBody.tsx [239-245] return { name: filterName, criteria, - id: attributeFilter?.id, - created: attributeFilter?.created, - modified: attributeFilter?.modified, - createdBy:attributeFilter?.createdBy, - modifiedBy:attributeFilter?.modifiedBy, + ...(attributeFilter && { + id: attributeFilter.id, + created: attributeFilter.created, + modified: attributeFilter.modified, + createdBy: attributeFilter.createdBy, + modifiedBy: attributeFilter.modifiedBy, + }), parentFilterId: selectedFilter.id, roles, users, variables: selectedFilter.variables, }; Suggestion importance[1-10]: 6 __ Why: Spreading metadata fields without checking attributeFilter can introduce undefined properties; conditional spreading prevents empty values.	Low

---

Previous suggestions

Suggestions up to commit 5d72c4c

Category	Suggestion	Impact
Possible issue	Fix save-button display logic The save-button visibility logic was inverted by the new condition. It should allow saving when the selected filter is not the default "All Tasks", or when editing an existing filter. forms-flow-review/src/components/TaskFilterModal/SaveFilterTab.tsx [215] -if (createFilters && (selectedFilter.name === "All Tasks" && !filterToEdit)) { +if (createFilters && (selectedFilter.name !== "All Tasks" || filterToEdit)) { Suggestion importance[1-10]: 9 __ Why: The PR inverted the save-button conditional, breaking core functionality; correcting this logic is critical for the save button to display under the intended conditions.	High
General	Replace magic number with constant Pull the hard-coded page size 25 into a named constant (e.g. DEFAULT_PAGE_LIMIT) to keep limit values consistent and easier to adjust. forms-flow-review/src/components/TaskList/TaskList.tsx [209-210] -dispatch(setTaskListLimit(25)); -dispatch(fetchServiceTaskList(currentFilter, null, 1, 25)); +const DEFAULT_PAGE_LIMIT = 25; +dispatch(setTaskListLimit(DEFAULT_PAGE_LIMIT)); +dispatch(fetchServiceTaskList(currentFilter, null, 1, DEFAULT_PAGE_LIMIT)); Suggestion importance[1-10]: 5 __ Why: Extracting the literal 25 into a named constant improves maintainability but is a minor refactor without functional impact.	Low
	Fix indentation consistency Align the indentation to match surrounding code and avoid mixing spaces; use four spaces for the if block to maintain consistency and readability. forms-flow-review/src/components/AttributeFilterModal/AttributeFilterDropdown.tsx [64-67] - if (processVariables && processVariables.length > 0) { - currentCriteria.processVariables = currentCriteria.processVariables || []; - currentCriteria.processVariables.push(...processVariables); -} + if (processVariables && processVariables.length > 0) { + currentCriteria.processVariables = currentCriteria.processVariables || []; + currentCriteria.processVariables.push(...processVariables); + } Suggestion importance[1-10]: 2 __ Why: Aligning spaces for indentation is purely stylistic and offers minimal impact on functionality or maintainability.	Low

Suggestions up to commit 52e9619

Category	Suggestion	Impact
Possible issue	Add error handling around update Wrap the async update call in a try/catch block to handle potential API errors. This prevents unhandled promise rejections and gives you a chance to revert UI state if the update fails. forms-flow-review/src/components/AttributeFilterModal/AttributeFilterModal.tsx [53-64] const handleSaveFilterAttributes = async () => { toggleUpdateModal(); - const response = await updateFilter( - attributeFilterToEdit, - attributeFilterToEdit?.id - ); - setUpdateSuccess(onClose, 2); - const filterList = attributeFilterList.filter((item) => item.id !== response.data.id); - dispatch(setSelectedBpmAttributeFilter(response.data)); - const newAttributeFilterList = [response.data, ...filterList]; - dispatch(setAttributeFilterList(newAttributeFilterList)); - dispatch(fetchServiceTaskList(response.data, null, 1, limit)); + try { + const response = await updateFilter(attributeFilterToEdit, attributeFilterToEdit?.id); + setUpdateSuccess(onClose, 2); + const filterList = attributeFilterList.filter((item) => item.id !== response.data.id); + dispatch(setSelectedBpmAttributeFilter(response.data)); + dispatch(setAttributeFilterList([response.data, ...filterList])); + dispatch(fetchServiceTaskList(response.data, null, 1, limit)); + } catch (error) { + console.error('Failed to update filter', error); + // optionally notify user or revert modal state + } }; Suggestion importance[1-10]: 7 __ Why: Wrapping the async update in a try/catch prevents unhandled promise rejections and lets the UI gracefully handle API failures.	Medium
General	Display all candidate group IDs If multiple candidate groups exist, join their IDs into a single string. This gives a fuller view rather than only showing the first group. forms-flow-review/src/components/TaskList/TasklistTable.tsx [65-67] case "roles": { - return candidateGroups.length > 0 ? candidateGroups[0]?.groupId ?? "-" : "-"; + return candidateGroups.length + ? candidateGroups.map((g) => g.groupId).join(", ") + : "-"; } Suggestion importance[1-10]: 6 __ Why: Joining all candidateGroups IDs provides a more complete view of roles rather than only the first group.	Low
	Add fallback defaults for optional fields Ensure these optional fields always have a consistent type by providing an explicit fallback. This avoids sending undefined to downstream code or APIs that expect null. forms-flow-review/src/components/AttributeFilterModal/AttributeFIlterModalBody.tsx [241-245] -id: attributeFilter?.id, -created: attributeFilter?.created, -modified: attributeFilter?.modified, -createdBy:attributeFilter?.createdBy, -modifiedBy:attributeFilter?.modifiedBy, +id: attributeFilter?.id ?? null, +created: attributeFilter?.created ?? null, +modified: attributeFilter?.modified ?? null, +createdBy: attributeFilter?.createdBy ?? null, +modifiedBy: attributeFilter?.modifiedBy ?? null, Suggestion importance[1-10]: 5 __ Why: The fallback ?? null ensures consistent non-undefined values for optional filter fields and prevents potential API mismatches.	Low
	Use immutable array concatenation Avoid mutating the existing array by creating a new array. This preserves immutability and prevents side effects if currentCriteria is reused elsewhere. forms-flow-review/src/components/AttributeFilterModal/AttributeFilterDropdown.tsx [64-67] if (processVariables && processVariables.length > 0) { - currentCriteria.processVariables = currentCriteria.processVariables || []; - currentCriteria.processVariables.push(...processVariables); + currentCriteria.processVariables = [ + ...(currentCriteria.processVariables || []), + ...processVariables + ]; } Suggestion importance[1-10]: 5 __ Why: Creating a new array instead of mutating currentCriteria.processVariables enhances immutability and avoids side effects.	Low

Suggestions up to commit 29baf88

Category	Suggestion	Impact
General	Add error handling block Wrap the updateFilter call in a try/catch block so that the modal state is correctly reset and errors can be handled gracefully if the API call fails. forms-flow-review/src/components/AttributeFilterModal/AttributeFilterModal.tsx [53-57] toggleUpdateModal(); -const response = await updateFilter( - attributeFilterToEdit, - attributeFilterToEdit?.id -); +let response; +try { + response = await updateFilter( + attributeFilterToEdit, + attributeFilterToEdit.id + ); +} catch (error) { + toggleUpdateModal(); + // TODO: show error notification + return; +} Suggestion importance[1-10]: 6 __ Why: Wrapping the updateFilter call in try/catch ensures proper UI state reset and error feedback, improving robustness with moderate impact.	Low
	Omit undefined metadata fields Only include these filter metadata fields when they are actually defined to avoid sending undefined values in the payload. You can use conditional spreads so that missing properties are omitted entirely. forms-flow-review/src/components/AttributeFilterModal/AttributeFIlterModalBody.tsx [241-245] -id: attributeFilter?.id, -created: attributeFilter?.created, -modified: attributeFilter?.modified, -createdBy:attributeFilter?.createdBy, -modifiedBy:attributeFilter?.modifiedBy, +...(attributeFilter?.id && { id: attributeFilter.id }), +...(attributeFilter?.created && { created: attributeFilter.created }), +...(attributeFilter?.modified && { modified: attributeFilter.modified }), +...(attributeFilter?.createdBy && { createdBy: attributeFilter.createdBy }), +...(attributeFilter?.modifiedBy && { modifiedBy: attributeFilter.modifiedBy }), Suggestion importance[1-10]: 5 __ Why: Using conditional spreads to omit undefined properties cleans up the payload and avoids sending unwanted undefined values, improving maintainability without altering core logic.	Low
Possible issue	Default attribute list fallback Guard against attributeFilterList being null or undefined before calling filter to prevent runtime errors. Default to an empty array if it's not set. forms-flow-review/src/components/AttributeFilterModal/AttributeFilterModal.tsx [60] -const filterList = attributeFilterList.filter((item) => item.id !== response.data.id); +const filterList = (attributeFilterList || []).filter((item) => item.id !== response.data.id); Suggestion importance[1-10]: 3 __ Why: Guarding attributeFilterList against null or undefined adds safety but is low impact since the selector likely always provides an array.	Low

[github-actions]

Persistent review updated to latest commit 52e9619

[github-actions]

Persistent review updated to latest commit 5d72c4c

[sonarqubecloud]

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[github-actions]

Persistent review updated to latest commit 8341f97