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

[Alerting] fix labels and links in PagerDuty action ui and docs #64032

Merged
merged 2 commits into from
May 4, 2020
Merged

[Alerting] fix labels and links in PagerDuty action ui and docs #64032

merged 2 commits into from
May 4, 2020

Conversation

pmuellr
Copy link
Member

@pmuellr pmuellr commented Apr 21, 2020 •
edited
Loading

resolves #63222, resolves #63768, resolves #63223

ui changes:

  • adds an "(optional)" label after the API URL label
  • changes help link to go to alerting docs and not watcher docs
  • changes the label "Routing key" to "Integration key" to match other docs
  • changes the order of the severity options to match other docs

doc changes:

  • changes the reference of "Routing key" to "Integration key" to match other docs
  • makes clearer that the API URL is optional

Checklist

@pmuellr
[Alerting] fix labels and links in PagerDuty action ui and docs
ae2d51c 
resolves elastic#63222, resolves elastic#63768, resolves elastic#63223

ui changes:

- adds an "(optional)" label after the API URL label
- changes help link to go to alerting docs and not watcher docs
- changes the label "Routing key" to "Integration key" to match other docs
- changes the order of the severity options to match other docs

doc changes:

- changes the reference of "Routing key" to "Integration key" to match other docs
- makes clearer that the API URL is optional
@pmuellr pmuellr added Feature:Alerting v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.8.0 v7.7.1 labels Apr 21, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

