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

Saved Object Aggregation View #1146

Merged
merged 14 commits into from
Oct 27, 2022
Merged

Saved Object Aggregation View #1146

merged 14 commits into from
Oct 27, 2022

Conversation

cliu123
Copy link
Member

@cliu123 cliu123 commented Oct 18, 2022

Description

Enable saved object table to show saved objects across all permitted tenants on a single panel. This is the 1st milestone of the new saved object sharing experience.

Category

New feature

Why these changes are required?

The 1st milestone of the new saved object sharing experience.

What is the old behavior before changes and new behavior after changes?

(See the issue)
Current Behavior:
The Saved Object table shows only the saved objects in the selected tenants.
New Behavior:
The Saved Object table shows all the saved objects across all tenants that the logged in user has permissions to.

Issues Resolved

opensearch-project/OpenSearch-Dashboards#2249

Testing

Check List

  • New functionality includes testing
  • New functionality has been documented
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

cliu123 and others added 5 commits October 17, 2022 16:42
Co-authored-by: Craig Perkins <[email protected]>
Co-authored-by: Ryan Liang <[email protected]>
Co-authored-by: Yan Zeng <[email protected]>

Signed-off-by: Chang Liu <[email protected]>
* Add cypress CI for aggregation view

Signed-off-by: Ryan Liang <[email protected]>
@cliu123 cliu123 requested a review from a team October 18, 2022 00:00
@codecov-commenter
Copy link

codecov-commenter commented Oct 18, 2022

Codecov Report

Merging #1146 (4e67d39) into main (7b80f9b) will decrease coverage by 1.18%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1146      +/-   ##
==========================================
- Coverage   75.08%   73.90%   -1.19%     
==========================================
  Files          86       86              
  Lines        1975     1901      -74     
  Branches      278      251      -27     
