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.

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

Revert "Remove MDS selector for tenancy tab" #1943

Merged
merged 2 commits into from
May 14, 2024

Conversation

derek-ho
Copy link
Collaborator

@derek-ho derek-ho commented May 9, 2024

Reverts #1937

Edit: Adding more context here. The reason for this revert is @kgcreative's comment here: #1933 (comment). Ideally, this tab should have the picker locked to the local cluster to maintain the same look across the plugin. Core doesn't have any plan to implement this, so we will add this back, and call it a user mis-configuration if that decide to hide the local cluster. Since security plugin is more of an admin view, when they visit they should have local cluster enabled.

Copy link

codecov bot commented May 9, 2024

Codecov Report

Attention: Patch coverage is 0% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 69.91%. Comparing base (e41d4d0) to head (6e4a575).

Files Patch % Lines
...s/configuration/panels/tenant-list/tenant-list.tsx 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1943      +/-   ##
==========================================
- Coverage   69.94%   69.91%   -0.03%     
==========================================
  Files          97       97              
  Lines        2505     2506       +1     
  Branches      343      336       -7     
==========================================
  Hits         1752     1752              
- Misses        672      673       +1     
  Partials       81       81              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@stephen-crawford stephen-crawford left a comment

Choose a reason for hiding this comment

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

Looks fine but is there context here?

@DarshitChanpura
Copy link
Member

Why are we reverting this?

@derek-ho
Copy link
Collaborator Author

Why are we reverting this?

Added context in the description

@DarshitChanpura
Copy link
Member

Thank you for updating the description @derek-ho !

@DarshitChanpura
Copy link
Member

@derek-ho Can you check code-cov failures once to determine if any tests are necessary?

@derek-ho
Copy link
Collaborator Author

@derek-ho Can you check code-cov failures once to determine if any tests are necessary?

I think it's fine, the test change also works and its a one-liner. Merging as is.

@derek-ho derek-ho merged commit 37c2695 into main May 14, 2024
34 of 36 checks passed
@derek-ho derek-ho added the backport 2.x backport to 2.x branch label May 14, 2024
opensearch-trigger-bot bot pushed a commit that referenced this pull request May 14, 2024
This reverts commit 9d0f262.

Co-authored-by: Darshit Chanpura <[email protected]>
(cherry picked from commit 37c2695)
derek-ho added a commit that referenced this pull request May 14, 2024
This reverts commit 9d0f262.

Co-authored-by: Darshit Chanpura <[email protected]>
(cherry picked from commit 37c2695)

Co-authored-by: Derek Ho <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x backport to 2.x branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants