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

[Maps] saved object tagging #83197

Merged
merged 18 commits into from
Nov 17, 2020
Merged

[Maps] saved object tagging #83197

merged 18 commits into from
Nov 17, 2020

Conversation

nreese
Copy link
Contributor

@nreese nreese commented Nov 11, 2020

Closes #82434

This PR adds saved object tagging to listing page and save modal

Screen Shot 2020-11-11 at 11 11 54 AM

Screen Shot 2020-11-11 at 11 12 06 AM

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-gis (Team:Geo)

@nreese
Copy link
Contributor Author

nreese commented Nov 11, 2020

@elasticmachine merge upstream

@nreese nreese requested review from a team as code owners November 12, 2020 01:40
Copy link
Contributor

@pgayvallet pgayvallet left a comment

Choose a reason for hiding this comment

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

That's great to see tagging implemented in another app!

Some comments:

Could you update your feature to add the tag SO read permission. Else a user would not be able to properly use tags when he's only granted permissions to see map

features.registerKibanaFeature({
id: APP_ID,
name: i18n.translate('xpack.maps.featureRegistry.mapsFeatureName', {
defaultMessage: 'Maps',
}),

See

savedObject: {
all: ['visualization', 'query', 'lens'],
read: ['index-pattern', 'search', 'tag'],

Could you update the RBAC integration tests to add a new map user

See

USERS.DEFAULT_SPACE_DASHBOARD_READ_USER,
USERS.DEFAULT_SPACE_VISUALIZE_READ_USER,

and

DEFAULT_SPACE_VISUALIZE_READ_USER: {
username: 'a_kibana_rbac_default_space_visualize_read_user',
password: 'password',
roles: [ROLES.KIBANA_RBAC_DEFAULT_SPACE_VISUALIZE_READ_USER.name],
},

(Optional) Could you add a FTR test for tag integration within map

As it is done for dash and vis in x-pack/test/saved_object_tagging/functional/tests/dashboard_integration.ts and x-pack/test/saved_object_tagging/functional/tests/visualize_integration.ts

@pgayvallet pgayvallet added the Feature:Saved Object Tagging Saved Objects Tagging feature label Nov 12, 2020
@alexfrancoeur
Copy link

🥳

Copy link
Member

@lukeelmers lukeelmers left a comment

Choose a reason for hiding this comment

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

app arch code changes LGTM

@nreese nreese requested a review from a team as a code owner November 12, 2020 19:20
@nreese
Copy link
Contributor Author

nreese commented Nov 12, 2020

@pgayvallet

Could you update your feature to add the tag SO read permission. Else a user would not be able to properly use tags when he's only granted permissions to see map

Add tag to features registration

Could you update the RBAC integration tests to add a new map user

Added RBAC integration test for map read only user

Could you add a FTR test for tag integration within map

Added functional test for maps integration testing listing, create, and edit

@nreese nreese requested a review from pgayvallet November 12, 2020 19:23
Copy link
Contributor

@pgayvallet pgayvallet left a comment

Choose a reason for hiding this comment

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

LGTM once these two points are addressed

@nreese
Copy link
Contributor Author

nreese commented Nov 12, 2020

@elasticmachine merge upstream

@nreese
Copy link
Contributor Author

nreese commented Nov 16, 2020

@elasticmachine merge upstream

Copy link
Contributor

@thomasneirynck thomasneirynck left a comment

Choose a reason for hiding this comment

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

lgtm!

@nreese
Copy link
Contributor Author

nreese commented Nov 16, 2020

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
maps 2.8MB 2.8MB -5.4KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
kibanaReact 131.9KB 131.9KB +38.0B
maps 150.7KB 150.9KB +157.0B
total +195.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@nreese nreese merged commit a2c91f1 into elastic:master Nov 17, 2020
nreese added a commit to nreese/kibana that referenced this pull request Nov 17, 2020
* add tag selector to save modal

* save tag references onSave

* populate tags when unwrapping attributes

* tslint

* update listing page to show tags

* fix data-test-subj id in functional tests

* i18n cleanup

* tslint

* remove unused import

* use listingTable service for functional tests

* tslint and fix mvt grid layer functional test

* review feedback

* add tags to all privileges and add test user to find, delete, get, get_all, and update tests

* move functions to module scope

Co-authored-by: Kibana Machine <[email protected]>
nreese added a commit that referenced this pull request Nov 18, 2020
* [Maps] saved object tagging (#83197)

* add tag selector to save modal

* save tag references onSave

* populate tags when unwrapping attributes

* tslint

* update listing page to show tags

* fix data-test-subj id in functional tests

* i18n cleanup

* tslint

* remove unused import

* use listingTable service for functional tests

* tslint and fix mvt grid layer functional test

* review feedback

* add tags to all privileges and add test user to find, delete, get, get_all, and update tests

* move functions to module scope

Co-authored-by: Kibana Machine <[email protected]>

* mappings.json

* add map saved object mapping

Co-authored-by: Kibana Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation Feature:Saved Object Tagging Saved Objects Tagging feature release_note:enhancement v7.11.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Maps] add support for SavedObjects tagging
7 participants