fantonangeli commented on code in PR #3183:
URL: 
https://github.com/apache/incubator-kie-tools/pull/3183#discussion_r2152247857


##########
packages/runtime-tools-swf-enveloped-components/src/workflowList/envelope/components/WorkflowListTable/WorkflowListTable.tsx:
##########
@@ -90,6 +90,38 @@ const WorkflowListTable: React.FC<WorkflowListTableProps & 
OUIAProps> = ({
   const [titleType, setTitleType] = useState<string>("");
   const [isModalOpen, setIsModalOpen] = useState<boolean>(false);
   const [selectedWorkflowInstance, setSelectedWorkflowInstance] = 
useState<WorkflowInstance | null>(null);
+  const [sortByState, setSortByState] = useState<{ index: number; direction: 
"asc" | "desc" }>({
+    index: 2,
+    direction: "asc",
+  });
+
+  const getComparableValue = (instance: WorkflowInstance, columnKey: string): 
any => {

Review Comment:
   it's always better to use `useCallback` to reduce re-rendering of components



##########
packages/runtime-tools-swf-enveloped-components/src/workflowList/envelope/components/WorkflowListTable/WorkflowListTable.tsx:
##########


Review Comment:
   IDKW but in this file at line 22 there is a change from `Table` to 
`TableComposable` which GH doesn't show:
   upstream/main:
   ```
   import { Table } from "@patternfly/react-table/deprecated";
   ```
   kumaradityaraj/sortWorkflow:
   ```
   import { TableComposable, Thead, Tbody, Tr, Th, Td } from 
"@patternfly/react-table/dist/js/components/TableComposable";
   ```
   you can see it running:
   ```
   git difftool -y upstream/main 
packages/runtime-tools-swf-enveloped-components/src/workflowList/envelope/components/WorkflowListTable/WorkflowListTable.tsx
   ```
   
   
https://github.com/apache/incubator-kie-tools/blob/main/packages/runtime-tools-swf-enveloped-components/src/workflowList/envelope/components/WorkflowListTable/WorkflowListTable.tsx#L23
   
   
https://github.com/kumaradityaraj/kie-tools/blob/bc8556ec487ffb47155a69dce34bbc43a8796a5c/packages/runtime-tools-swf-enveloped-components/src/workflowList/envelope/components/WorkflowListTable/WorkflowListTable.tsx#L22
   
   Was this intentional?



##########
packages/runtime-tools-swf-enveloped-components/src/workflowList/envelope/components/WorkflowListTable/WorkflowListTable.tsx:
##########
@@ -90,6 +90,38 @@ const WorkflowListTable: React.FC<WorkflowListTableProps & 
OUIAProps> = ({
   const [titleType, setTitleType] = useState<string>("");
   const [isModalOpen, setIsModalOpen] = useState<boolean>(false);
   const [selectedWorkflowInstance, setSelectedWorkflowInstance] = 
useState<WorkflowInstance | null>(null);
+  const [sortByState, setSortByState] = useState<{ index: number; direction: 
"asc" | "desc" }>({
+    index: 2,
+    direction: "asc",
+  });
+
+  const getComparableValue = (instance: WorkflowInstance, columnKey: string): 
any => {
+    switch (columnKey) {
+      case "Id":
+        return instance.id;
+      case "Status":
+        return instance.state;
+      case "Created":
+        return new Date(instance.start);
+      case "Last update":
+        return new Date(instance.lastUpdate);
+      default:
+        return "";
+    }
+  };
+
+  const onSortInternal = (_event: React.SyntheticEvent, index: number, 
direction: "asc" | "desc") => {

Review Comment:
   it's always better to use `useCallback` to reduce re-rendering of components



##########
packages/runtime-tools-swf-enveloped-components/src/workflowList/envelope/components/WorkflowListTable/WorkflowListTable.tsx:
##########
@@ -90,6 +90,38 @@ const WorkflowListTable: React.FC<WorkflowListTableProps & 
OUIAProps> = ({
   const [titleType, setTitleType] = useState<string>("");
   const [isModalOpen, setIsModalOpen] = useState<boolean>(false);
   const [selectedWorkflowInstance, setSelectedWorkflowInstance] = 
useState<WorkflowInstance | null>(null);
+  const [sortByState, setSortByState] = useState<{ index: number; direction: 
"asc" | "desc" }>({
+    index: 2,
+    direction: "asc",
+  });
+
+  const getComparableValue = (instance: WorkflowInstance, columnKey: string): 
any => {
+    switch (columnKey) {
+      case "Id":
+        return instance.id;
+      case "Status":
+        return instance.state;
+      case "Created":
+        return new Date(instance.start);
+      case "Last update":
+        return new Date(instance.lastUpdate);
+      default:
+        return "";
+    }
+  };
+
+  const onSortInternal = (_event: React.SyntheticEvent, index: number, 
direction: "asc" | "desc") => {

Review Comment:
   > This will sort the already-fetched items, but if the Data Index is 
returning a "paginated" list of items, this sort is not necessarily correct.
   > 
   > @fantonangeli , can the Data-Index sort by ID?
   
   @wmedvede @ricardozanini @domhanak 
   It seems that the Data Index does not support sorting by `id`, even though 
it's documented  
[here](https://sonataflow.org/serverlessworkflow/latest/data-index/data-index-core-concepts.html).
   I tried using the following query variables:
   ```
   {
     "offset": 0,
     "limit": 10,
     "orderBy": {
       "id": "ASC"
     }
   }
   ```
   Response: 
   ```
   GraphQLError: Variable \"$orderBy\" got invalid value { id: \"ASC\" }; Field 
\"id\" is not defined by type ProcessInstanceOrderBy.
   ```
   IMO we have a couple of ways:
   - Fix the Data Index to support sorting by `id`
   - Sort by processName instead and update the column label in the UI from 
`id` to `Name`, since that column displays both the process name and ID.
   
   Let me know what you think, or if I'm missing something in the query.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to