==========================================
- Hits         1483     1405      -78     
- Misses        432      439       +7     
+ Partials       60       57       -3     
Impacted Files Coverage Δ
...hboards-plugin/public/apps/account/account-app.tsx 54.83% <0.00%> (-11.02%) ⬇️
...n/public/apps/configuration/utils/tenant-utils.tsx 68.75% <0.00%> (-5.83%) ⬇️
...plugin/public/apps/account/tenant-switch-panel.tsx 89.47% <0.00%> (-2.67%) ⬇️
...-plugin/public/apps/account/account-nav-button.tsx 71.42% <0.00%> (-2.26%) ⬇️
...ps/configuration/panels/role-edit/tenant-panel.tsx 100.00% <0.00%> (ø)
...ration/panels/role-edit/index-permission-panel.tsx 93.02% <0.00%> (+2.11%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Member

@peternied peternied left a comment

Choose a reason for hiding this comment

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

The 1st milestone

I didn't seen milestones called out in opensearch-project/security#1869, is this Saved Objects Aggregated View the only consideration in scope?

public/apps/account/tenant-switch-panel.tsx Outdated Show resolved Hide resolved
server/auth/types/authentication_type.ts Outdated Show resolved Hide resolved
.github/workflows/cypress-test.yml Show resolved Hide resolved
.github/workflows/cypress-test.yml Show resolved Hide resolved
Copy link
Member

@DarshitChanpura DarshitChanpura 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 overall! Requested a few changes

public/apps/account/account-app.tsx Outdated Show resolved Hide resolved
public/apps/account/account-nav-button.tsx Show resolved Hide resolved
opensearch_dashboards.json Show resolved Hide resolved
public/plugin.tsx Outdated Show resolved Hide resolved
public/plugin.tsx Outdated Show resolved Hide resolved
public/plugin.tsx Outdated Show resolved Hide resolved
server/plugin.ts Outdated Show resolved Hide resolved
public/plugin.tsx Outdated Show resolved Hide resolved
public/plugin.tsx Outdated Show resolved Hide resolved
@cliu123
Copy link
Member Author

cliu123 commented Oct 21, 2022

The 1st milestone

I didn't seen milestones called out in opensearch-project/security#1869, is this Saved Objects Aggregated View the only consideration in scope?

The PM, UX and Eng are working on figuring out milestones in detail. We have a direction that we are going towards, but the expected UX needs to be fleshed out to provide the following milestones. The issue that this PR is trying to resolve has been linked to the RFC.

opensearch_dashboards.json Show resolved Hide resolved
public/apps/account/account-app.tsx Outdated Show resolved Hide resolved
public/apps/account/account-app.tsx Outdated Show resolved Hide resolved
public/plugin.tsx Outdated Show resolved Hide resolved
public/plugin.tsx Outdated Show resolved Hide resolved
server/auth/types/authentication_type.ts Outdated Show resolved Hide resolved
server/plugin.ts Outdated Show resolved Hide resolved
server/saved_objects/saved_objects_wrapper.ts Outdated Show resolved Hide resolved
server/saved_objects/saved_objects_wrapper.ts Outdated Show resolved Hide resolved
@cliu123 cliu123 added backport 2.x backport to 2.x branch v2.4.0 'Issues and PRs related to version v2.4.0' and removed backport 2.x backport to 2.x branch v2.4.0 'Issues and PRs related to version v2.4.0' labels Oct 27, 2022
@cliu123
Copy link
Member Author

cliu123 commented Oct 27, 2022

@peternied Thanks for reviewing! Sorry, I didn’t mean to re-request your review. I clicked the request review button on all reviewers by mistake.

FTR_PATH: 'ftr'
START_CMD: 'node ../scripts/opensearch_dashboards --dev --no-base-path --no-watch --opensearch_security.multitenancy.enable_aggregation_view=true'
OPENSEARCH_SNAPSHOT_CMD: 'node ../scripts/opensearch snapshot'
SPEC: 'cypress/integration/plugins/security-dashboards-plugin/aggregation_view.js,'
Copy link
Member

@DarshitChanpura DarshitChanpura Oct 27, 2022

Choose a reason for hiding this comment

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

should this changed to a folder path? Could be done in future if we are planning to add and test more cypress tests.

tenant = await fetchCurrentTenant(coreStart.http);
} catch (e) {
tenant = undefined;
console.log(e);
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this console.log?

@peternied peternied merged commit 7c8ede6 into main Oct 27, 2022
cliu123 added a commit to cliu123/security-dashboards-plugin that referenced this pull request Nov 3, 2022
(cherry picked from commit 7c8ede6)

Signed-off-by: Chang Liu <[email protected]>
Signed-off-by: Ryan Liang <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Yan Zeng <[email protected]>

Co-authored-by: Ryan Liang <[email protected]>
Co-authored-by: Ryan Liang <[email protected]>
Co-authored-by: Craig Perkins <[email protected]>
Co-authored-by: Yan Zeng <[email protected]>

Change version to 2.4 for backport

Signed-off-by: Chang Liu <[email protected]>
cliu123 added a commit to cliu123/security-dashboards-plugin that referenced this pull request Nov 3, 2022
(cherry picked from commit 7c8ede6)

Signed-off-by: Chang Liu <[email protected]>
Signed-off-by: Ryan Liang <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Yan Zeng <[email protected]>

Co-authored-by: Ryan Liang <[email protected]>
Co-authored-by: Ryan Liang <[email protected]>
Co-authored-by: Craig Perkins <[email protected]>
Co-authored-by: Yan Zeng <[email protected]>
peternied pushed a commit that referenced this pull request Nov 3, 2022
* Saved Object Aggregation View (#1146)
* Move tenant-related utils to common folder (#1184)
* [Saved Object Aggregation View] Use namespace registry to add tenant filter (#1169)

Signed-off-by: Chang Liu <[email protected]>
Signed-off-by: Ryan Liang <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Yan Zeng <[email protected]>

Co-authored-by: Ryan Liang <[email protected]>
Co-authored-by: Ryan Liang <[email protected]>
Co-authored-by: Craig Perkins <[email protected]>
Co-authored-by: Yan Zeng <[email protected]>
peternied pushed a commit that referenced this pull request Nov 3, 2022
* Saved Object Aggregation View (#1146)
* Move tenant-related utils to common folder (#1184)
* [Saved Object Aggregation View] Use namespace registry to add tenant filter (#1169)

Signed-off-by: Chang Liu <[email protected]>
Signed-off-by: Ryan Liang <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Yan Zeng <[email protected]>

Co-authored-by: Ryan Liang <[email protected]>
Co-authored-by: Ryan Liang <[email protected]>
Co-authored-by: Craig Perkins <[email protected]>
Co-authored-by: Yan Zeng <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants