Re: [PR] HDDS-15007. SCM Web UI search box should filter the entire datanodes set, not just the displayed rows [ozone]
navinko commented on PR #10076: URL: https://github.com/apache/ozone/pull/10076#issuecomment-4432754796 Thanks @smengcl for the review and merge. -- 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]
Re: [PR] HDDS-15007. SCM Web UI search box should filter the entire datanodes set, not just the displayed rows [ozone]
smengcl commented on PR #10076: URL: https://github.com/apache/ozone/pull/10076#issuecomment-4427682734 Thanks @navinko for the patch. -- 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]
Re: [PR] HDDS-15007. SCM Web UI search box should filter the entire datanodes set, not just the displayed rows [ozone]
smengcl merged PR #10076: URL: https://github.com/apache/ozone/pull/10076 -- 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]
Re: [PR] HDDS-15007. SCM Web UI search box should filter the entire datanodes set, not just the displayed rows [ozone]
navinko commented on PR #10076: URL: https://github.com/apache/ozone/pull/10076#issuecomment-4424691240 Thanks @smengcl for the review. Tried addressing suggestions , once you get some time please review once . -- 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]
Re: [PR] HDDS-15007. SCM Web UI search box should filter the entire datanodes set, not just the displayed rows [ozone]
navinko commented on code in PR #10076:
URL: https://github.com/apache/ozone/pull/10076#discussion_r3221782899
##
hadoop-hdds/server-scm/src/main/resources/webapps/scm/scm.js:
##
@@ -218,28 +221,48 @@
$scope.statistics.containers.health.open_without_pipeline
= ctrl.scmcontainermanager.OpenContainersWithoutPipeline;
});
-/*if option is 'All' display all records else display specified
record on page*/
-$scope.UpdateRecordsToShow = () => {
-if($scope.RecordsToDisplay == 'All') {
-$scope.lastIndex = 1;
-$scope.nodeStatus = nodeStatusCopy;
+/* Global Search Logic */
+$scope.applyGlobalSearch = function() {
+if (!$scope.search || $scope.search.trim() === "") {
+// Reset to full list if search is empty
+$scope.filteredNodes = [...nodeStatusCopy];
} else {
-$scope.lastIndex = Math.ceil(nodeStatusCopy.length /
$scope.RecordsToDisplay);
-$scope.nodeStatus = nodeStatusCopy.slice(0,
$scope.RecordsToDisplay);
-}
-$scope.currentPage = 1;
-}
-/* Page Slicing logic */
-$scope.handlePagination = (pageIndex, isDisabled) => {
-if(!isDisabled && $scope.RecordsToDisplay != 'All') {
-pageIndex = parseInt(pageIndex);
-let startIndex = 0, endIndex = 0;
-$scope.currentPage = pageIndex;
-startIndex = ($scope.currentPage - 1) *
parseInt($scope.RecordsToDisplay);
-endIndex = startIndex + parseInt($scope.RecordsToDisplay);
-$scope.nodeStatus = nodeStatusCopy.slice(startIndex,
endIndex);
+let query = $scope.search.toLowerCase();
+// Filter the master list
+$scope.filteredNodes =
nodeStatusCopy.filter(function(node) {
+return (node.hostname &&
node.hostname.toLowerCase().includes(query)) ||
+ (node.opstate &&
node.opstate.toLowerCase().includes(query)) ||
+ (node.comstate &&
node.comstate.toLowerCase().includes(query)) ||
+ (node.uuid &&
node.uuid.toLowerCase().includes(query));
+});
Review Comment:
Thanks @smengcl for reviewing , Definitely suggested approach would be
maintenance free and no need to update the search criteria if any columns are
being added/removed.
Instead of manually listing out each and every single column name (like
hostname, capacity, uuid, etc.), I just updated the search to use
Object.values(node). This tells the code to automatically grab every piece of
data inside that specific row, convert it to text, and check if it matches the
search query.
This also matches the original search behavior and it automatically
future-proofs the page so we never have to update this code if new columns are
added/removed later!
-> search by any colume for example , i used lastHeartbeat for dn sits on
2nd page
https://github.com/user-attachments/assets/2e84b7be-bdab-43eb-bb7c-c076029c04b8";
/>
-> global serach by lastHeartbeat
https://github.com/user-attachments/assets/64dfbcea-853d-4fa3-9d4a-4716f41cff17";
/>
--
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]
Re: [PR] HDDS-15007. SCM Web UI search box should filter the entire datanodes set, not just the displayed rows [ozone]
navinko commented on code in PR #10076:
URL: https://github.com/apache/ozone/pull/10076#discussion_r3221761432
##
hadoop-hdds/server-scm/src/main/resources/webapps/scm/scm.js:
##
@@ -218,28 +221,48 @@
$scope.statistics.containers.health.open_without_pipeline
= ctrl.scmcontainermanager.OpenContainersWithoutPipeline;
});
-/*if option is 'All' display all records else display specified
record on page*/
-$scope.UpdateRecordsToShow = () => {
-if($scope.RecordsToDisplay == 'All') {
-$scope.lastIndex = 1;
-$scope.nodeStatus = nodeStatusCopy;
+/* Global Search Logic */
+$scope.applyGlobalSearch = function() {
+if (!$scope.search || $scope.search.trim() === "") {
+// Reset to full list if search is empty
+$scope.filteredNodes = [...nodeStatusCopy];
} else {
-$scope.lastIndex = Math.ceil(nodeStatusCopy.length /
$scope.RecordsToDisplay);
-$scope.nodeStatus = nodeStatusCopy.slice(0,
$scope.RecordsToDisplay);
-}
-$scope.currentPage = 1;
-}
-/* Page Slicing logic */
-$scope.handlePagination = (pageIndex, isDisabled) => {
-if(!isDisabled && $scope.RecordsToDisplay != 'All') {
-pageIndex = parseInt(pageIndex);
-let startIndex = 0, endIndex = 0;
-$scope.currentPage = pageIndex;
-startIndex = ($scope.currentPage - 1) *
parseInt($scope.RecordsToDisplay);
-endIndex = startIndex + parseInt($scope.RecordsToDisplay);
-$scope.nodeStatus = nodeStatusCopy.slice(startIndex,
endIndex);
+let query = $scope.search.toLowerCase();
+// Filter the master list
+$scope.filteredNodes =
nodeStatusCopy.filter(function(node) {
+return (node.hostname &&
node.hostname.toLowerCase().includes(query)) ||
+ (node.opstate &&
node.opstate.toLowerCase().includes(query)) ||
+ (node.comstate &&
node.comstate.toLowerCase().includes(query)) ||
+ (node.uuid &&
node.uuid.toLowerCase().includes(query));
+});
}
-}
+$scope.totalItems = $scope.filteredNodes.length;
+$scope.UpdateRecordsToShow(); // Re-calculate pagination
+};
+ /* If option is 'All' display all records, else display specified
records on page */
+ $scope.UpdateRecordsToShow = () => {
+ if ($scope.RecordsToDisplay === 'All') {
+ $scope.lastIndex = 1;
+ $scope.nodeStatus = $scope.filteredNodes;
+ } else {
+ let limit = parseInt($scope.RecordsToDisplay);
+ $scope.lastIndex = Math.ceil($scope.filteredNodes.length
/ limit);
+ $scope.nodeStatus = $scope.filteredNodes.slice(0, limit);
+ }
+ $scope.currentPage = 1;
Review Comment:
Good catch! I missed this scenario of empty list .
In the existing code,the pagination logic is implemented in two separate
places:
Once for the initial page load
https://github.com/navinko/ozone/blob/master/hadoop-hdds/server-scm/src/main/resources/webapps/scm/scm.js#L166-L167
> $scope.totalItems = nodeStatusCopy.length;
**$scope.lastIndex = Math.ceil(nodeStatusCopy.length /
$scope.RecordsToDisplay);**
$scope.nodeStatus = nodeStatusCopy.slice(0, $scope.RecordsToDisplay);
and once user clicks for the dropdown updates.
https://github.com/navinko/ozone/blob/master/hadoop-hdds/server-scm/src/main/resources/webapps/scm/scm.js#L253-L254
> $scope.UpdateRecordsToShow = () => {
if($scope.RecordsToDisplay == 'All') {
$scope.lastIndex = 1;
$scope.nodeStatus = nodeStatusCopy;
} else {
**$scope.lastIndex = Math.ceil(nodeStatusCopy.length /
$scope.RecordsToDisplay);**
$scope.nodeStatus = nodeStatusCopy.slice(0,
$scope.RecordsToDisplay);
}
$scope.currentPage = 1;
}
In both places, if nodeStatusCopy.length would be 0, (cluster with empty
datanode list ) the lastIndex evaluated to 0. Since currentPage resets to 1,
the lastIndex == currentPage check failed and did not disbale next button
leaving the UI broken at "Page 1 of 0" with an active Next button.
In order to fix that used math.max(1,0) logic which will return lastIndex to
one during both the cases, fresh load with empty datanode list and once user
Re: [PR] HDDS-15007. SCM Web UI search box should filter the entire datanodes set, not just the displayed rows [ozone]
smengcl commented on code in PR #10076:
URL: https://github.com/apache/ozone/pull/10076#discussion_r3199108286
##
hadoop-hdds/server-scm/src/main/resources/webapps/scm/scm.js:
##
@@ -218,28 +221,48 @@
$scope.statistics.containers.health.open_without_pipeline
= ctrl.scmcontainermanager.OpenContainersWithoutPipeline;
});
-/*if option is 'All' display all records else display specified
record on page*/
-$scope.UpdateRecordsToShow = () => {
-if($scope.RecordsToDisplay == 'All') {
-$scope.lastIndex = 1;
-$scope.nodeStatus = nodeStatusCopy;
+/* Global Search Logic */
+$scope.applyGlobalSearch = function() {
+if (!$scope.search || $scope.search.trim() === "") {
+// Reset to full list if search is empty
+$scope.filteredNodes = [...nodeStatusCopy];
} else {
-$scope.lastIndex = Math.ceil(nodeStatusCopy.length /
$scope.RecordsToDisplay);
-$scope.nodeStatus = nodeStatusCopy.slice(0,
$scope.RecordsToDisplay);
-}
-$scope.currentPage = 1;
-}
-/* Page Slicing logic */
-$scope.handlePagination = (pageIndex, isDisabled) => {
-if(!isDisabled && $scope.RecordsToDisplay != 'All') {
-pageIndex = parseInt(pageIndex);
-let startIndex = 0, endIndex = 0;
-$scope.currentPage = pageIndex;
-startIndex = ($scope.currentPage - 1) *
parseInt($scope.RecordsToDisplay);
-endIndex = startIndex + parseInt($scope.RecordsToDisplay);
-$scope.nodeStatus = nodeStatusCopy.slice(startIndex,
endIndex);
+let query = $scope.search.toLowerCase();
+// Filter the master list
+$scope.filteredNodes =
nodeStatusCopy.filter(function(node) {
+return (node.hostname &&
node.hostname.toLowerCase().includes(query)) ||
+ (node.opstate &&
node.opstate.toLowerCase().includes(query)) ||
+ (node.comstate &&
node.comstate.toLowerCase().includes(query)) ||
+ (node.uuid &&
node.uuid.toLowerCase().includes(query));
+});
Review Comment:
This should also cover other fields like Used Space Percent / Capacity /
Last Heartbeat to match original search behavior?
Or even better for maintenance: is it possible to match the whole row
without having to specify each in case we add new columns in the future but
forget to change the code here?
##
hadoop-hdds/server-scm/src/main/resources/webapps/scm/scm.js:
##
@@ -218,28 +221,48 @@
$scope.statistics.containers.health.open_without_pipeline
= ctrl.scmcontainermanager.OpenContainersWithoutPipeline;
});
-/*if option is 'All' display all records else display specified
record on page*/
-$scope.UpdateRecordsToShow = () => {
-if($scope.RecordsToDisplay == 'All') {
-$scope.lastIndex = 1;
-$scope.nodeStatus = nodeStatusCopy;
+/* Global Search Logic */
+$scope.applyGlobalSearch = function() {
+if (!$scope.search || $scope.search.trim() === "") {
+// Reset to full list if search is empty
+$scope.filteredNodes = [...nodeStatusCopy];
} else {
-$scope.lastIndex = Math.ceil(nodeStatusCopy.length /
$scope.RecordsToDisplay);
-$scope.nodeStatus = nodeStatusCopy.slice(0,
$scope.RecordsToDisplay);
-}
-$scope.currentPage = 1;
-}
-/* Page Slicing logic */
-$scope.handlePagination = (pageIndex, isDisabled) => {
-if(!isDisabled && $scope.RecordsToDisplay != 'All') {
-pageIndex = parseInt(pageIndex);
-let startIndex = 0, endIndex = 0;
-$scope.currentPage = pageIndex;
-startIndex = ($scope.currentPage - 1) *
parseInt($scope.RecordsToDisplay);
-endIndex = startIndex + parseInt($scope.RecordsToDisplay);
-$scope.nodeStatus = nodeStatusCopy.slice(startIndex,
endIndex);
+let query = $scope.search.toLowerCase();
+// Filter the master list
+$scope.filteredNodes =
nodeStatusCopy.filter(function(node) {
+return (node.hostname &&
node.hostname.toLowerCase().includes(query)) ||
+ (node.opstate &&
node.opstate.toLowerCase().includes(query)) ||
+ (node.comstate &&
Re: [PR] HDDS-15007. SCM Web UI search box should filter the entire datanodes set, not just the displayed rows [ozone]
navinko commented on PR #10076: URL: https://github.com/apache/ozone/pull/10076#issuecomment-4360053650 Hi @smengcl @jojochuang Could you please review the changes. Thanks. -- 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]