pmuellr
pmuellr commented Apr 21, 2020
View reviewed changes
...plugins/triggers_actions_ui/public/application/components/builtin_action_types/pagerduty.tsx
@@ -175,20 +175,20 @@ const PagerDutyParamsFields: React.FunctionComponent<ActionParamsProps<PagerDuty
} = actionParams;
const severityOptions = [
{
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 changes below are a re-ordering of the critical/error/warning/info severities, which changes the order displayed in the UI to match the same ordering I've seen on PD's site.

@pmuellr pmuellr marked this pull request as ready for review April 21, 2020 13:51
@pmuellr pmuellr requested a review from a team as a code owner April 21, 2020 13:51
YulNaumenko
YulNaumenko approved these changes Apr 21, 2020
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

@gmmorris gmmorris self-requested a review April 21, 2020 14:36
gmmorris
gmmorris approved these changes Apr 21, 2020
View reviewed changes
Copy link
Contributor

@gmmorris gmmorris left a comment

Choose a reason for hiding this comment

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

LGTM.
While we're there it might be worth considering ways of making the Summery field stand out (it's weird to me having optional fields with one required field in the middle that's easily overlooked), but it doesn't have to be in this PR. 🤷

@pmuellr
Copy link
Member Author

pmuellr commented Apr 21, 2020

making the Summery field stand out

I noticed that as well, and it bugged me enough I had a very, very brief thought about fixing that, but ... I'll open a new issue instead :-)

@pmuellr
Copy link
Member Author

pmuellr commented Apr 21, 2020

I plan on waiting to merge this to 7.7, 7.x, and master once the 7.7.0 tag is cut; it's going to be confusing to me to only merge it to some branches right now, and I think the potential for conflicting changes in between is small, and price to fix conflicts should be small anyway.

@pmuellr
Copy link
Member Author

pmuellr commented Apr 21, 2020

I just opened #64112 for the re-arrangement of the Summary field that Gidi mentioned ^^^

@pmuellr
Copy link
Member Author

pmuellr commented May 4, 2020

@elasticmachine merge upstream

@pmuellr pmuellr merged commit cb00e5e into elastic:master May 4, 2020
pmuellr added a commit to pmuellr/kibana that referenced this pull request May 4, 2020
@pmuellr
[Alerting] fix labels and links in PagerDuty action ui and docs (elas…
988d9d4 
…tic#64032)

resolves elastic#63222, resolves elastic#63768, resolves elastic#63223

ui changes:

- adds an "(optional)" label after the API URL label
- changes help link to go to alerting docs and not watcher docs
- changes the label "Routing key" to "Integration key" to match other docs
- changes the order of the severity options to match other docs

doc changes:

- changes the reference of "Routing key" to "Integration key" to match other docs
- makes clearer that the API URL is optional
@pmuellr pmuellr mentioned this pull request May 4, 2020
Merged
gmmorris added a commit to gmmorris/kibana that referenced this pull request May 4, 2020
@gmmorris
Merge branch 'alerting/np-tests-migration' of github.com:gmmorris/kib…
85a6679 
…ana into alerting/np-tests-migration

* 'alerting/np-tests-migration' of github.com:gmmorris/kibana:
  [APM] Agent remote config: validation for Java agent configs (elastic#63956)
  [APM] Fix duplicate index patterns (elastic#64883)
  Drilldowns (elastic#61219)
  [Alerting] fix labels and links in PagerDuty action ui and docs (elastic#64032)
  [Event Log] Ensure sorting tests are less flaky (elastic#64781)
  update endpoint to restrict removing with datasources (elastic#64978)
  [Logs UI] [Alerting] Alerts management page enhancements (elastic#64654)
  Adjust kibana app owning files (elastic#65064)
  Migrate tutorial resources (elastic#64298)
  [Logs UI] Tweak copy in log alerts dialog (elastic#64645)
  [Logs UI] [Alerting] Documentation (elastic#64886)
  [Logs UI] Add dataset filter to ML module setup screen (elastic#64470)
  [TSVB] Fixing memory leak (elastic#64918)
  Bump backport to 5.4.1 (elastic#65041)
pmuellr added a commit that referenced this pull request May 4, 2020
@pmuellr
[Alerting] fix labels and links in PagerDuty action ui and docs (#64032…
71c14c6 
…) (#65075)

resolves #63222, resolves #63768, resolves #63223

ui changes:

- adds an "(optional)" label after the API URL label
- changes help link to go to alerting docs and not watcher docs
- changes the label "Routing key" to "Integration key" to match other docs
- changes the order of the severity options to match other docs

doc changes:

- changes the reference of "Routing key" to "Integration key" to match other docs
- makes clearer that the API URL is optional
@pmuellr
Copy link
Member Author

pmuellr commented May 4, 2020

I merged this to master and 7.x for 7.8 - I was originally planning on waiting till the 7.7.0 tag was created to merge to 7.7 for the 7.7.1 release, but it hasn't been created yet.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

pmuellr added a commit to pmuellr/kibana that referenced this pull request May 18, 2020
@pmuellr
[Alerting] fix labels and links in PagerDuty action ui and docs (elas…
f0d6c2e 
…tic#64032)

resolves elastic#63222, resolves elastic#63768, resolves elastic#63223

ui changes:

- adds an "(optional)" label after the API URL label
- changes help link to go to alerting docs and not watcher docs
- changes the label "Routing key" to "Integration key" to match other docs
- changes the order of the severity options to match other docs

doc changes:

- changes the reference of "Routing key" to "Integration key" to match other docs
- makes clearer that the API URL is optional
@pmuellr pmuellr mentioned this pull request May 18, 2020
Merged
pmuellr added a commit that referenced this pull request May 18, 2020
@pmuellr
[Alerting] fix labels and links in PagerDuty action ui and docs (#64032…
477ed19 
…) (#66895)

resolves #63222, resolves #63768, resolves #63223

ui changes:

- adds an "(optional)" label after the API URL label
- changes help link to go to alerting docs and not watcher docs
- changes the label "Routing key" to "Integration key" to match other docs
- changes the order of the severity options to match other docs

doc changes:

- changes the reference of "Routing key" to "Integration key" to match other docs
- makes clearer that the API URL is optional
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

@gmmorris gmmorris gmmorris approved these changes

Assignees
No one assigned
Labels
backported Feature:Alerting release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.7.1 v7.8.0 v8.0.0
Projects
None yet
Milestone
No milestone
5 participants
@pmuellr @elasticmachine @kibanamachine @gmmorris @YulNaumenko