Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[BUG] Fix management overview page duplicate rendering #4636

Merged
merged 10 commits into from
Aug 31, 2023
Merged

[BUG] Fix management overview page duplicate rendering #4636

merged 10 commits into from
Aug 31, 2023

Conversation

Hailong-am
Copy link
Collaborator

Description

Refactor management overview page add useMemo to avoid duplicate rendering.

Issues Resolved

#4287 Add a new management overview page, whenever go to app/opensearch_management_overview page, the console reports below errors.

Screenshot

image

Testing the changes

Check List

  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
    • yarn test:ftr
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

@Hailong-am Hailong-am changed the title [BUG] Refactor for management overview page [BUG] Refactor for management overview page to avoid duplicate rendering Jul 28, 2023
@Hailong-am Hailong-am changed the title [BUG] Refactor for management overview page to avoid duplicate rendering [BUG] Fix management overview page duplicate rendering Jul 28, 2023
@codecov
Copy link

codecov bot commented Jul 28, 2023 •
edited
Loading

Codecov Report

Merging #4636 (0e1a830) into main (e9ad10f) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #4636      +/-   ##
==========================================
- Coverage   66.43%   66.43%   -0.01%     
==========================================
  Files        3396     3397       +1     
  Lines       64869    64882      +13     
  Branches    10358    10359       +1     
==========================================
+ Hits        43096    43102       +6     
- Misses      19214    19300      +86     
+ Partials     2559     2480      -79
Flag Coverage Δ
Linux_1 34.85% <ø> (ø)
Linux_2 55.15% <ø> (ø)
Linux_3 44.23% <ø> (-0.01%) ⬇️
Linux_4 35.05% <100.00%> (+<0.01%) ⬆️
Windows_1 34.87% <ø> (ø)
Windows_2 55.12% <ø> (ø)
Windows_3 44.24% <ø> (ø)
Windows_4 35.05% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
...plugins/management_overview/public/application.tsx 61.53% <100.00%> (ø)

... and 20 files with indirect coverage changes

@Hailong-am Hailong-am force-pushed the optimize_management_overview branch 2 times, most recently from ebd9a24 to fd252cd Compare August 2, 2023 01:00
@Hailong-am
Copy link
Collaborator Author

@ashwin-pc @manasvinibs @ananzh could you help to review the fix?

ashwin-pc
ashwin-pc previously approved these changes Aug 4, 2023
View reviewed changes
Copy link
Member

@ashwin-pc ashwin-pc left a comment

Choose a reason for hiding this comment

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

LGTM, can you resolve the changelog conflict though?

@Hailong-am
Copy link
Collaborator Author

LGTM, can you resolve the changelog conflict though?

rebased from main.

@Hailong-am
Copy link
Collaborator Author

Link check failed due to, not sure whether it's transient issue

Errors in packages/osd-ui-shared-deps/flot_charts/jquery_flot_navigate.js
+ echo
+ echo ::set-output name=exit_code::2
✗ http://brandonaaron.net/ (HTTP status server error (523 <unknown status code>) for url (http://brandonaaron.net/))```

@Hailong-am
Copy link
Collaborator Author

@ashwin-pc conflicts merged and please help to add backport 2.x label.

@Hailong-am Hailong-am requested a review from curq as a code owner August 29, 2023 04:09
ashwin-pc
ashwin-pc previously approved these changes Aug 29, 2023
View reviewed changes
Copy link
Member

@ashwin-pc ashwin-pc left a comment

Choose a reason for hiding this comment

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

LGTM :)

joshuarrrr
joshuarrrr reviewed Aug 30, 2023
View reviewed changes
Copy link
Member

@joshuarrrr joshuarrrr left a comment

Choose a reason for hiding this comment

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

Looks good - I have one question for other maintainers to clarify first.

