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

Feature: get contact information from a user's page #85

Merged
merged 11 commits into from Aug 1, 2019

Conversation

@tlwr
Copy link
Member

commented Aug 1, 2019

This PR is based off #84, once that is merged I will rebase. It will help you test it out locally though.

User stories

As someone who is on a rota, I need to get the contact details for someone on a different rota, so I can contact them in an emergency.

Currently only Pagerduty global admins can see everyone in Pagerduty, which means not everyone who is on-call can contact everyone else.

Not everyone has their People Finder or Google Contacts up to date. We are much more strict about ensuring Pagerduty is up to date.

As someone who is NOT on a rota, I need to get the contact details for someone who is presently on-call, so I can contact them in an emergency.

There are a number of stakeholders who do not have Pagerduty accounts (because they are expensive), who are currently able to see who is currently on call, using the rotas app.

However they are unable to contact the people who are currently on the rota because they do not have Pagerduty accounts, nor would they unless they were Pagerduty global admins.

Other solutions to this problem have been tried:

  • Giving the stakeholders Pagerduty accounts
    • Vetoed because expensive
    • Stakeholders did not accept invites, or could not use Pagerduty because it is not user-friendly
  • The vexatious distribution of spreadsheets, where on-call engineers were expected to put their phone numbers
    • These went partially unfilled
    • People forgot to update the sheets, or could not find the sheets
    • IA position unclear on whether copious amounts of contact data should be stored in Google drive

This can be solved by reading from Pagerduty directly and not storing any of the PII.

As someone who works in information assurance, I need to know who accessed someone's contact details, for audit and compliance reasons.

(This is allegedly a user need, but I haven't confirmed the actual need)

What

Adds a button to a user's page

Screen Shot 2019-08-01 at 10 20 33
Image of "Get contact information from Pagerduty" button, on the existing show user page_

Screen Shot 2019-08-01 at 10 25 52
Image of contact information table, on the new page

Consequences

Anyone with a @digital.cabinet-office.gov.uk email address will be able to get the contact details of anyone who is in Pagerduty.

I personally think this is beneficial to the organisation.

We have an additional table in the database

It is for audit events, should be fine

The rotas application will have read-only access to Pagerduty via the API

This is necessary for fulfilling the user need

How to review

Code review

Run the tests: bundle install && bundle exec rake test

Test it out locally:

  • Generate a Pagerduty API key and put it in your environment PAGERDUTY_API_TOKEN
  • Navigate to toby.lornewelch-richards@digital.cabinet-office.gov.uk/contact-info
  • Steal my phone number
  • ???
  • Success

@tlwr tlwr requested review from issyl0, patrickGDS and stephengrier Aug 1, 2019

@tlwr tlwr force-pushed the pagerduty-get-contacts branch from 88c9340 to fd0350c Aug 1, 2019

tlwr added some commits Jul 31, 2019

Add audit event model and migrations
Signed-off-by: Toby Lorne <toby.lornewelch-richards@digital.cabinet-office.gov.uk>
add name format helper function
when email is too long and gross

Signed-off-by: Toby Lorne <toby.lornewelch-richards@digital.cabinet-office.gov.uk>
add model and validation for audit events
Signed-off-by: Toby Lorne <toby.lornewelch-richards@digital.cabinet-office.gov.uk>
Always set current user even if DISABLE_AUTH
so things get audited in development and test

Signed-off-by: Toby Lorne <toby.lornewelch-richards@digital.cabinet-office.gov.uk>
Add contact information page (no content yet)
which is audited

Signed-off-by: Toby Lorne <toby.lornewelch-richards@digital.cabinet-office.gov.uk>
Write test for getting contact details from PD
Signed-off-by: Toby Lorne <toby.lornewelch-richards@digital.cabinet-office.gov.uk>
Implement pagerduty api get contact details
Signed-off-by: Toby Lorne <toby.lornewelch-richards@digital.cabinet-office.gov.uk>
Bump govuk-frontend-> 3.0 so contact table renders
Signed-off-by: Toby Lorne <toby.lornewelch-richards@digital.cabinet-office.gov.uk>
Test failure case for pagerduty contact details
Signed-off-by: Toby Lorne <toby.lornewelch-richards@digital.cabinet-office.gov.uk>
@howardstaple

This comment has been minimized.

Copy link

commented Aug 1, 2019

IA are content for this to go ahead.

@issyl0
Copy link
Member

left a comment

LGTM, bonus points for tests.

I have some requested changes about PagerDuty capitalisation, apart from that this is 💯.

app/controllers/users_controller.rb Outdated Show resolved Hide resolved
app/views/users/contact_information.html.erb Outdated Show resolved Hide resolved
app/views/users/show.html.erb Outdated Show resolved Hide resolved
app/views/users/show.html.erb Outdated Show resolved Hide resolved
test/controllers/users_controller_test.rb Outdated Show resolved Hide resolved

tlwr added some commits Aug 1, 2019

Rename Pagerduty to PagerDuty
Signed-off-by: Toby Lorne <toby.lornewelch-richards@digital.cabinet-office.gov.uk>
Update view contact details audit message
If we expand audit events to other things then making this more
descriptive will be useful

Signed-off-by: Toby Lorne <toby.lornewelch-richards@digital.cabinet-office.gov.uk>
@issyl0

issyl0 approved these changes Aug 1, 2019

@tlwr tlwr merged commit 090525f into master Aug 1, 2019

@tlwr tlwr deleted the pagerduty-get-contacts branch Aug 1, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.