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

Add Disabled column to user table so individual users can be blocked. #1806

Merged
merged 5 commits into from Mar 6, 2019

Conversation

3 participants
@Igarfinkle
Copy link
Contributor

Igarfinkle commented Mar 1, 2019

Description

A couple of users are annoyingly stress-testing our system, so we're adding a functionality to disable individual users.

Code Review Verification Steps

  • Code follows the guidelines for Logging
  • The requirements listed in
    Querying the Database Safely
    have been satisfied.
  • Any new migrations/schema changes:
  • There are no aXe warnings for UI.
  • This works in IE.
  • Request review from a member of a different team.
  • Have the Pivotal acceptance criteria been met for this change?

References

@chrisgilmerproj
Copy link
Contributor

chrisgilmerproj left a comment

I think this looks great. You've got one broken test in dutystationsloader/duty_stations_loader_test.go because you changed the user model. You might also want to try to generate test data and see if there's any issue there. I'd also like to see a secure migration so we only modify the staging environment for that one user. But then this is probably ready to go.

Btw, I love that you split out the logic in the auth.go file. That's been needed for a while now.

@codecov

This comment has been minimized.

Copy link

codecov bot commented Mar 4, 2019

Codecov Report

Merging #1806 into master will decrease coverage by 0.01%.
The diff coverage is 5.45%.

@@            Coverage Diff            @@
##           master   #1806      +/-   ##
=========================================
- Coverage   49.42%   49.4%   -0.02%     
=========================================
  Files         427     427              
  Lines       18328   18347      +19     
  Branches     1631    1631              
=========================================
+ Hits         9058    9064       +6     
- Misses       8473    8486      +13     
  Partials      797     797
@@ -0,0 +1 @@
exec("./apply-secure-migration.sh 20190304210740_disable-unwanted-staging-user.sql")

This comment has been minimized.

@chrisgilmerproj

chrisgilmerproj Mar 6, 2019

Contributor

I ❤️ describing them as "unwanted"

@@ -8,9 +8,12 @@ import (
"reflect"
"time"

"github.com/honeycombio/beeline-go/trace"
"github.com/markbates/goth"

This comment has been minimized.

@chrisgilmerproj

chrisgilmerproj Mar 6, 2019

Contributor

Drop this newline and it should alls ort correctly.

@chrisgilmerproj
Copy link
Contributor

chrisgilmerproj left a comment

🚀 - This is great! Thanks for handling this so quickly. Solid improvements to the auth package too, makes it much easier to read.

@Igarfinkle Igarfinkle force-pushed the roc-ig-#164282562-add-disable-user branch from fc2d5b0 to 3d4055d Mar 6, 2019

@Igarfinkle Igarfinkle merged commit b2be02e into master Mar 6, 2019

17 of 19 checks passed

codecov/patch 5.45% of diff hit (target 49.42%)
Details
codecov/project 49.4% (-0.02%) compared to 0a60bb0
Details
Codacy/PR Quality Review Up to standards. A positive pull request.
Details
ci/circleci: acceptance_tests_experimental Your tests passed on CircleCI!
Details
ci/circleci: acceptance_tests_local Your tests passed on CircleCI!
Details
ci/circleci: acceptance_tests_staging Your tests passed on CircleCI!
Details
ci/circleci: build_app Your tests passed on CircleCI!
Details
ci/circleci: build_migrations Your tests passed on CircleCI!
Details
ci/circleci: build_tools Your tests passed on CircleCI!
Details
ci/circleci: client_test Your tests passed on CircleCI!
Details
ci/circleci: client_test_coverage Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_mymove Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_office Your tests passed on CircleCI!
Details
ci/circleci: integration_tests_tsp Your tests passed on CircleCI!
Details
ci/circleci: pre_deps_golang Your tests passed on CircleCI!
Details
ci/circleci: pre_deps_yarn Your tests passed on CircleCI!
Details
ci/circleci: pre_test Your tests passed on CircleCI!
Details
ci/circleci: server_test Your tests passed on CircleCI!
Details
ci/circleci: server_test_coverage Your tests passed on CircleCI!
Details

@Igarfinkle Igarfinkle deleted the roc-ig-#164282562-add-disable-user branch Mar 6, 2019

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