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

Added additional information about user on each schedule view: phone … #126

Merged

Conversation

suhomozgy-andrey
Copy link
Contributor

@suhomozgy-andrey suhomozgy-andrey commented Jan 15, 2020

What's this PR do?

In this PR I added an additional request for user contact methods. To display user phone on each schedule page. It would be useful for example for peoples who don't have access to PagerDuty, but they should see user phone for any emergency case.

How should this be manually tested?

On each schedule panel user, the phone should appear if user have at least one phone contact method

Example screenshot

image

What are the relevant tickets?

Fixes #125

@codecov
Copy link

codecov bot commented Jan 15, 2020

Codecov Report

Merging #126 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master    #126   +/-   ##
======================================
  Coverage    7.27%   7.27%           
======================================
  Files          22      22           
  Lines         522     522           
======================================
  Hits           38      38           
  Misses        484     484

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 99f3817...3fde365. Read the comment docs.

@sergiitk
Copy link
Owner

Thank you for your contribution! I'm planning to get some work in next week, will review this and get it merged as well.

@sergiitk sergiitk self-requested a review January 16, 2020 15:06
Copy link
Owner

@sergiitk sergiitk left a comment

Choose a reason for hiding this comment

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

This looks very good! I just have one ask - let's make this feature enabled via environment variable PAGERBEAUTY_LOAD_USER_CONTACT_METHODS=true.
The reason for this is I asked around and looks like this use-case is not very common. Some folks even insisted they want to avoid showing their phone number without a reason.

@suhomozgy-andrey
Copy link
Contributor Author

@sergiitk sure. will add a key for that

Copy link
Owner

@sergiitk sergiitk 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 the contribution! I'll give it a couple of manual tests, and I'm hoping I'll merge soon.

@sergiitk sergiitk merged commit 0c51186 into sergiitk:master Jan 24, 2020
sergiitk pushed a commit that referenced this pull request Jan 25, 2020
# [1.1.0](v1.0.3...v1.1.0) (2020-01-25)

### Cleanup

- [134](#134) **io** Limit information exposed though contact methods

### Features

- [126](#126) **ui** Added optional user contact information to the schedule view
@sergiitk
Copy link
Owner

🎉 This PR is included in version 1.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add phone number to on-call person name
2 participants