Skip to content

HDDS-10018. Fix search function don't show on the first page about Node Status for SCM webUI#5983

Closed
aierate wants to merge 2 commits intoapache:masterfrom
aierate:HDDS-10018
Closed

HDDS-10018. Fix search function don't show on the first page about Node Status for SCM webUI#5983
aierate wants to merge 2 commits intoapache:masterfrom
aierate:HDDS-10018

Conversation

@aierate
Copy link
Contributor

@aierate aierate commented Jan 11, 2024

What changes were proposed in this pull request?

Fix search function don't show on the first page about Node Status for SCM webUI

Please describe your PR in detail:

In the Node Status column of SCM's webUI, when I searched for datanode, the detailed information of the datanode was not displayed on the first page, so I fixed it. After fixing, perform a search operation and the detailed information of the datanode will be displayed on the first page.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-10018

How was this patch tested?

This is a screenshot of the fixed version:

图片

when I search conway-hadoop4:
图片

@aierate
Copy link
Contributor Author

aierate commented Jan 11, 2024

Please help me review the code, thanks.

@adoroszlai adoroszlai added the UI label Jan 12, 2024
@ArafatKhan2198
Copy link
Contributor

Thank you for addressing this, @aierate. Could you provide further clarification on the issue you're encountering? Are you attempting to implement a search functionality for the Node Status table, or are you resolving an existing issue with it?

The logic for handling the search in this table is present in the scm-overview.html file at :-

<tr ng-repeat="typestat in nodeStatus|filter:search|orderBy:columnName:reverse">

How it works :-

When the user types something into the search input, AngularJS updates the value of $scope.search.
The filter:search part of the ng-repeat directive then filters the items in the nodeStatus array based on the value of $scope.search, showing only those items that match the search criteria. The items are then displayed in the table row () according to the filtering applied.

I checked it with the upsteream master build and the search works fine.
image

@aierate
Copy link
Contributor Author

aierate commented Apr 15, 2024

Thank you for addressing this, @aierate. Could you provide further clarification on the issue you're encountering? Are you attempting to implement a search functionality for the Node Status table, or are you resolving an existing issue with it?

The logic for handling the search in this table is present in the scm-overview.html file at :-

<tr ng-repeat="typestat in nodeStatus|filter:search|orderBy:columnName:reverse">

How it works :-

When the user types something into the search input, AngularJS updates the value of $scope.search. The filter:search part of the ng-repeat directive then filters the items in the nodeStatus array based on the value of $scope.search, showing only those items that match the search criteria. The items are then displayed in the table row () according to the filtering applied.

I checked it with the upsteream master build and the search works fine. image

Thanks @ArafatKhan2198 . When the number of DataNodes exceeds one page, if you search for DataNodes on the second page, the search results will be displayed on the second page.

Copy link
Contributor

@ArafatKhan2198 ArafatKhan2198 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aierate, thank you for clarifying. It seems that the search function only works for the records currently displayed on the page, which indeed is a bug. I appreciate you bringing this to our attention. The approach looks sound overall; however, I do have a few suggestions that you can consider.

Copy link
Contributor

@ArafatKhan2198 ArafatKhan2198 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the changes @aierate
LGTM +1

@ArafatKhan2198
Copy link
Contributor

@adoroszlai could you take a look as well!

@adoroszlai adoroszlai requested a review from dombizita April 24, 2024 13:14
@smitajoshi12
Copy link
Contributor

smitajoshi12 commented May 9, 2024

@muskan1012
Can you also review this PR

@smitajoshi12
Copy link
Contributor

Looks good to me.

@aierate
Copy link
Contributor Author

aierate commented Jun 28, 2024

Can this PR be merged, please?

Copy link
Contributor

@dombizita dombizita left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this @aierate! Sorry for the late review and thanks for pinging us on the PR. I checked out you changes, built it and locally tried it with docker. I created a cluster with 15 datanodes and checked the SCM UI. Without the search everything looked good.

  • I searched for "1"
step1
  • The "Next" button was clickable, I clicked on it.
step2
  • I was able to go back with the "Previous" button
step3

Could you look into this? It should have show me the 7 seven datanodes with "1" in their names, I shouldn't have been able to go to the next page. It's also unexpected that when I came back to the first page again, I saw 6th datanode in the list, but not the 2 datanodes that were visible on the second page.

@ArafatKhan2198
Copy link
Contributor

@aierate Could you please take a look at the comments and fix them if necessary.

@aierate
Copy link
Contributor Author

aierate commented Jul 16, 2024

@aierate Could you please take a look at the comments and fix them if necessary.

Thanks @ArafatKhan2198 @dombizita. I will try to fix these issues later. Thank you for your suggestions

@adoroszlai
Copy link
Contributor

@aierate would you like to continue work on this?

@adoroszlai
Copy link
Contributor

/pending will try to fix these issues later

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Marking this issue as un-mergeable as requested.

Please use /ready comment when it's resolved.

Please note that the PR will be closed after 21 days of inactivity from now. (But can be re-opened anytime later...)

will try to fix these issues later

@github-actions
Copy link

Thank you very much for the patch. I am closing this PR temporarily as there was no activity recently and it is waiting for response from its author.

It doesn't mean that this PR is not important or ignored: feel free to reopen the PR at any time.

It only means that attention of committers is not required. We prefer to keep the review queue clean. This ensures PRs in need of review are more visible, which results in faster feedback for all PRs.

If you need ANY help to finish this PR, please contact the community on the mailing list or the slack channel."

@github-actions github-actions bot closed this Apr 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants