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

[Canvas] Simplify module level services #194050

Closed
Tracked by #154677
Heenawter opened this issue Sep 25, 2024 · 1 comment · Fixed by #194634
Closed
Tracked by #154677

[Canvas] Simplify module level services #194050

Heenawter opened this issue Sep 25, 2024 · 1 comment · Fixed by #194634
Assignees
@Heenawter
Labels
Feature:Canvas impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. loe:medium Medium Level of Effort Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas technical debt Improvement of the software architecture and operational architecture

Comments

@Heenawter
Copy link
Contributor

Problem
The Canvas plugin currently uses an overcomplicated services layer which requires us to re-state all dependencies, their types and their implementations for stubbed usage, for storybooks (which are mostly unused), and for Kibana.

Solution
Instead of the Presentation Util module services, we should standardize on the approach used in the Embeddables plugin. See src/plugins/embeddable/public/kibana_services.ts for more information.
@Heenawter Heenawter added Feature:Canvas impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. loe:small Small Level of Effort Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas technical debt Improvement of the software architecture and operational architecture labels Sep 25, 2024
@elasticmachine
Copy link
Contributor

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

@Heenawter Heenawter mentioned this issue Sep 25, 2024
Closed
@Heenawter Heenawter self-assigned this Sep 26, 2024
@Heenawter Heenawter mentioned this issue Oct 1, 2024
Merged
2 tasks
@Heenawter Heenawter added loe:medium Medium Level of Effort and removed loe:small Small Level of Effort labels Oct 7, 2024
Heenawter added a commit that referenced this issue Oct 8, 2024
@Heenawter @cqliu1
[Canvas] Cleanup services (#194634)
91c045d 
Closes #194050

## Summary

This PR refactors the Canvas services to no longer use the
`PluginServiceProvider` from the `PresentationUtil` plugin. Note that
the Canvas storybooks are broken on main (and they have been for who
knows how long) and so, while I did make some changes to the storybooks
to make them **compile**, I didn't bother to get them fully functional.

Note that the Ecommerce workpad is broken - this is not due to this PR,
it is a [bug](#195297) that is
present on main.

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)


<!--ONMERGE {"backportTargets":["8.x"]} ONMERGE-->

---------

Co-authored-by: Catherine Liu <catherine.liu@elastic.co>
@Heenawter Heenawter mentioned this issue Oct 9, 2024
Merged
Heenawter added a commit to Heenawter/kibana that referenced this issue Oct 9, 2024
@Heenawter
[Canvas] Cleanup services (elastic#194634)
82fa144 
Closes elastic#194050

## Summary

This PR refactors the Canvas services to no longer use the
`PluginServiceProvider` from the `PresentationUtil` plugin. Note that
the Canvas storybooks are broken on main (and they have been for who
knows how long) and so, while I did make some changes to the storybooks
to make them **compile**, I didn't bother to get them fully functional.

Note that the Ecommerce workpad is broken - this is not due to this PR,
it is a [bug](elastic#195297) that is
present on main.

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

<!--ONMERGE {"backportTargets":["8.x"]} ONMERGE-->

---------

Co-authored-by: Catherine Liu <catherine.liu@elastic.co>
(cherry picked from commit 91c045d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Heenawter Heenawter

Labels
Feature:Canvas impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. loe:medium Medium Level of Effort Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas technical debt Improvement of the software architecture and operational architecture
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Canvas] Cleanup services Heenawter/kibana
2 participants
@Heenawter @elasticmachine