fixed the sorting key having same key

[abilpraju-aot]

User description

Issue Tracking

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

Changes

---

PR Type

Bug fix

---

Description

• Fix sorting when keys duplicate

• Persist actual sort key in state

• Disambiguate headers with unique keys

• Prevent checkbox id collisions

---

Diagram Walkthrough

flowchart LR
  UI["Table headers & Sort modal"] -- "select/trigger sort" --> State["filterListSortParams with actualSortKey + isFormVariable"]
  State -- "build request" --> Helper["createReqPayload uses actualSortKey"]
  Helper -- "correct sort params" --> API["Task list API"]
  UI -- "unique ids/keys" --> Accessibility["Unique checkbox ids & header keys"]

Loading

File Walkthrough

Relevant files

Bug fix

DragandDropSort.tsx Unique checkbox ids to prevent collisions                                forms-flow-components/src/components/CustomComponents/DragandDropSort.tsx Add unique checkbox ids for form/static fields Update htmlFor/id to avoid collisions Minor formatting and indentation fixes +9/-4      TaskList.tsx Store and use actualSortKey; robust sort options                  forms-flow-review/src/components/TaskList/TaskList.tsx Track actualSortKey and isFormVariable in state Resolve selected sort option from modal options Deduplicate and label dynamic sort options Use actualSortKey when determining sort behavior +106/-26 TasklistTable.tsx Unique sortable keys and persisted actualSortKey                  forms-flow-review/src/components/TaskList/TasklistTable.tsx Use unique column keys with _form suffix Toggle sort using unique keys Persist actualSortKey for API calls Update SortableHeader props and test ids +16/-6    taskHelper.ts Build payload with actualSortKey to avoid clashes                forms-flow-review/src/helper/taskHelper.ts Use actualSortKey for sortBy/parameters.variable Respect non-form enabled sort fields Keep existing type mapping logic +6/-3

---

[github-actions]

PR Reviewer Guide 🔍

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 Type Handling Ensure that the logic determining isFormVariable aligns with backend expectations; current fallback uses enabledSort to infer isFormVariable which may misclassify non-form variables sharing keys, affecting payload type resolution. const actualSortKey = filterListSortParams?.actualSortKey || filterListSortParams?.activeKey; // Check if it's explicitly set as a form variable or is in the enabled sort set const isFormVariable = filterListSortParams?.isFormVariable !== undefined ? filterListSortParams.isFormVariable : enabledSort.has(actualSortKey); if (filterCached) { State Consistency The new unique column keys (e.g., key_form) must be fully compatible with SortableHeader and any selectors; verify no consumers assume original sortKey names for lookup or test IDs. // Create a unique key for the column that includes isFormVariable to differentiate between columns with same sortKey const uniqueColumnKey = column.isFormVariable ? `${column.sortKey}_form` : column.sortKey; return ( <SortableHeader className="header-sortable" columnKey={uniqueColumnKey} title={t(column.name)} currentSort={filterListSortParams} handleSort={() => handleSort(column)} dataTestId={`sort-header-${uniqueColumnKey}`} ariaLabel={t("Sort by {{columnName}}", { ID Uniqueness The checkbox id/htmlFor prefixes "static-" depend on isFormVariable; confirm item.id/name cannot contain spaces or characters invalid in HTML id attributes to avoid selector/accessibility issues. <label htmlFor={`${!(item.isFormVariable) ? "static-" + (item.id || item.name) : item.id || item.name }-checkbox-id`} className="input-checkbox"> <input id={`${!(item.isFormVariable) ? "static-" + (item.id || item.name) : item.id || item.name }-checkbox-id`} type="checkbox" checked={item.isChecked} onChange={() => onCheckboxChange(index)} disabled={preventLastCheck && item.isChecked && filterItems.filter(i => i.isChecked).length === 1} data-testid={`${item.name}-checkbox`} /> <span>{item.label ?? item.name}</span> {item.isChecked ? <CheckboxCheckedIcon /> : <CheckboxUncheckedIcon /> }

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Guard sort toggling per column Preserve previous sort order only for the same active column; otherwise default to "asc". Without this guard, toggling a different column could incorrectly flip order based on unrelated state. forms-flow-review/src/components/TaskList/TasklistTable.tsx [255-269] const uniqueColumnKey = column.isFormVariable ? `${column.sortKey}_form` : column.sortKey; +const prevIsSameColumn = filterListSortParams?.activeKey === uniqueColumnKey; +const nextOrder = prevIsSameColumn && filterListSortParams?.[uniqueColumnKey]?.sortOrder === "asc" ? "desc" : "asc"; const updatedFilterListSortParams = { ...resetSortOrders, [uniqueColumnKey]: { - sortOrder: - filterListSortParams[uniqueColumnKey]?.sortOrder === "asc" ? "desc" : "asc", - ...((column.isFormVariable || enabledSort.has(column.sortKey)) && { - type: column.type, - }) + sortOrder: nextOrder, + ...((column.isFormVariable || enabledSort.has(column.sortKey)) && { + type: column.type, + }) }, activeKey: uniqueColumnKey, - // Store the actual sortKey to use in API requests actualSortKey: column.sortKey, }; Suggestion importance[1-10]: 7 __ Why: Ensuring the sort order toggles only when the same column is re-selected avoids unintended flips, improving UX correctness. It fits the updated unique key approach and is a meaningful but moderate improvement.	Medium
	Add safe fallback on missing option Avoid silently failing when a sort option is not found. Surface a user-visible fallback by resetting to a safe default sort and proceed, to prevent the UI from getting stuck and keep requests consistent. forms-flow-review/src/components/TaskList/TaskList.tsx [176-181] const selectedOption = allSortOptions.find(option => option.value === selectedSortOption); if (!selectedOption) { console.error("Selected sort option not found:", selectedSortOption); + const defaultOption = optionSortBy.options?.[0]; + if (!defaultOption) return; + const updatedData = { + ...HelperServices.getResetSortOrders(optionSortBy.options), + activeKey: defaultOption.value, + [defaultOption.value]: { sortOrder: "asc" }, + actualSortKey: defaultOption.value, + isFormVariable: false + }; + dispatch(setFilterListSortParams(updatedData)); + setShowSortModal(false); + fetchTaskListData({ sortData: updatedData }); return; } Suggestion importance[1-10]: 6 __ Why: Adding a user-visible fallback prevents a no-op return and keeps sorting functional, improving robustness. The change aligns with the new logic and is low-risk, though not critical.	Low
	Fix form-variable detection logic Ensure isFormVariable reflects only form variables. Falling back to enabledSort.has(actualSortKey) misclassifies static fields as form variables, causing wrong API payload shape. forms-flow-review/src/components/TaskList/TaskList.tsx [133-135] -const isFormVariable = filterListSortParams?.isFormVariable !== undefined - ? filterListSortParams.isFormVariable - : enabledSort.has(actualSortKey); +const isFormVariable = filterListSortParams?.isFormVariable === true + ? true + : false; Suggestion importance[1-10]: 3 __ Why: The concern is valid that static fields shouldn't be flagged as form variables, but forcing a strict boolean true/false drops the useful fallback and may break scenarios where isFormVariable isn't provided. Improvement is debatable and potentially over-restrictive.	Low

[arun-s-aot]

This PR needs some extensive testing before merging. Will take some time