src/plugins/management_overview/public/index.ts Outdated Show resolved Hide resolved
src/plugins/management_overview/public/application.test.tsx Outdated Show resolved Hide resolved
src/plugins/management_overview/public/application.test.tsx Outdated Show resolved Hide resolved
Hailong-am and others added 7 commits August 30, 2023 11:45
@Hailong-am
refactor for management overview page
1021018 
Signed-off-by: Hailong Cui <ihailong@amazon.com>
@Hailong-am
Add missing changelog
fb4b4e7 
Signed-off-by: Hailong Cui <ihailong@amazon.com>
@Hailong-am
Add unit test
b074a28 
Signed-off-by: Hailong Cui <ihailong@amazon.com>
@Hailong-am
remove duplicate snapshot validation
5234da6 
Signed-off-by: Hailong Cui <ihailong@amazon.com>
@Hailong-am @joshuarrrr
Update src/plugins/management_overview/public/application.test.tsx
45a831c 
Co-authored-by: Josh Romero <rmerqg@amazon.com>
Signed-off-by: Hailong Cui <ihailong@amazon.com>
@Hailong-am @joshuarrrr
Update src/plugins/management_overview/public/application.test.tsx
ecb378a 
Co-authored-by: Josh Romero <rmerqg@amazon.com>
Signed-off-by: Hailong Cui <ihailong@amazon.com>
@Hailong-am
export empty plugin start to make it consistent
bad97e2 
Signed-off-by: Hailong Cui <ihailong@amazon.com>
@ashwin-pc ashwin-pc merged commit 16b1e29 into opensearch-project:main Aug 31, 2023
54 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Aug 31, 2023
@github-actions
[BUG] Fix management overview page duplicate rendering (#4636)
48393eb 
* refactor for management overview page

Signed-off-by: Hailong Cui <ihailong@amazon.com>

* Add missing changelog

Signed-off-by: Hailong Cui <ihailong@amazon.com>

* Add unit test

Signed-off-by: Hailong Cui <ihailong@amazon.com>

* remove duplicate snapshot validation

Signed-off-by: Hailong Cui <ihailong@amazon.com>

* Update src/plugins/management_overview/public/application.test.tsx

Co-authored-by: Josh Romero <rmerqg@amazon.com>
Signed-off-by: Hailong Cui <ihailong@amazon.com>

* Update src/plugins/management_overview/public/application.test.tsx

Co-authored-by: Josh Romero <rmerqg@amazon.com>
Signed-off-by: Hailong Cui <ihailong@amazon.com>

* export empty plugin start to make it consistent

Signed-off-by: Hailong Cui <ihailong@amazon.com>

* export empty plugin start to make it consistent

Signed-off-by: Hailong Cui <ihailong@amazon.com>

---------

Signed-off-by: Hailong Cui <ihailong@amazon.com>
Co-authored-by: Josh Romero <rmerqg@amazon.com>
Co-authored-by: Ashwin P Chandran <ashwinpc@amazon.com>
(cherry picked from commit 16b1e29)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

# Conflicts:
#	CHANGELOG.md
@opensearch-trigger-bot opensearch-trigger-bot bot mentioned this pull request Aug 31, 2023
Merged
manasvinibs pushed a commit that referenced this pull request Sep 1, 2023
@opensearch-trigger-bot @github-actions
[BUG] Fix management overview page duplicate rendering (#4636) (#4871)
4db18c9 
* refactor for management overview page

Signed-off-by: Hailong Cui <ihailong@amazon.com>

* Add missing changelog

Signed-off-by: Hailong Cui <ihailong@amazon.com>

* Add unit test

Signed-off-by: Hailong Cui <ihailong@amazon.com>

* remove duplicate snapshot validation

Signed-off-by: Hailong Cui <ihailong@amazon.com>

* Update src/plugins/management_overview/public/application.test.tsx

Co-authored-by: Josh Romero <rmerqg@amazon.com>
Signed-off-by: Hailong Cui <ihailong@amazon.com>

* Update src/plugins/management_overview/public/application.test.tsx

Co-authored-by: Josh Romero <rmerqg@amazon.com>
Signed-off-by: Hailong Cui <ihailong@amazon.com>

* export empty plugin start to make it consistent

Signed-off-by: Hailong Cui <ihailong@amazon.com>

* export empty plugin start to make it consistent

Signed-off-by: Hailong Cui <ihailong@amazon.com>

---------

Signed-off-by: Hailong Cui <ihailong@amazon.com>
Co-authored-by: Josh Romero <rmerqg@amazon.com>
Co-authored-by: Ashwin P Chandran <ashwinpc@amazon.com>
(cherry picked from commit 16b1e29)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

# Conflicts:
#	CHANGELOG.md

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joshuarrrr joshuarrrr joshuarrrr left review comments

@AMoo-Miki AMoo-Miki AMoo-Miki approved these changes

@ashwin-pc ashwin-pc ashwin-pc approved these changes

@ananzh ananzh Awaiting requested review from ananzh ananzh is a code owner

@kavilla kavilla Awaiting requested review from kavilla kavilla is a code owner

@seanneumann seanneumann Awaiting requested review from seanneumann

@abbyhu2000 abbyhu2000 Awaiting requested review from abbyhu2000 abbyhu2000 is a code owner

@zengyan-amazon zengyan-amazon Awaiting requested review from zengyan-amazon zengyan-amazon is a code owner

@kristenTian kristenTian Awaiting requested review from kristenTian

@zhongnansu zhongnansu Awaiting requested review from zhongnansu zhongnansu is a code owner

@manasvinibs manasvinibs Awaiting requested review from manasvinibs manasvinibs is a code owner

@ZilongX ZilongX Awaiting requested review from ZilongX ZilongX is a code owner

@Flyingliuhub Flyingliuhub Awaiting requested review from Flyingliuhub Flyingliuhub is a code owner

@BSFishy BSFishy Awaiting requested review from BSFishy

@curq curq Awaiting requested review from curq curq is a code owner

Assignees
No one assigned
Labels
backport 2.x repeat-contributor v2.10.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Hailong-am @joshuarrrr @AMoo-Miki @ashwin-pc