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

[Connectors][ServiceNow] Update store links #117374

Merged
merged 5 commits into from
Nov 8, 2021
Merged

[Connectors][ServiceNow] Update store links #117374

merged 5 commits into from
Nov 8, 2021

Conversation

cnasikas
Copy link
Member

@cnasikas cnasikas commented Nov 3, 2021

Summary

This PR updates the ServiceNow Store URLs for the Elastic applications.

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@cnasikas cnasikas self-assigned this Nov 3, 2021
@cnasikas cnasikas requested a review from a team as a code owner November 3, 2021 16:20
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-threat-hunting-cases (Team:Threat Hunting:Cases)

@cnasikas
Copy link
Member Author

cnasikas commented Nov 3, 2021

@elasticmachine merge upstream

YulNaumenko
YulNaumenko reviewed Nov 3, 2021
View reviewed changes
x-pack/plugins/actions/server/builtin_action_types/servicenow/config.ts Outdated
@@ -26,13 +26,15 @@ export const snExternalServiceConfig: SNProductsConfig = {
table: 'incident',
useImportAPI: ENABLE_NEW_SN_ITSM_CONNECTOR,
commentFieldKey: 'work_notes',
appId: '7148dbc91bf1f450ced060a7234bcb88',
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it make sense to create a constants for this ids? Is it any chance this id could be changed?

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right. When the applications are published the ids will stay the same forever. There is a very rare occasion where they can change on publish.

YulNaumenko
YulNaumenko reviewed Nov 3, 2021
View reviewed changes
x-pack/plugins/actions/server/builtin_action_types/servicenow/types.ts
@@ -253,6 +253,7 @@ export interface SNProductsConfigValue {
useImportAPI: boolean;
importSetTable: string;
commentFieldKey: string;
appId?: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

Wonder why this is optional...

Copy link
Member Author

Choose a reason for hiding this comment

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

I make it optional because ITOM does not have an application.

YulNaumenko
YulNaumenko reviewed Nov 3, 2021
View reviewed changes
...application/components/builtin_action_types/servicenow/application_required_callout.test.tsx
describe('ApplicationRequiredCallout', () => {
test('it renders the callout', () => {
render(<ApplicationRequiredCallout />);
render(<ApplicationRequiredCallout appId={appId} />);
Copy link
Contributor

Choose a reason for hiding this comment

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

If this ids is something which will never change, then I think the better approach to avoid passing this as props and just use a constants

Copy link
Member Author

Choose a reason for hiding this comment

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

The reason I pass the id as a pros is that the component is used by both the ITSM & SecOps so it is different for each connector. Do you want me to do something different?

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for the explanation. It seems to be right.

@cnasikas cnasikas requested a review from YulNaumenko November 4, 2021 15:51
YulNaumenko
YulNaumenko approved these changes Nov 7, 2021
View reviewed changes
Copy link
Contributor

@YulNaumenko YulNaumenko left a comment

Choose a reason for hiding this comment

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

LGTM

@cnasikas
Copy link
Member Author

cnasikas commented Nov 8, 2021

@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
triggersActionsUi 777.0KB 777.6KB +684.0B

History

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

cc @cnasikas

@cnasikas cnasikas merged commit bbc50bc into elastic:main Nov 8, 2021
@cnasikas cnasikas deleted the sn_store_links branch November 8, 2021 16:01
@cnasikas cnasikas added the auto-backport Deprecated - use backport:version if exact versions are needed label Nov 8, 2021
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Nov 8, 2021
@cnasikas @kibanamachine
[Connectors][ServiceNow] Update store links (elastic#117374)
70212d9 
Co-authored-by: Kibana Machine <[email protected]>
@kibanamachine kibanamachine mentioned this pull request Nov 8, 2021
Merged
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Nov 8, 2021
@cnasikas @kibanamachine
[Connectors][ServiceNow] Update store links (elastic#117374)
5e10ba1 
Co-authored-by: Kibana Machine <[email protected]>
@kibanamachine kibanamachine mentioned this pull request Nov 8, 2021
Merged
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
✅ 8.0
✅ 7.16

The backport PRs will be merged automatically after passing CI.

@cnasikas cnasikas mentioned this pull request Nov 8, 2021
Closed
cnasikas added a commit to cnasikas/kibana that referenced this pull request Nov 8, 2021
@cnasikas @kibanamachine
[Connectors][ServiceNow] Update store links (elastic#117374)
74edab9 
Co-authored-by: Kibana Machine <[email protected]>
@cnasikas cnasikas removed the auto-backport Deprecated - use backport:version if exact versions are needed label Nov 8, 2021
kibanamachine added a commit that referenced this pull request Nov 8, 2021
@kibanamachine @cnasikas
[Connectors][ServiceNow] Update store links (#117374) (#117881)
202dfb5 
Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Christos Nasikas <[email protected]>
kibanamachine added a commit that referenced this pull request Nov 8, 2021
@kibanamachine @cnasikas
[Connectors][ServiceNow] Update store links (#117374) (#117882)
c6a7523 
Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Christos Nasikas <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@YulNaumenko YulNaumenko YulNaumenko approved these changes

Assignees

@cnasikas cnasikas

Labels
release_note:skip Skip the PR/issue when compiling release notes v7.16.0 v8.0.0 v8.1.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@cnasikas @elasticmachine @kibanamachine @YulNaumenko