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

Crash after trying to create a user with email still present in soft deleted scope #130

Closed
casaper opened this issue Jul 20, 2017 · 8 comments
Labels

Comments

@casaper
Copy link
Collaborator

casaper commented Jul 20, 2017

I, [2017-07-20T12:10:02.779412 #30468]  INFO -- : [83e9f9ad-240d-4f03-a939-95405ab4f0e1] Started GET "/users/new" for 212.51.155.234 at 2017-07-20 12:10:02 +0200
I, [2017-07-20T12:10:02.785684 #30468]  INFO -- : [83e9f9ad-240d-4f03-a939-95405ab4f0e1] Processing by UsersController#new as HTML
D, [2017-07-20T12:10:02.832463 #30468] DEBUG -- : [83e9f9ad-240d-4f03-a939-95405ab4f0e1]   User Load (1.1ms)  SELECT  "users".* FROM "users" WHERE "users"."deleted_at" IS NULL AND "users"."id" = $1 ORDER BY "users"."id" ASC LIMIT $2  [["id", 1], ["LIMIT", 1]]
I, [2017-07-20T12:10:02.877400 #30468]  INFO -- : [83e9f9ad-240d-4f03-a939-95405ab4f0e1]   Rendering users/new.html.slim within layouts/application
I, [2017-07-20T12:10:02.913437 #30468]  INFO -- : [83e9f9ad-240d-4f03-a939-95405ab4f0e1]   Rendered users/_form.html.slim (19.9ms)
I, [2017-07-20T12:10:02.913597 #30468]  INFO -- : [83e9f9ad-240d-4f03-a939-95405ab4f0e1]   Rendered users/new.html.slim within layouts/application (36.0ms)
D, [2017-07-20T12:10:02.941449 #30468] DEBUG -- : [83e9f9ad-240d-4f03-a939-95405ab4f0e1]   Profile Load (0.6ms)  SELECT  "profiles".* FROM "profiles" WHERE "profiles"."deleted_at" IS NULL AND "profiles"."user_id" = $1 LIMIT $2  [["user_id", 1], ["LIMIT", 1]]
D, [2017-07-20T12:10:02.970016 #30468] DEBUG -- : [83e9f9ad-240d-4f03-a939-95405ab4f0e1]   Contact Load (0.9ms)  SELECT  "contacts".* FROM "contacts" WHERE "contacts"."deleted_at" IS NULL AND "contacts"."contactable_id" = $1 AND "contacts"."contactable_type" = $2 LIMIT $3  [["contactable_id", 1], ["contactable_type", "Profile"], ["LIMIT", 1]]
I, [2017-07-20T12:10:02.992138 #30468]  INFO -- : [83e9f9ad-240d-4f03-a939-95405ab4f0e1]   Rendered application/_navigation.html.slim (69.2ms)
I, [2017-07-20T12:10:02.998588 #30468]  INFO -- : [83e9f9ad-240d-4f03-a939-95405ab4f0e1]   Rendered application/_notification.html.slim (4.0ms)
I, [2017-07-20T12:10:03.004099 #30468]  INFO -- : [83e9f9ad-240d-4f03-a939-95405ab4f0e1]   Rendered application/_footer.html.slim (3.4ms)
I, [2017-07-20T12:10:03.004604 #30468]  INFO -- : [83e9f9ad-240d-4f03-a939-95405ab4f0e1] Completed 200 OK in 219ms (Views: 121.6ms | ActiveRecord: 24.5ms)
I, [2017-07-20T12:10:14.441777 #30473]  INFO -- : [9c00d704-2dc4-43bf-8f17-c92259ab755b] Started POST "/users" for 212.51.155.234 at 2017-07-20 12:10:14 +0200
I, [2017-07-20T12:10:14.443861 #30473]  INFO -- : [9c00d704-2dc4-43bf-8f17-c92259ab755b] Processing by UsersController#create as HTML
I, [2017-07-20T12:10:14.443946 #30473]  INFO -- : [9c00d704-2dc4-43bf-8f17-c92259ab755b]   Parameters: {"utf8"=>"✓", "authenticity_token"=>"YzjjiXqC+ZnkRQr8Q2CInZ863bAqxXSPt/9seA/YikIsYXhf5Z89HRTQEax/OkzNVkYe2vPeibxv9B1UsVTYAw==", "user"=>{"email"=>"vok@panter.ch", "role"=>"superadmin"}, "commit"=>"Benutzer/in erstellen"}
D, [2017-07-20T12:10:14.447265 #30473] DEBUG -- : [9c00d704-2dc4-43bf-8f17-c92259ab755b]   User Load (0.5ms)  SELECT  "users".* FROM "users" WHERE "users"."deleted_at" IS NULL AND "users"."id" = $1 ORDER BY "users"."id" ASC LIMIT $2  [["id", 1], ["LIMIT", 1]]
D, [2017-07-20T12:10:14.653239 #30473] DEBUG -- : [9c00d704-2dc4-43bf-8f17-c92259ab755b]    (0.2ms)  BEGIN
D, [2017-07-20T12:10:14.655953 #30473] DEBUG -- : [9c00d704-2dc4-43bf-8f17-c92259ab755b]   User Exists (0.4ms)  SELECT  1 AS one FROM "users" WHERE "users"."email" = $1 AND "users"."deleted_at" IS NULL LIMIT $2  [["email", "vok@panter.ch"], ["LIMIT", 1]]
D, [2017-07-20T12:10:14.659131 #30473] DEBUG -- : [9c00d704-2dc4-43bf-8f17-c92259ab755b]   SQL (1.1ms)  INSERT INTO "users" ("email", "encrypted_password", "role", "created_at", "updated_at") VALUES ($1, $2, $3, $4, $5) RETURNING "id"  [["email", "vok@panter.ch"], ["encrypted_password", "$2a$11$Pd1ofy9WgLdWDotPnMFQ6uMFecqrF9fr.4anXu9Wmj2q42ncFdb2W"], ["role", "superadmin"], ["created_at", "2017-07-20 10:10:14.656581"], ["updated_at", "2017-07-20 10:10:14.656581"]]
D, [2017-07-20T12:10:14.659612 #30473] DEBUG -- : [9c00d704-2dc4-43bf-8f17-c92259ab755b]    (0.1ms)  ROLLBACK
I, [2017-07-20T12:10:14.666416 #30473]  INFO -- : [9c00d704-2dc4-43bf-8f17-c92259ab755b] Completed 500 Internal Server Error in 222ms (ActiveRecord: 6.5ms)
F, [2017-07-20T12:10:14.667671 #30473] FATAL -- : [9c00d704-2dc4-43bf-8f17-c92259ab755b]
F, [2017-07-20T12:10:14.667731 #30473] FATAL -- : [9c00d704-2dc4-43bf-8f17-c92259ab755b] ActiveRecord::RecordNotUnique (PG::UniqueViolation: ERROR:  duplicate key value violates unique constraint "index_users_on_email"
DETAIL:  Key (email)=(vok@panter.ch) already exists.
: INSERT INTO "users" ("email", "encrypted_password", "role", "created_at", "updated_at") VALUES ($1, $2, $3, $4, $5) RETURNING "id"):
F, [2017-07-20T12:10:14.667775 #30473] FATAL -- : [9c00d704-2dc4-43bf-8f17-c92259ab755b]
F, [2017-07-20T12:10:14.667818 #30473] FATAL -- : [9c00d704-2dc4-43bf-8f17-c92259ab755b] app/controllers/users_controller.rb:22:in `create'
irb(main):002:0> User.where(email: 'vok@panter.ch').count
   (0.9ms)  SELECT COUNT(*) FROM "users" WHERE "users"."deleted_at" IS NULL AND "users"."email" = $1  [["email", "vok@panter.ch"]]
=> 0
@casaper casaper added the bug label Jul 20, 2017
@toupeira
Copy link
Contributor

We should add a uniqueness validation on users.email

@casaper
Copy link
Collaborator Author

casaper commented Jul 20, 2017

Instead of a unique index?

@toupeira
Copy link
Contributor

In addition to :) All unique indexes should have a corresponding validation, so the error can be caught earlier.

IMHO Rails should have integrated something like https://github.com/SchemaPlus/schema_validations long ago.

@casaper
Copy link
Collaborator Author

casaper commented Jul 20, 2017

Sounds like a plan.
But how are we going to handle the case a user is soft deleted and again created... with the same email?
Somewhat of an edge case, but not totally irrelevant. They may want to do that.

@casaper
Copy link
Collaborator Author

casaper commented Jul 20, 2017

how about using gem 'schema_validations' ?

@toupeira
Copy link
Contributor

But how are we going to handle the case a user is soft deleted and again created... with the same email?
Somewhat of an edge case, but not totally irrelevant. They may want to do that.

Yeah I was thinking about that too, I see a few different options:

  • automatically reactivate a deleted user on creation (including all their associated data)
  • block creation, let admins deal with reactivating the user
  • change email address on deletion (e.g. add +deleted@)

Definitely something to discuss with the customer :)

Was there a specific motivation for using the paranoia gem, besides actual paranoia?

@toupeira
Copy link
Contributor

how about using gem 'schema_validations' ?

I haven't used it yet but it looks solid, definitely something we could do if we have the time.

@casaper
Copy link
Collaborator Author

casaper commented Jul 20, 2017

automatically reactivate a deleted user on creation (including all their associated data)

That looks like the best solution to me. Especially because the newly recreated user will also have all its relations still in place.

@casaper casaper closed this as completed Aug 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants