You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "smitajoshi12 (via GitHub)" <gi...@apache.org> on 2023/08/23 18:07:05 UTC

[GitHub] [ozone] smitajoshi12 opened a new pull request, #5213: HDDS-9161. Recon Pipelines datanode columns search does not work.

smitajoshi12 opened a new pull request, #5213:
URL: https://github.com/apache/ozone/pull/5213

   ## What changes were proposed in this pull request?
   There are values present in the column, but the search does not filter them out. so Now search Functionality is working on Array and Objects
   
   ## What is the link to the Apache JIRA
   https://issues.apache.org/jira/browse/HDDS-9161
   
   ## How was this patch tested?
   Manually
   
   ![image](https://github.com/apache/ozone/assets/112169209/e88f45c1-bbd1-45cc-8409-d5dcff5d6235)
   
   


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] devabhishekpal commented on a diff in pull request #5213: HDDS-9161. Recon Pipelines datanode columns search does not work

Posted by "devabhishekpal (via GitHub)" <gi...@apache.org>.
devabhishekpal commented on code in PR #5213:
URL: https://github.com/apache/ozone/pull/5213#discussion_r1303406766


##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/utils/columnSearch.tsx:
##########
@@ -69,8 +69,15 @@ class ColumnSearch extends React.PureComponent {
     filterIcon: (filtered: boolean) => (
       <Icon type='search' style={{color: filtered ? '#1890ff' : undefined}}/>
     ),
-    onFilter: (value: string, record: any) =>
-      record[dataIndex].toString().toLowerCase().includes(value.toLowerCase()),
+    onFilter: (value: string, record: any) => {
+        if (record[dataIndex]) {
+          return typeof (record[dataIndex]) == 'object' ? Object.values(...record[dataIndex]).toString().toLowerCase().includes(value.toLowerCase())

Review Comment:
   Since object can have many different meanings, just to be on a safer side can we do a stricter typecheck?
   Something like:
   `typeof(record[dataIndex]) === typeof({})`



##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/utils/columnSearch.tsx:
##########
@@ -69,8 +69,15 @@ class ColumnSearch extends React.PureComponent {
     filterIcon: (filtered: boolean) => (
       <Icon type='search' style={{color: filtered ? '#1890ff' : undefined}}/>
     ),
-    onFilter: (value: string, record: any) =>
-      record[dataIndex].toString().toLowerCase().includes(value.toLowerCase()),
+    onFilter: (value: string, record: any) => {
+        if (record[dataIndex]) {
+          return typeof (record[dataIndex]) == 'object' ? Object.values(...record[dataIndex]).toString().toLowerCase().includes(value.toLowerCase())

Review Comment:
   Can we shorten the lines to keep at par with the code-style/readability?
   ```
   return typeof (record[dataIndex] == 'object'
       ? Object.values(...record[dataIndex]).toString().toLowerCase().includes(value.toLowerCase())
       : record[dataIndex].toString().toLowerCase().includes(value.toLowerCase())
    ```



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] dombizita commented on pull request #5213: HDDS-9161. Recon Pipelines datanode columns search does not work

Posted by "dombizita (via GitHub)" <gi...@apache.org>.
dombizita commented on PR #5213:
URL: https://github.com/apache/ozone/pull/5213#issuecomment-1697035807

   please review it again, if you have time @devabhishekpal 


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] smitajoshi12 commented on a diff in pull request #5213: HDDS-9161. Recon Pipelines datanode columns search does not work

Posted by "smitajoshi12 (via GitHub)" <gi...@apache.org>.
smitajoshi12 commented on code in PR #5213:
URL: https://github.com/apache/ozone/pull/5213#discussion_r1312042785


##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/utils/columnSearch.tsx:
##########
@@ -69,8 +69,16 @@ class ColumnSearch extends React.PureComponent {
     filterIcon: (filtered: boolean) => (
       <Icon type='search' style={{color: filtered ? '#1890ff' : undefined}}/>
     ),
-    onFilter: (value: string, record: any) =>
-      record[dataIndex].toString().toLowerCase().includes(value.toLowerCase()),
+    onFilter: (value: string, record: any) => {
+      if (record[dataIndex] !== undefined || record[dataIndex] !== null) {
+          return typeof (record[dataIndex]) === typeof {}
+            ? Boolean (record[dataIndex].find(item => Object.values(item).toString().toLowerCase().includes(value.toLowerCase())) ? true :false)
+            : record[dataIndex].toString().toLowerCase().includes(value.toLowerCase())
+        }
+        else {

Review Comment:
   @devmadhuu 
   Boolean (record[dataIndex].find(item => Object.values(item).toString().toLowerCase().includes(value.toLowerCase())))
   Added short code for array search.



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] smitajoshi12 commented on a diff in pull request #5213: HDDS-9161. Recon Pipelines datanode columns search does not work

Posted by "smitajoshi12 (via GitHub)" <gi...@apache.org>.
smitajoshi12 commented on code in PR #5213:
URL: https://github.com/apache/ozone/pull/5213#discussion_r1303971618


##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/utils/columnSearch.tsx:
##########
@@ -69,8 +69,15 @@ class ColumnSearch extends React.PureComponent {
     filterIcon: (filtered: boolean) => (
       <Icon type='search' style={{color: filtered ? '#1890ff' : undefined}}/>
     ),
-    onFilter: (value: string, record: any) =>
-      record[dataIndex].toString().toLowerCase().includes(value.toLowerCase()),
+    onFilter: (value: string, record: any) => {
+        if (record[dataIndex]) {
+          return typeof (record[dataIndex]) == 'object' ? Object.values(...record[dataIndex]).toString().toLowerCase().includes(value.toLowerCase())

Review Comment:
   @devabhishekpal 
   Updated with latest code



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] devabhishekpal commented on a diff in pull request #5213: HDDS-9161. Recon Pipelines datanode columns search does not work

Posted by "devabhishekpal (via GitHub)" <gi...@apache.org>.
devabhishekpal commented on code in PR #5213:
URL: https://github.com/apache/ozone/pull/5213#discussion_r1313725709


##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/utils/columnSearch.tsx:
##########
@@ -69,8 +69,16 @@ class ColumnSearch extends React.PureComponent {
     filterIcon: (filtered: boolean) => (
       <Icon type='search' style={{color: filtered ? '#1890ff' : undefined}}/>
     ),
-    onFilter: (value: string, record: any) =>
-      record[dataIndex].toString().toLowerCase().includes(value.toLowerCase()),
+    onFilter: (value: string, record: any) => {
+      if (record[dataIndex] !== undefined || record[dataIndex] !== null) {
+          return typeof (record[dataIndex]) === typeof {}
+            ? Boolean (record[dataIndex].find(item => Object.values(item).toString().toLowerCase().includes(value.toLowerCase())) ? true :false)
+            : record[dataIndex].toString().toLowerCase().includes(value.toLowerCase())
+        }
+        else {

Review Comment:
   I guess it is safer to have it in place, in case there is a scenario like the data-fetching for say DataNodes failed, so now record[DataIndex] is undefined. In that case if we try to perform a search in the column, it might give error.



##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/utils/columnSearch.tsx:
##########
@@ -69,8 +69,16 @@ class ColumnSearch extends React.PureComponent {
     filterIcon: (filtered: boolean) => (
       <Icon type='search' style={{color: filtered ? '#1890ff' : undefined}}/>
     ),
-    onFilter: (value: string, record: any) =>
-      record[dataIndex].toString().toLowerCase().includes(value.toLowerCase()),
+    onFilter: (value: string, record: any) => {
+      if (record[dataIndex] !== undefined || record[dataIndex] !== null) {
+          return typeof (record[dataIndex]) === typeof {}
+            ? Boolean (record[dataIndex].find(item => Object.values(item).toString().toLowerCase().includes(value.toLowerCase())) ? true :false)
+            : record[dataIndex].toString().toLowerCase().includes(value.toLowerCase())
+        }
+        else {

Review Comment:
   @devmadhuu I guess it is safer to have it in place, in case there is a scenario like the data-fetching for say DataNodes failed, so now record[DataIndex] is undefined. In that case if we try to perform a search in the column, it might give error.



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] smitajoshi12 commented on pull request #5213: HDDS-9161. Recon Pipelines datanode columns search does not work

Posted by "smitajoshi12 (via GitHub)" <gi...@apache.org>.
smitajoshi12 commented on PR #5213:
URL: https://github.com/apache/ozone/pull/5213#issuecomment-1695755723

   > thanks for working on this @smitajoshi12! I tested your patch in a docker cluster locally and when I filtered for a datanode (both in case of hostname or UUID) that is not the first datanode in the list at the RATIS (THREE) pipeline it only showed the RATIS (ONE) pipeline. please see the following screeshots:
   > 
   > 1. before filtering
   > 
   > <img alt="before_filter" width="1477" src="https://user-images.githubusercontent.com/50611074/263278424-2287808b-65e3-4f01-9d94-700933108472.png">
   > 2. after filtering for `ozone_datanode_3.ozone_default`
   > 
   > <img alt="after_filter" width="1475" src="https://user-images.githubusercontent.com/50611074/263278506-79a253b2-3995-42de-bbd2-7ef3b3df738c.png">
   
   @dombizita 
   I have uploaded video of Recent Changes which covers bug you found.
   Thanks Zita


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] devabhishekpal commented on a diff in pull request #5213: HDDS-9161. Recon Pipelines datanode columns search does not work

Posted by "devabhishekpal (via GitHub)" <gi...@apache.org>.
devabhishekpal commented on code in PR #5213:
URL: https://github.com/apache/ozone/pull/5213#discussion_r1311486729


##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/utils/columnSearch.tsx:
##########
@@ -69,8 +69,16 @@ class ColumnSearch extends React.PureComponent {
     filterIcon: (filtered: boolean) => (
       <Icon type='search' style={{color: filtered ? '#1890ff' : undefined}}/>
     ),
-    onFilter: (value: string, record: any) =>
-      record[dataIndex].toString().toLowerCase().includes(value.toLowerCase()),
+    onFilter: (value: string, record: any) => {
+      if (record[dataIndex] !== undefined || record[dataIndex] !== null) {
+          return typeof (record[dataIndex]) === typeof {}
+            ? Boolean (record[dataIndex].find(item => Object.values(item).toString().toLowerCase().includes(value.toLowerCase())) ? true :false)

Review Comment:
   Hey sorry my previous comment was not very clear, I meant can we do it as:
   ```suggestion
               ? Boolean (record[dataIndex].find(item => Object.values(item).toString().toLowerCase().includes(value.toLowerCase())))
   ```
   
   since Boolean() will actually evaluate the expression and return true/false as is required



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] dombizita commented on a diff in pull request #5213: HDDS-9161. Recon Pipelines datanode columns search does not work

Posted by "dombizita (via GitHub)" <gi...@apache.org>.
dombizita commented on code in PR #5213:
URL: https://github.com/apache/ozone/pull/5213#discussion_r1308441011


##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/utils/columnSearch.tsx:
##########
@@ -69,8 +69,16 @@ class ColumnSearch extends React.PureComponent {
     filterIcon: (filtered: boolean) => (
       <Icon type='search' style={{color: filtered ? '#1890ff' : undefined}}/>
     ),
-    onFilter: (value: string, record: any) =>
-      record[dataIndex].toString().toLowerCase().includes(value.toLowerCase()),
+    onFilter: (value: string, record: any) => {
+      if (record[dataIndex] !== undefined || record[dataIndex] !== null) {
+          return typeof (record[dataIndex]) === typeof {}
+            ? record[dataIndex].find(item => Object.values(item).toString().toLowerCase().includes(value.toLowerCase())) ? true :false

Review Comment:
   do I understand it correctly, that in this line there is an if condition based on the value of `record[dataIndex].find(item => Object.values(item).toString().toLowerCase().includes(value.toLowerCase()))` and we return `true` if it's true and `false` if it's false? 
   ```suggestion
               ? record[dataIndex].find(item => Object.values(item).toString().toLowerCase().includes(value.toLowerCase()))
   ```



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] dombizita commented on a diff in pull request #5213: HDDS-9161. Recon Pipelines datanode columns search does not work

Posted by "dombizita (via GitHub)" <gi...@apache.org>.
dombizita commented on code in PR #5213:
URL: https://github.com/apache/ozone/pull/5213#discussion_r1311296657


##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/utils/columnSearch.tsx:
##########
@@ -69,8 +69,16 @@ class ColumnSearch extends React.PureComponent {
     filterIcon: (filtered: boolean) => (
       <Icon type='search' style={{color: filtered ? '#1890ff' : undefined}}/>
     ),
-    onFilter: (value: string, record: any) =>
-      record[dataIndex].toString().toLowerCase().includes(value.toLowerCase()),
+    onFilter: (value: string, record: any) => {
+      if (record[dataIndex] !== undefined || record[dataIndex] !== null) {
+          return typeof (record[dataIndex]) === typeof {}
+            ? record[dataIndex].find(item => Object.values(item).toString().toLowerCase().includes(value.toLowerCase())) ? true :false

Review Comment:
   @smitajoshi12 could you please check my [comment](https://github.com/apache/ozone/pull/5213#discussion_r1308441011) above? I think that hasn't been addressed in your latest commit and I think there is an unnecessary if statement there, please correct me if I'm wrong. 



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] devabhishekpal commented on a diff in pull request #5213: HDDS-9161. Recon Pipelines datanode columns search does not work

Posted by "devabhishekpal (via GitHub)" <gi...@apache.org>.
devabhishekpal commented on code in PR #5213:
URL: https://github.com/apache/ozone/pull/5213#discussion_r1310961916


##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/utils/columnSearch.tsx:
##########
@@ -69,8 +69,16 @@ class ColumnSearch extends React.PureComponent {
     filterIcon: (filtered: boolean) => (
       <Icon type='search' style={{color: filtered ? '#1890ff' : undefined}}/>
     ),
-    onFilter: (value: string, record: any) =>
-      record[dataIndex].toString().toLowerCase().includes(value.toLowerCase()),
+    onFilter: (value: string, record: any) => {
+      if (record[dataIndex] !== undefined || record[dataIndex] !== null) {
+          return typeof (record[dataIndex]) === typeof {}
+            ? record[dataIndex].find(item => Object.values(item).toString().toLowerCase().includes(value.toLowerCase())) ? true :false

Review Comment:
   find() in JS will return the first matched element else undefined.
   What this statement might be doing is if find() returns a match then true, else if undefined, then false.
   
   But can we write this a but more cleaner @smitajoshi12 ?
   Something like `Boolean(record[dataIndex].find(.....))`



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] smitajoshi12 commented on a diff in pull request #5213: HDDS-9161. Recon Pipelines datanode columns search does not work

Posted by "smitajoshi12 (via GitHub)" <gi...@apache.org>.
smitajoshi12 commented on code in PR #5213:
URL: https://github.com/apache/ozone/pull/5213#discussion_r1311263080


##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/utils/columnSearch.tsx:
##########
@@ -69,8 +69,16 @@ class ColumnSearch extends React.PureComponent {
     filterIcon: (filtered: boolean) => (
       <Icon type='search' style={{color: filtered ? '#1890ff' : undefined}}/>
     ),
-    onFilter: (value: string, record: any) =>
-      record[dataIndex].toString().toLowerCase().includes(value.toLowerCase()),
+    onFilter: (value: string, record: any) => {
+      if (record[dataIndex] !== undefined || record[dataIndex] !== null) {
+          return typeof (record[dataIndex]) === typeof {}
+            ? record[dataIndex].find(item => Object.values(item).toString().toLowerCase().includes(value.toLowerCase())) ? true :false

Review Comment:
   @devabhishekpal 
   Abhishek Completed change in recent commit as per your suggestion.



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] smitajoshi12 commented on a diff in pull request #5213: HDDS-9161. Recon Pipelines datanode columns search does not work

Posted by "smitajoshi12 (via GitHub)" <gi...@apache.org>.
smitajoshi12 commented on code in PR #5213:
URL: https://github.com/apache/ozone/pull/5213#discussion_r1312044428


##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/utils/columnSearch.tsx:
##########
@@ -69,8 +69,16 @@ class ColumnSearch extends React.PureComponent {
     filterIcon: (filtered: boolean) => (
       <Icon type='search' style={{color: filtered ? '#1890ff' : undefined}}/>
     ),
-    onFilter: (value: string, record: any) =>
-      record[dataIndex].toString().toLowerCase().includes(value.toLowerCase()),
+    onFilter: (value: string, record: any) => {
+      if (record[dataIndex] !== undefined || record[dataIndex] !== null) {
+          return typeof (record[dataIndex]) === typeof {}
+            ? Boolean (record[dataIndex].find(item => Object.values(item).toString().toLowerCase().includes(value.toLowerCase())) ? true :false)
+            : record[dataIndex].toString().toLowerCase().includes(value.toLowerCase())
+        }
+        else {

Review Comment:
   record[dataIndex].toString().toLowerCase().includes(value.toLowerCase()) For normal string search.



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] dombizita merged pull request #5213: HDDS-9161. Recon Pipelines datanode columns search does not work

Posted by "dombizita (via GitHub)" <gi...@apache.org>.
dombizita merged PR #5213:
URL: https://github.com/apache/ozone/pull/5213


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] devmadhuu commented on a diff in pull request #5213: HDDS-9161. Recon Pipelines datanode columns search does not work

Posted by "devmadhuu (via GitHub)" <gi...@apache.org>.
devmadhuu commented on code in PR #5213:
URL: https://github.com/apache/ozone/pull/5213#discussion_r1311501854


##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/utils/columnSearch.tsx:
##########
@@ -69,8 +69,16 @@ class ColumnSearch extends React.PureComponent {
     filterIcon: (filtered: boolean) => (
       <Icon type='search' style={{color: filtered ? '#1890ff' : undefined}}/>
     ),
-    onFilter: (value: string, record: any) =>
-      record[dataIndex].toString().toLowerCase().includes(value.toLowerCase()),
+    onFilter: (value: string, record: any) => {
+      if (record[dataIndex] !== undefined || record[dataIndex] !== null) {
+          return typeof (record[dataIndex]) === typeof {}
+            ? Boolean (record[dataIndex].find(item => Object.values(item).toString().toLowerCase().includes(value.toLowerCase())) ? true :false)
+            : record[dataIndex].toString().toLowerCase().includes(value.toLowerCase())
+        }
+        else {

Review Comment:
   Is this else needed ?



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] dombizita commented on a diff in pull request #5213: HDDS-9161. Recon Pipelines datanode columns search does not work

Posted by "dombizita (via GitHub)" <gi...@apache.org>.
dombizita commented on code in PR #5213:
URL: https://github.com/apache/ozone/pull/5213#discussion_r1322715714


##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/utils/columnSearch.tsx:
##########
@@ -69,8 +69,16 @@ class ColumnSearch extends React.PureComponent {
     filterIcon: (filtered: boolean) => (
       <Icon type='search' style={{color: filtered ? '#1890ff' : undefined}}/>
     ),
-    onFilter: (value: string, record: any) =>
-      record[dataIndex].toString().toLowerCase().includes(value.toLowerCase()),
+    onFilter: (value: string, record: any) => {
+      if (record[dataIndex] !== undefined || record[dataIndex] !== null) {
+          return typeof (record[dataIndex]) === typeof {}
+            ? Boolean (record[dataIndex].find(item => Object.values(item).toString().toLowerCase().includes(value.toLowerCase())))
+            : record[dataIndex].toString().toLowerCase().includes(value.toLowerCase())
+        }
+        else {
+          return;
+        }

Review Comment:
   could you fix the indentation here? it is not to easy to understand the if/elses here, so it's better to have it more readable. I'm not sure if my suggestion is correct, it's hard to do it in the comment, but as I see in this file it should be 2 spaces. 
   ```suggestion
           return typeof (record[dataIndex]) === typeof {}
             ? Boolean (record[dataIndex].find(item => Object.values(item).toString().toLowerCase().includes(value.toLowerCase())))
             : record[dataIndex].toString().toLowerCase().includes(value.toLowerCase())
         }
         else {
           return;
         }
   ```
   ```suggestion
             return typeof (record[dataIndex]) === typeof {}
               ? Boolean (record[dataIndex].find(item => Object.values(item).toString().toLowerCase().includes(value.toLowerCase())))
               : record[dataIndex].toString().toLowerCase().includes(value.toLowerCase())
           }
           else {
             return;
           }
   ```



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] smitajoshi12 commented on a diff in pull request #5213: HDDS-9161. Recon Pipelines datanode columns search does not work

Posted by "smitajoshi12 (via GitHub)" <gi...@apache.org>.
smitajoshi12 commented on code in PR #5213:
URL: https://github.com/apache/ozone/pull/5213#discussion_r1322896340


##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/utils/columnSearch.tsx:
##########
@@ -69,8 +69,16 @@ class ColumnSearch extends React.PureComponent {
     filterIcon: (filtered: boolean) => (
       <Icon type='search' style={{color: filtered ? '#1890ff' : undefined}}/>
     ),
-    onFilter: (value: string, record: any) =>
-      record[dataIndex].toString().toLowerCase().includes(value.toLowerCase()),
+    onFilter: (value: string, record: any) => {
+      if (record[dataIndex] !== undefined || record[dataIndex] !== null) {
+          return typeof (record[dataIndex]) === typeof {}
+            ? Boolean (record[dataIndex].find(item => Object.values(item).toString().toLowerCase().includes(value.toLowerCase())))
+            : record[dataIndex].toString().toLowerCase().includes(value.toLowerCase())
+        }
+        else {
+          return;
+        }

Review Comment:
   @dombizita 
   Completed Change of space Indentation.



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] smitajoshi12 commented on a diff in pull request #5213: HDDS-9161. Recon Pipelines datanode columns search does not work

Posted by "smitajoshi12 (via GitHub)" <gi...@apache.org>.
smitajoshi12 commented on code in PR #5213:
URL: https://github.com/apache/ozone/pull/5213#discussion_r1312041398


##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/utils/columnSearch.tsx:
##########
@@ -69,8 +69,16 @@ class ColumnSearch extends React.PureComponent {
     filterIcon: (filtered: boolean) => (
       <Icon type='search' style={{color: filtered ? '#1890ff' : undefined}}/>
     ),
-    onFilter: (value: string, record: any) =>
-      record[dataIndex].toString().toLowerCase().includes(value.toLowerCase()),
+    onFilter: (value: string, record: any) => {
+      if (record[dataIndex] !== undefined || record[dataIndex] !== null) {
+          return typeof (record[dataIndex]) === typeof {}
+            ? record[dataIndex].find(item => Object.values(item).toString().toLowerCase().includes(value.toLowerCase())) ? true :false

Review Comment:
   @dombizita  @devabhishekpal 
   Boolean (record[dataIndex].find(item => Object.values(item).toString().toLowerCase().includes(value.toLowerCase())))
   added below condition in latest commit.



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] devabhishekpal commented on a diff in pull request #5213: HDDS-9161. Recon Pipelines datanode columns search does not work

Posted by "devabhishekpal (via GitHub)" <gi...@apache.org>.
devabhishekpal commented on code in PR #5213:
URL: https://github.com/apache/ozone/pull/5213#discussion_r1303409779


##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/utils/columnSearch.tsx:
##########
@@ -69,8 +69,15 @@ class ColumnSearch extends React.PureComponent {
     filterIcon: (filtered: boolean) => (
       <Icon type='search' style={{color: filtered ? '#1890ff' : undefined}}/>
     ),
-    onFilter: (value: string, record: any) =>
-      record[dataIndex].toString().toLowerCase().includes(value.toLowerCase()),
+    onFilter: (value: string, record: any) => {
+        if (record[dataIndex]) {
+          return typeof (record[dataIndex]) == 'object' ? Object.values(...record[dataIndex]).toString().toLowerCase().includes(value.toLowerCase())

Review Comment:
   Can we shorten the lines to keep at par with the code-style/readability?
   ```
   return typeof (record[dataIndex] === typeof({})
       ? Object.values(...record[dataIndex]).toString().toLowerCase().includes(value.toLowerCase())
       : record[dataIndex].toString().toLowerCase().includes(value.toLowerCase())
    ```



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] smitajoshi12 commented on a diff in pull request #5213: HDDS-9161. Recon Pipelines datanode columns search does not work

Posted by "smitajoshi12 (via GitHub)" <gi...@apache.org>.
smitajoshi12 commented on code in PR #5213:
URL: https://github.com/apache/ozone/pull/5213#discussion_r1303971178


##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/utils/columnSearch.tsx:
##########
@@ -69,8 +69,15 @@ class ColumnSearch extends React.PureComponent {
     filterIcon: (filtered: boolean) => (
       <Icon type='search' style={{color: filtered ? '#1890ff' : undefined}}/>
     ),
-    onFilter: (value: string, record: any) =>
-      record[dataIndex].toString().toLowerCase().includes(value.toLowerCase()),
+    onFilter: (value: string, record: any) => {
+        if (record[dataIndex]) {
+          return typeof (record[dataIndex]) == 'object' ? Object.values(...record[dataIndex]).toString().toLowerCase().includes(value.toLowerCase())

Review Comment:
   @devabhishekpal  Updated with Latest Commit.



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] dombizita commented on pull request #5213: HDDS-9161. Recon Pipelines datanode columns search does not work

Posted by "dombizita (via GitHub)" <gi...@apache.org>.
dombizita commented on PR #5213:
URL: https://github.com/apache/ozone/pull/5213#issuecomment-1717541765

   thanks for working on this @smitajoshi12! thanks for the review @devabhishekpal!


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org