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

Introduce saved object plugin into maps plugin #67

Merged
merged 1 commit into from
Oct 28, 2022
Merged

Introduce saved object plugin into maps plugin #67

merged 1 commit into from
Oct 28, 2022

Conversation

junqiu-lei
Copy link
Member

@junqiu-lei junqiu-lei commented Oct 26, 2022 •
edited
Loading

Signed-off-by: Junqiu Lei [email protected]

Description

  • Introduce saved objects plugin so that maps object can be stored into OpenSearch.
  • Able to fetch maps data into list page.
  • Add top nav menu component and save modal.
  • Able to click save button to save object with custom tittle and description.
  • Able to delete maps in map list.
  • Use OpenSearchDashboardsContextProvider to pass down OSD services to a service object.

After this PR merged, will integrate layerlist, mapstate into store logic.

Issues Resolved

#68

Updated Demo

maps_store_with_saved_object_demo.mov

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.

@junqiu-lei junqiu-lei self-assigned this Oct 26, 2022
@junqiu-lei junqiu-lei requested a review from a team October 26, 2022 17:44
@junqiu-lei junqiu-lei changed the title integrate saved object plugin into maps plugin Introduce saved object plugin into maps plugin Oct 26, 2022
@VijayanB
Copy link
Member

@junqiu-lei Can you include demo to this PR? Thanks

@junqiu-lei
Copy link
Member Author

@junqiu-lei Can you include demo to this PR? Thanks

Yes, just added the demo on using saved_object plugin to store maps object. Thanks

@junqiu-lei junqiu-lei requested a review from ashwin-pc October 26, 2022 18:52
@ashwin-pc
Copy link
Member

@junqiu-lei thanks for the recording. Very helpful with the review. A few high level questions:

  1. Is the entrypoint for the map permanently from the side nav or is it just temporarily there for now?
  2. In the saved object management plugin, you also have the option to inspect the underlying saved object data too. Did you look into that?
  3. Any particular reason you arent reusing the Visualizations List and are creating your own listing page?

@junqiu-lei
Copy link
Member Author

junqiu-lei commented Oct 26, 2022 •
edited
Loading

@junqiu-lei thanks for the recording. Very helpful with the review. A few high level questions:

  1. Is the entrypoint for the map permanently from the side nav or is it just temporarily there for now?
  2. In the saved object management plugin, you also have the option to inspect the underlying saved object data too. Did you look into that?
  3. Any particular reason you arent reusing the Visualizations List and are creating your own listing page?

Just offline talked with @ashwin-pc, great thanks on the feedback.

  1. Yes, it follows UX design to entry from side nav
  2. Good idea, will check to add.
  3. Because from question 1, the entrypoint for maps is side nav, so it need its own listing page. But later I will raise PR to also enable use Visualization entrypoint to create and list map.

@junqiu-lei junqiu-lei requested a review from navneet1v October 27, 2022 00:30
ashwin-pc
ashwin-pc reviewed Oct 27, 2022
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.

Nice! Overall i'm good with the implementation, just a few comments

maps_dashboards/public/components/app.tsx Outdated Show resolved Hide resolved
maps_dashboards/public/components/map_page/map_page.tsx Outdated Show resolved Hide resolved
maps_dashboards/public/components/map_page/map_page.tsx Outdated Show resolved Hide resolved
maps_dashboards/public/components/maps_list/maps_list.tsx Outdated Show resolved Hide resolved
maps_dashboards/server/saved_objects/map_saved_object.ts Show resolved Hide resolved
maps_dashboards/server/saved_objects/map_saved_object.ts Outdated Show resolved Hide resolved
maps_dashboards/server/saved_objects/map_saved_object.ts Show resolved Hide resolved
maps_dashboards/public/application.tsx Outdated Show resolved Hide resolved
@junqiu-lei junqiu-lei force-pushed the store_with_saved_object branch from 80069ad to d7270dc Compare October 28, 2022 17:01
@junqiu-lei junqiu-lei requested review from ashwin-pc, abbyhu2000, VijayanB and a team October 28, 2022 17:01
VijayanB
VijayanB previously approved these changes Oct 28, 2022
View reviewed changes
Copy link
Member

@VijayanB VijayanB left a comment

Choose a reason for hiding this comment

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

LGTM

ashwin-pc
ashwin-pc reviewed Oct 28, 2022
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.

Looks much better! Thanks! Just a few minor comments.

maps_dashboards/public/components/map_top_nav/get_top_nav_config.tsx Outdated Show resolved Hide resolved
maps_dashboards/public/components/map_top_nav/get_top_nav_config.tsx Show resolved Hide resolved
maps_dashboards/public/components/maps_list/maps_list.tsx Show resolved Hide resolved
maps_dashboards/server/saved_objects/map_saved_object.ts Outdated Show resolved Hide resolved
@junqiu-lei junqiu-lei force-pushed the store_with_saved_object branch from d7270dc to f56cc6d Compare October 28, 2022 19:02
@junqiu-lei junqiu-lei force-pushed the store_with_saved_object branch from f56cc6d to 4049c3a Compare October 28, 2022 19:25
ashwin-pc
ashwin-pc approved these changes Oct 28, 2022
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.

Thanks for making those changes!

abbyhu2000
abbyhu2000 approved these changes Oct 28, 2022
View reviewed changes
Copy link
Member

@abbyhu2000 abbyhu2000 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!

maps_dashboards/server/saved_objects/capabilities_provider.ts Show resolved Hide resolved
maps_dashboards/public/components/map_top_nav/get_top_nav_config.tsx Show resolved Hide resolved
maps_dashboards/public/components/map_top_nav/get_top_nav_config.tsx Show resolved Hide resolved
@junqiu-lei junqiu-lei merged commit 61933bb into opensearch-project:feature/new-maps Oct 28, 2022
@junqiu-lei junqiu-lei added the v2.5.0 'Issues and PRs related to version v2.5.0' label Oct 28, 2022
@junqiu-lei junqiu-lei deleted the store_with_saved_object branch February 23, 2023 23:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abbyhu2000 abbyhu2000 abbyhu2000 approved these changes

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

@VijayanB VijayanB VijayanB left review comments

@navneet1v navneet1v Awaiting requested review from navneet1v

Assignees

@junqiu-lei junqiu-lei

Labels
feature maps-dashboards v2.5.0 'Issues and PRs related to version v2.5.0'
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@junqiu-lei @VijayanB @ashwin-pc @navneet1v @abbyhu2000