From f776f863bb912877b33951647723c620ff32df7a Mon Sep 17 00:00:00 2001 From: kikito Date: Fri, 15 Apr 2016 16:29:01 +0200 Subject: [PATCH 01/11] Adds missing step to oauth spec --- spec/features/users_auth_spec.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/spec/features/users_auth_spec.rb b/spec/features/users_auth_spec.rb index a1c9374be..f9b92bb12 100644 --- a/spec/features/users_auth_spec.rb +++ b/spec/features/users_auth_spec.rb @@ -169,6 +169,12 @@ feature 'Users' do expect(current_path).to eq(finish_signup_path) + expect(page).to have_field('user_username', with: 'manuela') + + click_button 'Register' + + expect(current_path).to eq(do_finish_signup_path) + fill_in 'user_username', with: 'manuela2' click_button 'Register' From 3cd12bd6eade68d67687f654bb61683c75e8a45c Mon Sep 17 00:00:00 2001 From: kikito Date: Fri, 15 Apr 2016 16:29:57 +0200 Subject: [PATCH 02/11] Simplifies use of registering_with_oauth in User Now we just ignore all validations when saving a user for oauth --- app/models/user.rb | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index f7980c796..113781ef1 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -182,11 +182,11 @@ class User < ActiveRecord::Base end def username_required? - !organization? && !erased? && !registering_with_oauth + !organization? && !erased? end def email_required? - !erased? && !registering_with_oauth + !erased? end def has_official_email? @@ -215,11 +215,15 @@ class User < ActiveRecord::Base end def save_requiring_finish_signup - self.update(registering_with_oauth: true) - end - - def save_requiring_finish_signup_without_email - self.update(registering_with_oauth: true, email: nil) + begin + self.registering_with_oauth= true + self.save(validate: false) + # Devise puts unique constraints for the email the db, so we must detect & handle that + rescue ActiveRecord::RecordNotUnique + self.email = nil + self.save(validate: false) + end + true end def ability From f86912c77c7bd8f51ba9e1bf3ad0531f0ccc1c58 Mon Sep 17 00:00:00 2001 From: kikito Date: Fri, 15 Apr 2016 16:32:42 +0200 Subject: [PATCH 03/11] Adapts to new User api --- app/controllers/users/omniauth_callbacks_controller.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb index 0bdd2a801..81eaa08cb 100644 --- a/app/controllers/users/omniauth_callbacks_controller.rb +++ b/app/controllers/users/omniauth_callbacks_controller.rb @@ -41,9 +41,7 @@ class Users::OmniauthCallbacksController < Devise::OmniauthCallbacksController end def save_user(user) - @user.save || - @user.save_requiring_finish_signup || - @user.save_requiring_finish_signup_without_email + @user.save || @user.save_requiring_finish_signup end end From 3504a27ca3c6e2ba98f0aa635b1b1c08a7da230e Mon Sep 17 00:00:00 2001 From: kikito Date: Mon, 18 Apr 2016 13:03:05 +0200 Subject: [PATCH 04/11] limits username uniqueness when registering with oauth --- app/models/user.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/user.rb b/app/models/user.rb index 113781ef1..e8b885e0d 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -26,7 +26,7 @@ class User < ActiveRecord::Base belongs_to :geozone validates :username, presence: true, if: :username_required? - validates :username, uniqueness: true, if: :username_required? + validates :username, uniqueness: { scope: :registering_with_oauth }, if: :username_required? validates :document_number, uniqueness: { scope: :document_type }, allow_nil: true validate :validate_username_length From 3637c95fe2b121aedf743b05a91e2b5ce87811e3 Mon Sep 17 00:00:00 2001 From: kikito Date: Mon, 18 Apr 2016 16:07:53 +0200 Subject: [PATCH 05/11] adds note to finish_signup --- app/views/users/registrations/finish_signup.html.erb | 4 ++++ config/locales/en.yml | 1 + config/locales/es.yml | 1 + 3 files changed, 6 insertions(+) diff --git a/app/views/users/registrations/finish_signup.html.erb b/app/views/users/registrations/finish_signup.html.erb index fa7bb621e..79f160a51 100644 --- a/app/views/users/registrations/finish_signup.html.erb +++ b/app/views/users/registrations/finish_signup.html.erb @@ -3,6 +3,10 @@ <%= form_for current_user, as: :user, url: do_finish_signup_path, html: { role: 'form'} do |f| %> <%= render 'shared/errors', resource: current_user %> +
+ <%= t("omniauth.finish_signup.username_warning") %> +
+ <% if current_user.errors.include? :username %> <%= f.text_field :username, placeholder: t("devise_views.users.registrations.new.username_label") %> <% else %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 8609c079c..b5899e63a 100755 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -542,6 +542,7 @@ en: omniauth: finish_signup: title: "Additional details" + username_warning: "Due to a change in the way we interact with social networks, it is possible that your username now appears as 'already in use'. If that is your case, please choose a different username." twitter: sign_in: "Sign in with Twitter" sign_up: "Sign up with Twitter" diff --git a/config/locales/es.yml b/config/locales/es.yml index a15528d5a..468bb2dd7 100755 --- a/config/locales/es.yml +++ b/config/locales/es.yml @@ -542,6 +542,7 @@ es: omniauth: finish_signup: title: "Detalles adicionales de tu cuenta" + username_warning: "Debido a que hemos cambiado la forma en la que nos conectamos con redes sociales y es que tu nombre de usuario aparezca como 'ya en uso', incluso si antes podías acceder con él. Si es tu caso, por favor elije un nombre de usuario distinto." twitter: sign_in: "Entra con Twitter" sign_up: "Regístrate con Twitter" From d752a334f86f7e1a1ee43180c335c935ddd8ffb6 Mon Sep 17 00:00:00 2001 From: kikito Date: Mon, 18 Apr 2016 16:48:35 +0200 Subject: [PATCH 06/11] adds index for username --- db/migrate/20160418144013_add_index_to_username.rb | 5 +++++ db/schema.rb | 1 + 2 files changed, 6 insertions(+) create mode 100644 db/migrate/20160418144013_add_index_to_username.rb diff --git a/db/migrate/20160418144013_add_index_to_username.rb b/db/migrate/20160418144013_add_index_to_username.rb new file mode 100644 index 000000000..191f24ac6 --- /dev/null +++ b/db/migrate/20160418144013_add_index_to_username.rb @@ -0,0 +1,5 @@ +class AddIndexToUsername < ActiveRecord::Migration + def change + add_index :users, :username + end +end diff --git a/db/schema.rb b/db/schema.rb index 223a14d47..49f0011f2 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -432,6 +432,7 @@ ActiveRecord::Schema.define(version: 20160418172919) do add_index "users", ["geozone_id"], name: "index_users_on_geozone_id", using: :btree add_index "users", ["hidden_at"], name: "index_users_on_hidden_at", using: :btree add_index "users", ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true, using: :btree + add_index "users", ["username"], name: "index_users_on_username", using: :btree create_table "valuation_assignments", force: :cascade do |t| t.integer "valuator_id" From 581253219b6d1869e4dae308711a6ca0a1dbd8e5 Mon Sep 17 00:00:00 2001 From: kikito Date: Mon, 18 Apr 2016 18:13:12 +0200 Subject: [PATCH 07/11] adds rake task to mark some users with duplicated usernames so they change them --- lib/tasks/users.rake | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/lib/tasks/users.rake b/lib/tasks/users.rake index c56df2dd0..f0ef16b76 100644 --- a/lib/tasks/users.rake +++ b/lib/tasks/users.rake @@ -59,4 +59,16 @@ namespace :users do end end + desc "Makes duplicate username users change their username" + task social_network_reset: :environment do + duplicated_usernames = User.all.select(:username).group(:username).having('count(username) > 1').pluck(:username) + + duplicated_usernames.each do |username| + print "." + user_ids = User.where(username: username).order(created_at: :asc).pluck(:id) + user_ids_to_review = Identity.where(user_id: user_ids).pluck(:user_id) + user_ids_to_review.shift if user_ids.size == user_ids_to_review.size + user_ids_to_review.each { |id| User.find(id).update(registering_with_oauth: true) } + end + end end From e6c42860a75271b56bd34433bc6f3bcd807f5af1 Mon Sep 17 00:00:00 2001 From: kikito Date: Mon, 18 Apr 2016 18:43:28 +0200 Subject: [PATCH 08/11] destroy the identities associated with an user when the user deletes his account --- app/models/user.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/models/user.rb b/app/models/user.rb index e8b885e0d..924219ffd 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -154,6 +154,7 @@ class User < ActiveRecord::Base confirmed_phone: nil, unconfirmed_phone: nil ) + self.identities.destroy_all end def erased? From 8569e4f22e3d6e488f9c3753df0a5ae3359dcc20 Mon Sep 17 00:00:00 2001 From: kikito Date: Mon, 18 Apr 2016 18:52:21 +0200 Subject: [PATCH 09/11] adds rake task to destroy identities from erased users --- lib/tasks/users.rake | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/tasks/users.rake b/lib/tasks/users.rake index f0ef16b76..efbeeaa94 100644 --- a/lib/tasks/users.rake +++ b/lib/tasks/users.rake @@ -71,4 +71,9 @@ namespace :users do user_ids_to_review.each { |id| User.find(id).update(registering_with_oauth: true) } end end + + desc "Removes identities associated to erased users" + task remove_erased_identities: :environment do + Identity.joins(:user).where('users.erased_at IS NOT NULL').destroy_all + end end From d577a2b82eb79d13ba3b15eb815c7376ebe3786d Mon Sep 17 00:00:00 2001 From: kikito Date: Tue, 19 Apr 2016 12:07:20 +0200 Subject: [PATCH 10/11] add spec for user identity erasure --- spec/models/user_spec.rb | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 4d1b269ec..b49e3cc6b 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -333,6 +333,7 @@ describe User do email_verification_token: "token3", confirmed_phone:"5678", unconfirmed_phone:"5678") + user.erase('a test') user.reload @@ -353,6 +354,14 @@ describe User do expect(user.confirmation_token).to be_nil expect(user.reset_password_token).to be_nil expect(user.email_verification_token).to be_nil + + end + + it "destroys associated identities" do + user = create(:user) + identity = create(:identity, user: user) + user.erase('an identity test') + expect(Identity.exists?(identity.id)).to_not be end end From aa351256d072c42f8f33fd315cfa21eea1413a74 Mon Sep 17 00:00:00 2001 From: kikito Date: Tue, 19 Apr 2016 15:35:02 +0200 Subject: [PATCH 11/11] spanish typo spanish typo style --- app/models/user.rb | 2 +- config/locales/es.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 924219ffd..ea889ac57 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -217,7 +217,7 @@ class User < ActiveRecord::Base def save_requiring_finish_signup begin - self.registering_with_oauth= true + self.registering_with_oauth = true self.save(validate: false) # Devise puts unique constraints for the email the db, so we must detect & handle that rescue ActiveRecord::RecordNotUnique diff --git a/config/locales/es.yml b/config/locales/es.yml index 468bb2dd7..f1241bb15 100755 --- a/config/locales/es.yml +++ b/config/locales/es.yml @@ -542,7 +542,7 @@ es: omniauth: finish_signup: title: "Detalles adicionales de tu cuenta" - username_warning: "Debido a que hemos cambiado la forma en la que nos conectamos con redes sociales y es que tu nombre de usuario aparezca como 'ya en uso', incluso si antes podías acceder con él. Si es tu caso, por favor elije un nombre de usuario distinto." + username_warning: "Debido a que hemos cambiado la forma en la que nos conectamos con redes sociales y es posible que tu nombre de usuario aparezca como 'ya en uso', incluso si antes podías acceder con él. Si es tu caso, por favor elige un nombre de usuario distinto." twitter: sign_in: "Entra con Twitter" sign_up: "Regístrate con Twitter"