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

[Shareable Dashboards] Make image embeddable shareable #169870

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

cqliu1
Copy link
Contributor

@cqliu1 cqliu1 commented Oct 25, 2023

Summary

Note: this PR is merging into this Feature Branch: #168166
This will be merged into main now.
Blocked by #171461.
Closes #170770.

This enables sharing for image embeddables and file saved objects.

Copy to spaces and import/export of a file saved object doesn't currently work and is not within the scope of this PR.

Checklist

Delete any items that are not applicable to this PR.

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk Probability Severity Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space. Low High Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks. High Low Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled. Medium High Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

@cqliu1 cqliu1 force-pushed the shareable-images branch 2 times, most recently from 5d30c54 to ca61a8e Compare October 25, 2023 20:17
@cqliu1 cqliu1 added Project:ShareToSpace Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) Feature:Dashboard Dashboard related features labels Nov 1, 2023
getTitle(obj) {
return `${obj.attributes.name}.${obj.attributes.extension}`;
},
importableAndExportable: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Currently files don't support export/import, as besides exporting the metadata saved object we would also need to copy the file blob contents. From my understanding it can be achieved, but we would need to implement that first.

Copy link
Contributor Author

@cqliu1 cqliu1 Nov 7, 2023

Choose a reason for hiding this comment

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

I'm going to set importableAndExportable: false and won't be tackling support for import/export as part of this PR/sharing dashboards feature. After testing, it seems the combination of hidden: true and importableAndExportable: false allows files to be shared but prevents file objects from appearing in the Saved Object Management page.

Copy link
Member

Choose a reason for hiding this comment

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

Since file objects are hidden from the Saved Object Management page, there is no way in the UI to share files to other Spaces. To resolve this we need to add the "Spaces" column to the filesManagement page. I have a PoC PR that shows how we could eventually implement that feature. But I don't see why that should block this PR going into the feature branch.

@cqliu1 cqliu1 requested a review from nickpeihl November 9, 2023 18:56
@cqliu1 cqliu1 marked this pull request as ready for review November 9, 2023 18:56
@cqliu1 cqliu1 requested review from a team as code owners November 9, 2023 18:56
@elasticmachine
Copy link
Contributor

Pinging @elastic/appex-sharedux (Team:SharedUX)

Copy link
Contributor

@rshen91 rshen91 left a comment

Choose a reason for hiding this comment

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

Shared ux changes LGTM - code review only

Copy link
Member

@nickpeihl nickpeihl left a comment

Choose a reason for hiding this comment

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

lgtm!

code review and tested sharing files to multiple spaces with image embeddable

type: 'image',
};

expect(inject!(state, references)).toEqual(result);
Copy link
Member

Choose a reason for hiding this comment

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

nice tests! 💯

@cqliu1 cqliu1 changed the base branch from shareable-dashboards to main November 20, 2023 23:40
@cqliu1 cqliu1 requested review from a team as code owners November 20, 2023 23:40
@cqliu1 cqliu1 changed the base branch from main to shareable-dashboards November 20, 2023 23:41
@cqliu1 cqliu1 removed request for a team November 20, 2023 23:41
@cqliu1 cqliu1 added the blocked label Nov 21, 2023
@kibanamachine kibanamachine added the Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas label Nov 21, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

@cqliu1 cqliu1 marked this pull request as draft November 21, 2023 20:44
@cqliu1
Copy link
Contributor Author

cqliu1 commented Nov 21, 2023

Sorry for the pings everyone. I rebased this PR and am targeting main as a full feature independent of the shareable dashboard work. Instead of merging this as is, I'm waiting until #169870 is done, so we can enable sharing on the files management page and follow up with additional functional tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Feature:Dashboard Dashboard related features Project:ShareToSpace Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Shareable dashboards] Make image embeddable shareable
9 participants