From f3ce7c86133b4e75dace86021fb0a7935c2f6b32 Mon Sep 17 00:00:00 2001 From: Xihai Luo Date: Sun, 10 Jul 2022 10:38:14 -0400 Subject: [PATCH 01/25] create view test to verify email and phone number fields show up --- spec/views/devise/passwords/new.html.erb_spec.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 spec/views/devise/passwords/new.html.erb_spec.rb diff --git a/spec/views/devise/passwords/new.html.erb_spec.rb b/spec/views/devise/passwords/new.html.erb_spec.rb new file mode 100644 index 0000000000..9cfe61ea49 --- /dev/null +++ b/spec/views/devise/passwords/new.html.erb_spec.rb @@ -0,0 +1,16 @@ +require "rails_helper" + +RSpec.describe "users/password/new", type: :view do + it "displays title" do + render template: "devise/passwords/new" + expect(rendered).to have_text("Forgot your password?") + end + + it "displays contact fields for user to reset password" do + render template: "devise/passwords/new" + expect(rendered).to have_text("Email") + expect(rendered).to have_field("user_email") + expect(rendered).to have_text("Phone number") + expect(rendered).to have_field("user_phone_number") + end +end From 83d194773b50b6616664984f91a9bf59e4b72e82 Mon Sep 17 00:00:00 2001 From: Xihai Luo Date: Sun, 10 Jul 2022 10:39:27 -0400 Subject: [PATCH 02/25] add phone number field to forgot your password page --- app/helpers/application_helper.rb | 12 ++++++++++++ app/views/devise/passwords/new.html.erb | 5 +++++ 2 files changed, 17 insertions(+) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 21f8ebf75f..306f0af64a 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -44,4 +44,16 @@ def flash_class(level) def og_tag(type, options = {}) tag.meta(property: "og:#{type}", **options) end + + def resource_name + :user + end + + def resource + @resource ||= User.new + end + + def devise_mapping + @devise_mapping ||= Devise.mappings[:user] + end end diff --git a/app/views/devise/passwords/new.html.erb b/app/views/devise/passwords/new.html.erb index 03d4549d98..88f8753c72 100644 --- a/app/views/devise/passwords/new.html.erb +++ b/app/views/devise/passwords/new.html.erb @@ -15,6 +15,11 @@ <%= f.email_field :email, autofocus: true, autocomplete: "email", class: "form-control", required: true %> +
+ <%= f.label :phone_number %>
+ <%= f.text_field :phone_number, autofocus: true, autocomplete: "phone number", class: "form-control" %> +
+
<%= f.submit "Send me reset password instructions", class: "btn btn-primary" %>
From c7008ccf3e2b369cdde96e24115be8cbe048361f Mon Sep 17 00:00:00 2001 From: Xihai Luo Date: Sun, 10 Jul 2022 10:39:48 -0400 Subject: [PATCH 03/25] initial branch commit --- app/models/casa_admin.rb | 2 +- app/models/casa_case_contact_type.rb | 4 +-- app/models/case_assignment.rb | 4 +-- app/models/case_contact.rb | 4 +-- app/models/case_contact_contact_type.rb | 4 +-- app/models/contact_type.rb | 2 +- app/models/contact_type_group.rb | 2 +- app/models/court_date.rb | 2 +- app/models/emancipation_option.rb | 2 +- app/models/hearing_type.rb | 2 +- app/models/judge.rb | 2 +- app/models/supervisor.rb | 2 +- app/models/supervisor_volunteer.rb | 4 +-- app/models/user.rb | 2 +- app/models/volunteer.rb | 2 +- db/schema.rb | 47 ++++++++++++------------- 16 files changed, 43 insertions(+), 44 deletions(-) diff --git a/app/models/casa_admin.rb b/app/models/casa_admin.rb index c0dbf6ef5b..511bc62a3a 100644 --- a/app/models/casa_admin.rb +++ b/app/models/casa_admin.rb @@ -46,7 +46,7 @@ def change_to_supervisor! # created_at :datetime not null # updated_at :datetime not null # casa_org_id :bigint not null -# invited_by_id :bigint +# invited_by_id :integer # # Indexes # diff --git a/app/models/casa_case_contact_type.rb b/app/models/casa_case_contact_type.rb index a41eda5dfc..570ddbf4bd 100644 --- a/app/models/casa_case_contact_type.rb +++ b/app/models/casa_case_contact_type.rb @@ -12,8 +12,8 @@ class CasaCaseContactType < ApplicationRecord # id :bigint not null, primary key # created_at :datetime not null # updated_at :datetime not null -# casa_case_id :bigint not null -# contact_type_id :bigint not null +# casa_case_id :integer not null +# contact_type_id :integer not null # # Indexes # diff --git a/app/models/case_assignment.rb b/app/models/case_assignment.rb index a9a7a918bf..93b601b6fe 100644 --- a/app/models/case_assignment.rb +++ b/app/models/case_assignment.rb @@ -36,8 +36,8 @@ def casa_case_and_volunteer_must_belong_to_same_casa_org # hide_old_contacts :boolean default(FALSE) # created_at :datetime not null # updated_at :datetime not null -# casa_case_id :bigint not null -# volunteer_id :bigint not null +# casa_case_id :integer not null +# volunteer_id :integer not null # # Indexes # diff --git a/app/models/case_contact.rb b/app/models/case_contact.rb index 5b1e238001..d798791afa 100644 --- a/app/models/case_contact.rb +++ b/app/models/case_contact.rb @@ -215,8 +215,8 @@ def self.options_for_sorted_by # want_driving_reimbursement :boolean default(FALSE) # created_at :datetime not null # updated_at :datetime not null -# casa_case_id :bigint not null -# creator_id :bigint not null +# casa_case_id :integer not null +# creator_id :integer not null # # Indexes # diff --git a/app/models/case_contact_contact_type.rb b/app/models/case_contact_contact_type.rb index 9376453cba..a15836a2e6 100644 --- a/app/models/case_contact_contact_type.rb +++ b/app/models/case_contact_contact_type.rb @@ -12,8 +12,8 @@ class CaseContactContactType < ApplicationRecord # id :bigint not null, primary key # created_at :datetime not null # updated_at :datetime not null -# case_contact_id :bigint not null -# contact_type_id :bigint not null +# case_contact_id :integer not null +# contact_type_id :integer not null # # Indexes # diff --git a/app/models/contact_type.rb b/app/models/contact_type.rb index 91f400e08b..0552ffc790 100644 --- a/app/models/contact_type.rb +++ b/app/models/contact_type.rb @@ -21,7 +21,7 @@ class ContactType < ApplicationRecord # name :string not null # created_at :datetime not null # updated_at :datetime not null -# contact_type_group_id :bigint not null +# contact_type_group_id :integer not null # # Indexes # diff --git a/app/models/contact_type_group.rb b/app/models/contact_type_group.rb index 5d4e97c577..752d5ddb87 100644 --- a/app/models/contact_type_group.rb +++ b/app/models/contact_type_group.rb @@ -49,7 +49,7 @@ def generate_for_org!(casa_org) # name :string not null # created_at :datetime not null # updated_at :datetime not null -# casa_org_id :bigint not null +# casa_org_id :integer not null # # Indexes # diff --git a/app/models/court_date.rb b/app/models/court_date.rb index 045b9bef0b..73ddd69fef 100644 --- a/app/models/court_date.rb +++ b/app/models/court_date.rb @@ -73,7 +73,7 @@ def case_court_orders_context_hash # date :datetime not null # created_at :datetime not null # updated_at :datetime not null -# casa_case_id :bigint not null +# casa_case_id :integer not null # hearing_type_id :bigint # judge_id :bigint # diff --git a/app/models/emancipation_option.rb b/app/models/emancipation_option.rb index 17ad42f96d..6f1293ff26 100644 --- a/app/models/emancipation_option.rb +++ b/app/models/emancipation_option.rb @@ -24,7 +24,7 @@ class EmancipationOption < ApplicationRecord # name :string not null # created_at :datetime not null # updated_at :datetime not null -# emancipation_category_id :bigint not null +# emancipation_category_id :integer not null # # Indexes # diff --git a/app/models/hearing_type.rb b/app/models/hearing_type.rb index e35d0f0d7f..2c1ff0969c 100644 --- a/app/models/hearing_type.rb +++ b/app/models/hearing_type.rb @@ -38,7 +38,7 @@ def generate_for_org!(casa_org) # active :boolean default(TRUE), not null # checklist_updated_date :string default("None"), not null # name :string not null -# casa_org_id :bigint not null +# casa_org_id :integer not null # # Indexes # diff --git a/app/models/judge.rb b/app/models/judge.rb index 0892f10b80..2d34129209 100644 --- a/app/models/judge.rb +++ b/app/models/judge.rb @@ -16,7 +16,7 @@ class Judge < ApplicationRecord # name :string # created_at :datetime not null # updated_at :datetime not null -# casa_org_id :bigint not null +# casa_org_id :integer not null # # Indexes # diff --git a/app/models/supervisor.rb b/app/models/supervisor.rb index 55a445d483..3f9262abaa 100644 --- a/app/models/supervisor.rb +++ b/app/models/supervisor.rb @@ -73,7 +73,7 @@ def recently_unassigned_volunteers # created_at :datetime not null # updated_at :datetime not null # casa_org_id :bigint not null -# invited_by_id :bigint +# invited_by_id :integer # # Indexes # diff --git a/app/models/supervisor_volunteer.rb b/app/models/supervisor_volunteer.rb index a4dfbd9e1b..e8df9d30a7 100644 --- a/app/models/supervisor_volunteer.rb +++ b/app/models/supervisor_volunteer.rb @@ -24,8 +24,8 @@ def ensure_supervisor_and_volunteer_belong_to_same_casa_org # is_active :boolean default(TRUE) # created_at :datetime not null # updated_at :datetime not null -# supervisor_id :bigint not null -# volunteer_id :bigint not null +# supervisor_id :integer not null +# volunteer_id :integer not null # # Indexes # diff --git a/app/models/user.rb b/app/models/user.rb index f212295595..6ea60b36d2 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -165,7 +165,7 @@ def serving_transition_aged_youth? # created_at :datetime not null # updated_at :datetime not null # casa_org_id :bigint not null -# invited_by_id :bigint +# invited_by_id :integer # # Indexes # diff --git a/app/models/volunteer.rb b/app/models/volunteer.rb index 6554ceaa27..89e2694f5b 100644 --- a/app/models/volunteer.rb +++ b/app/models/volunteer.rb @@ -164,7 +164,7 @@ def cases_where_contact_made_in_days(num_days = CONTACT_MADE_IN_DAYS_NUM) # created_at :datetime not null # updated_at :datetime not null # casa_org_id :bigint not null -# invited_by_id :bigint +# invited_by_id :integer # # Indexes # diff --git a/db/schema.rb b/db/schema.rb index 32a76b2236..89d530e0b5 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -37,7 +37,7 @@ end create_table "active_storage_variant_records", force: :cascade do |t| - t.bigint "blob_id", null: false + t.integer "blob_id", null: false t.string "variation_digest", null: false t.index ["blob_id", "variation_digest"], name: "index_active_storage_variant_records_uniqueness", unique: true end @@ -79,8 +79,8 @@ end create_table "casa_case_contact_types", force: :cascade do |t| - t.bigint "contact_type_id", null: false - t.bigint "casa_case_id", null: false + t.integer "contact_type_id", null: false + t.integer "casa_case_id", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false t.index ["casa_case_id"], name: "index_casa_case_contact_types_on_casa_case_id" @@ -136,17 +136,17 @@ t.string "footer_links", default: [], array: true t.string "slug" t.boolean "show_driving_reimbursement", default: true - t.boolean "show_fund_request", default: false t.string "twilio_phone_number" t.string "twilio_account_sid" t.string "twilio_api_key_sid" t.string "twilio_api_key_secret" + t.boolean "show_fund_request", default: false t.index ["slug"], name: "index_casa_orgs_on_slug", unique: true end create_table "case_assignments", force: :cascade do |t| - t.bigint "casa_case_id", null: false - t.bigint "volunteer_id", null: false + t.integer "casa_case_id", null: false + t.integer "volunteer_id", null: false t.boolean "active", default: true, null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false @@ -156,8 +156,8 @@ end create_table "case_contact_contact_types", force: :cascade do |t| - t.bigint "case_contact_id", null: false - t.bigint "contact_type_id", null: false + t.integer "case_contact_id", null: false + t.integer "contact_type_id", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false t.index ["case_contact_id"], name: "index_case_contact_contact_types_on_case_contact_id" @@ -165,8 +165,8 @@ end create_table "case_contacts", force: :cascade do |t| - t.bigint "creator_id", null: false - t.bigint "casa_case_id", null: false + t.integer "creator_id", null: false + t.integer "casa_case_id", null: false t.integer "duration_minutes", null: false t.datetime "occurred_at", precision: nil, null: false t.datetime "created_at", null: false @@ -206,7 +206,7 @@ end create_table "contact_type_groups", force: :cascade do |t| - t.bigint "casa_org_id", null: false + t.integer "casa_org_id", null: false t.string "name", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false @@ -215,7 +215,7 @@ end create_table "contact_types", force: :cascade do |t| - t.bigint "contact_type_group_id", null: false + t.integer "contact_type_group_id", null: false t.string "name", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false @@ -225,7 +225,7 @@ create_table "court_dates", force: :cascade do |t| t.datetime "date", precision: nil, null: false - t.bigint "casa_case_id", null: false + t.integer "casa_case_id", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false t.bigint "hearing_type_id" @@ -259,7 +259,7 @@ end create_table "emancipation_options", force: :cascade do |t| - t.bigint "emancipation_category_id", null: false + t.integer "emancipation_category_id", null: false t.string "name", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false @@ -309,7 +309,7 @@ end create_table "hearing_types", force: :cascade do |t| - t.bigint "casa_org_id", null: false + t.integer "casa_org_id", null: false t.string "name", null: false t.boolean "active", default: true, null: false t.string "checklist_updated_date", default: "None", null: false @@ -317,7 +317,7 @@ end create_table "judges", force: :cascade do |t| - t.bigint "casa_org_id", null: false + t.integer "casa_org_id", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false t.boolean "active", default: true @@ -433,8 +433,8 @@ end create_table "supervisor_volunteers", force: :cascade do |t| - t.bigint "supervisor_id", null: false - t.bigint "volunteer_id", null: false + t.integer "supervisor_id", null: false + t.integer "volunteer_id", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false t.boolean "is_active", default: true @@ -446,13 +446,12 @@ t.string "version", null: false end - create_table "user_reminder_times", force: :cascade do |t| + create_table "user_case_contact_types_reminders", force: :cascade do |t| t.bigint "user_id", null: false - t.datetime "case_contact_types" - t.datetime "no_contact_made" + t.datetime "reminder_sent" t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.index ["user_id"], name: "index_user_reminder_times_on_user_id" + t.index ["user_id"], name: "index_user_case_contact_types_reminders_on_user_id" end create_table "user_sms_notification_events", force: :cascade do |t| @@ -479,7 +478,7 @@ t.datetime "invitation_accepted_at", precision: nil t.integer "invitation_limit" t.string "invited_by_type" - t.bigint "invited_by_id" + t.integer "invited_by_id" t.integer "invitations_count", default: 0 t.string "type" t.boolean "active", default: true @@ -527,7 +526,7 @@ add_foreign_key "sent_emails", "users" add_foreign_key "supervisor_volunteers", "users", column: "supervisor_id" add_foreign_key "supervisor_volunteers", "users", column: "volunteer_id" - add_foreign_key "user_reminder_times", "users" + add_foreign_key "user_case_contact_types_reminders", "users" add_foreign_key "user_sms_notification_events", "sms_notification_events" add_foreign_key "user_sms_notification_events", "users" add_foreign_key "users", "casa_orgs" From a2d80741c94711dd24d7b3e3bfd495b702593a32 Mon Sep 17 00:00:00 2001 From: Xihai Luo Date: Sun, 10 Jul 2022 11:32:27 -0400 Subject: [PATCH 04/25] create system tests to verify fields can handle correct inputs --- spec/system/devise/passwords/new_spec.rb | 28 ++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 spec/system/devise/passwords/new_spec.rb diff --git a/spec/system/devise/passwords/new_spec.rb b/spec/system/devise/passwords/new_spec.rb new file mode 100644 index 0000000000..0498cbf92d --- /dev/null +++ b/spec/system/devise/passwords/new_spec.rb @@ -0,0 +1,28 @@ +require "rails_helper" + +RSpec.describe "users/passwords/new", type: :system do + before do + visit root_path + click_on "Forgot your password?" + end + + it "displays error messages for non-existent user" do + fill_in "Email", with: "tangerine@forward.com" + fill_in "Phone number", with: "+16578900012" + + click_on "Send me reset password instructions" + expect(page).to have_content "2 errors prohibited this User from being saved:" + expect(page).to have_text("Email not found") + expect(page).to have_text("Phone number not found") + end + + it "displays phone number error messages for incorrect formatting" do + create(:user, email: "glados@aperture.labs") + fill_in "Email", with: "glados@aperture.labs" + fill_in "Phone number", with: "2134567eee" + + click_on "Send me reset password instructions" + expect(page).to have_content "1 error prohibited this User from being saved:" + expect(page).to have_text("Phone number must be 12 digits including country code (+1)") + end +end From 5b28758b4e52bbdff9d95ef5d07f50b759ebace7 Mon Sep 17 00:00:00 2001 From: Xihai Luo Date: Wed, 13 Jul 2022 11:48:42 -0400 Subject: [PATCH 05/25] set up route to map to custom password controller implemented from devise --- config/routes.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/routes.rb b/config/routes.rb index ea3a43f87d..373a7aa5b8 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -2,7 +2,7 @@ Rails.application.routes.draw do devise_for :all_casa_admins, path: "all_casa_admins", controllers: {sessions: "all_casa_admins/sessions"} - devise_for :users, controllers: {sessions: "users/sessions"} + devise_for :users, controllers: {sessions: "users/sessions", passwords: "users/passwords"} concern :with_datatable do post "datatable", on: :collection From f8dbab0e753e003791e3d468df01fc71c763caf1 Mon Sep 17 00:00:00 2001 From: Xihai Luo Date: Wed, 13 Jul 2022 11:50:30 -0400 Subject: [PATCH 06/25] implement form error handling in custom password controller --- app/controllers/users/passwords_controller.rb | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 app/controllers/users/passwords_controller.rb diff --git a/app/controllers/users/passwords_controller.rb b/app/controllers/users/passwords_controller.rb new file mode 100644 index 0000000000..fd7c987609 --- /dev/null +++ b/app/controllers/users/passwords_controller.rb @@ -0,0 +1,23 @@ +class Users::PasswordsController < Devise::PasswordsController + include ApplicationHelper + include PhoneNumberHelper + + def create + email = params[resource_name][:email] + phone_number = params[resource_name][:phone_number] + # try to find user by email + if !User.find_by(email: email) + resource.errors.add(:base, "Email not found") + end + # validate and add any errors + validation = valid_phone_number(phone_number) + if validation[0] + User.find_by(phone_number: phone_number) ? "" : resource.errors.add(:base, "Phone number not found") + else + resource.errors.add(:phone_number, validation[1]) + end + # re-render and display errors + respond_with(resource) + # otherwise, send reset email and sms + end +end From 7c46dc112806f47aa1c9cbe15ec5abd871032977 Mon Sep 17 00:00:00 2001 From: Xihai Luo Date: Wed, 13 Jul 2022 15:49:29 -0400 Subject: [PATCH 07/25] add ability to send emails with password reset link in custom password controller route --- app/controllers/users/passwords_controller.rb | 19 ++++++++++++++++++- spec/system/devise/passwords/new_spec.rb | 8 ++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/app/controllers/users/passwords_controller.rb b/app/controllers/users/passwords_controller.rb index fd7c987609..0a9b76da50 100644 --- a/app/controllers/users/passwords_controller.rb +++ b/app/controllers/users/passwords_controller.rb @@ -5,10 +5,13 @@ class Users::PasswordsController < Devise::PasswordsController def create email = params[resource_name][:email] phone_number = params[resource_name][:phone_number] + reset_token = ""; + # try to find user by email if !User.find_by(email: email) resource.errors.add(:base, "Email not found") end + # validate and add any errors validation = valid_phone_number(phone_number) if validation[0] @@ -16,8 +19,22 @@ def create else resource.errors.add(:phone_number, validation[1]) end + # re-render and display errors - respond_with(resource) + if resource.errors.any? + respond_with(resource) + return + end + # otherwise, send reset email and sms + @resource = User.find_by(email: email) + # generate a reset token + # call devise mailer + reset_token = @resource.send_reset_password_instructions + + if successfully_sent?(@resource) + p reset_token + respond_with({}, location: after_sending_reset_password_instructions_path_for(resource_name)) + end end end diff --git a/spec/system/devise/passwords/new_spec.rb b/spec/system/devise/passwords/new_spec.rb index 0498cbf92d..219d943756 100644 --- a/spec/system/devise/passwords/new_spec.rb +++ b/spec/system/devise/passwords/new_spec.rb @@ -25,4 +25,12 @@ expect(page).to have_content "1 error prohibited this User from being saved:" expect(page).to have_text("Phone number must be 12 digits including country code (+1)") end + + it "successfully redirects to sign up page with a notice" do + create(:user, email: "glados@aperture.labs") + fill_in "Email", with: "glados@aperture.labs" + + click_on "Send me reset password instructions" + expect(page).to have_content "You will receive an email with instructions on how to reset your password in a few minutes." + end end From c48b094669212c24b232a754a94020658f745960 Mon Sep 17 00:00:00 2001 From: Xihai Luo Date: Thu, 14 Jul 2022 15:41:37 -0400 Subject: [PATCH 08/25] create request specs to verify short io and twilio services hit and also redirect --- spec/requests/users/passwords_spec.rb | 32 +++++++++++++++++++++ spec/support/stubbed_requests/twilio_api.rb | 12 ++++++++ 2 files changed, 44 insertions(+) create mode 100644 spec/requests/users/passwords_spec.rb diff --git a/spec/requests/users/passwords_spec.rb b/spec/requests/users/passwords_spec.rb new file mode 100644 index 0000000000..1796e7efe5 --- /dev/null +++ b/spec/requests/users/passwords_spec.rb @@ -0,0 +1,32 @@ +require "rails_helper" +require "support/stubbed_requests/webmock_helper" + +RSpec.describe "/users/passwords", type: :request do + describe "POST /create" do + before do + stubbed_sites = ["api.twilio.com", "api.short.io"] + web_mock = WebMockHelper.new(stubbed_sites) + web_mock.stub_network_connection + end + + it "sends a password reset SMS to existing user" do + org = create(:casa_org) + user = create(:user, phone_number: "+12222222222", casa_org: org) + + @short_io_stub = WebMockHelper.short_io_stub_sms + @twilio_stub = WebMockHelper.twilio_password_reset_stub(user) + + params = { + user: { + email: user.email, + phone_number: user.phone_number + } + } + + post "/users/password", params: params + expect(@short_io_stub).to have_been_requested.times(1) + expect(@twilio_stub).to have_been_requested.times(1) + expect(response).to have_http_status(:redirect) + end + end +end diff --git a/spec/support/stubbed_requests/twilio_api.rb b/spec/support/stubbed_requests/twilio_api.rb index f1c6784120..f15aa62494 100644 --- a/spec/support/stubbed_requests/twilio_api.rb +++ b/spec/support/stubbed_requests/twilio_api.rb @@ -70,4 +70,16 @@ def twilio_no_contact_made_stub(resousne = "") ) .to_return(body: "{\"error_code\":null, \"status\":\"sent\", \"body\":\"It's been two weeks since you've tried reaching 'test'. Try again! https://42ni.short.gy/jzTwdF\"}") end + + def twilio_password_reset_stub(resource) + WebMock.stub_request(:post, "https://api.twilio.com/2010-04-01/Accounts/articuno34/Messages.json") + .with( + body: {From: "+15555555555", Body: "Hi #{resource.display_name}, click here to reset your password: https://42ni.short.gy/jzTwdF", To: "+12222222222"}, + headers: { + "Content-Type" => "application/x-www-form-urlencoded", + "Authorization" => "Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ==" + } + ) + .to_return(body: "{\"error_code\":null, \"status\":\"sent\", \"body\":\"Execute Order 66 - https://42ni.short.gy/jzTwdF\"}") + end end From a6fa24a06b771768e6e4c0cec166efa0e1f0cab2 Mon Sep 17 00:00:00 2001 From: Xihai Luo Date: Thu, 14 Jul 2022 15:48:19 -0400 Subject: [PATCH 09/25] extend password controller create action to send pass reset email and SMS --- app/controllers/users/passwords_controller.rb | 18 +++++++++++++++++- app/helpers/sms_body_helper.rb | 4 ++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/app/controllers/users/passwords_controller.rb b/app/controllers/users/passwords_controller.rb index 0a9b76da50..57117717ce 100644 --- a/app/controllers/users/passwords_controller.rb +++ b/app/controllers/users/passwords_controller.rb @@ -1,6 +1,7 @@ class Users::PasswordsController < Devise::PasswordsController include ApplicationHelper include PhoneNumberHelper + include SmsBodyHelper def create email = params[resource_name][:email] @@ -32,8 +33,23 @@ def create # call devise mailer reset_token = @resource.send_reset_password_instructions + if !phone_number.blank? + reset_password_link = request.base_url + "/resource/password/edit?reset_password_token=#{reset_token}" + short_io_service = ShortUrlService.new + twilio_service = TwilioService.new(@resource.casa_org.twilio_api_key_sid, @resource.casa_org.twilio_api_key_secret, @resource.casa_org.twilio_account_sid) + + short_io_service.create_short_url(reset_password_link) + body_msg = password_reset_msg(@resource.display_name, short_io_service.short_url) + + sms_params = { + From: @resource.casa_org.twilio_phone_number, + Body: body_msg, + To: phone_number + } + twilio_service.send_sms(sms_params) + end + if successfully_sent?(@resource) - p reset_token respond_with({}, location: after_sending_reset_password_instructions_path_for(resource_name)) end end diff --git a/app/helpers/sms_body_helper.rb b/app/helpers/sms_body_helper.rb index 42cd991670..3cfc66b53e 100644 --- a/app/helpers/sms_body_helper.rb +++ b/app/helpers/sms_body_helper.rb @@ -29,4 +29,8 @@ def no_contact_made_msg(contact_type, short_link) def case_contact_flagged_msg(display_name, short_link) "#{display_name} has flagged a Case Contact that needs follow up. Click to see more: #{short_link}" end + + def password_reset_msg(display_name, short_link) + "Hi #{display_name}, click here to reset your password: #{short_link}" + end end From ad6bf4109b4c66f3cd982145b861a8903e2f1f8e Mon Sep 17 00:00:00 2001 From: Xihai Luo Date: Thu, 14 Jul 2022 16:51:28 -0400 Subject: [PATCH 10/25] add text in views to notify users they only need to enter an email or phone number --- app/views/devise/passwords/new.html.erb | 4 +++- spec/system/devise/passwords/new_spec.rb | 8 ++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/app/views/devise/passwords/new.html.erb b/app/views/devise/passwords/new.html.erb index 88f8753c72..c0008a01fc 100644 --- a/app/views/devise/passwords/new.html.erb +++ b/app/views/devise/passwords/new.html.erb @@ -10,9 +10,11 @@ <%= form_for(resource, as: resource_name, url: password_path(resource_name), html: {method: :post}) do |f| %> <%= render "/shared/error_messages", resource: resource %> +

Please enter email or phone number to recieve reset instructions.

+
<%= f.label :email %>
- <%= f.email_field :email, autofocus: true, autocomplete: "email", class: "form-control", required: true %> + <%= f.email_field :email, autofocus: true, autocomplete: "email", class: "form-control" %>
diff --git a/spec/system/devise/passwords/new_spec.rb b/spec/system/devise/passwords/new_spec.rb index 219d943756..b9ceaba45b 100644 --- a/spec/system/devise/passwords/new_spec.rb +++ b/spec/system/devise/passwords/new_spec.rb @@ -6,6 +6,10 @@ click_on "Forgot your password?" end + it "displays text above form fields" do + expect(page).to have_text("Please enter email or phone number to recieve reset instructions.") + end + it "displays error messages for non-existent user" do fill_in "Email", with: "tangerine@forward.com" fill_in "Phone number", with: "+16578900012" @@ -26,11 +30,11 @@ expect(page).to have_text("Phone number must be 12 digits including country code (+1)") end - it "successfully redirects to sign up page with a notice" do + it "redirects to sign up page for email" do create(:user, email: "glados@aperture.labs") fill_in "Email", with: "glados@aperture.labs" click_on "Send me reset password instructions" - expect(page).to have_content "You will receive an email with instructions on how to reset your password in a few minutes." + expect(page).to have_content "You will receive an email or SMS with instructions on how to reset your password in a few minutes." end end From 5ce13d09bfe505050dc95b5120fa86e550c836dc Mon Sep 17 00:00:00 2001 From: Xihai Luo Date: Thu, 14 Jul 2022 16:52:43 -0400 Subject: [PATCH 11/25] refactor controller to allow the option of handling one or the other (email or SMS) --- app/controllers/users/passwords_controller.rb | 10 ++++------ spec/requests/users/passwords_spec.rb | 2 ++ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/controllers/users/passwords_controller.rb b/app/controllers/users/passwords_controller.rb index 57117717ce..d9ba8685a6 100644 --- a/app/controllers/users/passwords_controller.rb +++ b/app/controllers/users/passwords_controller.rb @@ -6,10 +6,10 @@ class Users::PasswordsController < Devise::PasswordsController def create email = params[resource_name][:email] phone_number = params[resource_name][:phone_number] - reset_token = ""; + reset_token = "" # try to find user by email - if !User.find_by(email: email) + if !email.blank? && !User.find_by(email: email) resource.errors.add(:base, "Email not found") end @@ -28,7 +28,7 @@ def create end # otherwise, send reset email and sms - @resource = User.find_by(email: email) + @resource = email.blank? ? User.find_by(phone_number: phone_number) : User.find_by(email: email) # generate a reset token # call devise mailer reset_token = @resource.send_reset_password_instructions @@ -49,8 +49,6 @@ def create twilio_service.send_sms(sms_params) end - if successfully_sent?(@resource) - respond_with({}, location: after_sending_reset_password_instructions_path_for(resource_name)) - end + redirect_to after_sending_reset_password_instructions_path_for(resource_name), notice: "You will receive an email or SMS with instructions on how to reset your password in a few minutes." end end diff --git a/spec/requests/users/passwords_spec.rb b/spec/requests/users/passwords_spec.rb index 1796e7efe5..e635742323 100644 --- a/spec/requests/users/passwords_spec.rb +++ b/spec/requests/users/passwords_spec.rb @@ -27,6 +27,8 @@ expect(@short_io_stub).to have_been_requested.times(1) expect(@twilio_stub).to have_been_requested.times(1) expect(response).to have_http_status(:redirect) + follow_redirect! + expect(flash[:notice]).to match(/You will receive an email or SMS with instructions on how to reset your password in a few minutes./) end end end From ee24de3bea62bc6b2e1ccf57bee403e9ee50cfd3 Mon Sep 17 00:00:00 2001 From: Xihai Luo Date: Thu, 14 Jul 2022 23:35:13 -0400 Subject: [PATCH 12/25] handle edge cases such as no fields entered and only phone number entered (so email should not be sent) --- app/controllers/users/passwords_controller.rb | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/app/controllers/users/passwords_controller.rb b/app/controllers/users/passwords_controller.rb index d9ba8685a6..216ca55de2 100644 --- a/app/controllers/users/passwords_controller.rb +++ b/app/controllers/users/passwords_controller.rb @@ -6,7 +6,11 @@ class Users::PasswordsController < Devise::PasswordsController def create email = params[resource_name][:email] phone_number = params[resource_name][:phone_number] - reset_token = "" + reset_token = nil + + if email.blank? && phone_number.blank? + resource.errors.add(:base, "Please enter at least one field.") + end # try to find user by email if !email.blank? && !User.find_by(email: email) @@ -31,9 +35,15 @@ def create @resource = email.blank? ? User.find_by(phone_number: phone_number) : User.find_by(email: email) # generate a reset token # call devise mailer - reset_token = @resource.send_reset_password_instructions + if !email.blank? + reset_token = @resource.send_reset_password_instructions + end if !phone_number.blank? + # when user enters ONLY a phone number, generate a new reset token to use; + # otherwise, use the same reset token as sent in the email + reset_token ||= @resource.generate_password_reset_token + reset_password_link = request.base_url + "/resource/password/edit?reset_password_token=#{reset_token}" short_io_service = ShortUrlService.new twilio_service = TwilioService.new(@resource.casa_org.twilio_api_key_sid, @resource.casa_org.twilio_api_key_secret, @resource.casa_org.twilio_account_sid) From cfa843446f0544091dfd4c90d6e6043ee66592cd Mon Sep 17 00:00:00 2001 From: Xihai Luo Date: Sat, 16 Jul 2022 09:26:51 -0400 Subject: [PATCH 13/25] fix bug with reset pass link --- app/controllers/users/passwords_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/users/passwords_controller.rb b/app/controllers/users/passwords_controller.rb index 216ca55de2..8dcaefeb17 100644 --- a/app/controllers/users/passwords_controller.rb +++ b/app/controllers/users/passwords_controller.rb @@ -44,7 +44,7 @@ def create # otherwise, use the same reset token as sent in the email reset_token ||= @resource.generate_password_reset_token - reset_password_link = request.base_url + "/resource/password/edit?reset_password_token=#{reset_token}" + reset_password_link = request.base_url + "/users/password/edit?reset_password_token=#{reset_token}" short_io_service = ShortUrlService.new twilio_service = TwilioService.new(@resource.casa_org.twilio_api_key_sid, @resource.casa_org.twilio_api_key_secret, @resource.casa_org.twilio_account_sid) From f2db6b9cefdb71bae2ee7a1ad18aaf983d4fccd0 Mon Sep 17 00:00:00 2001 From: Xihai Luo Date: Sat, 16 Jul 2022 09:34:44 -0400 Subject: [PATCH 14/25] clean up and lint files --- app/controllers/users/passwords_controller.rb | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/app/controllers/users/passwords_controller.rb b/app/controllers/users/passwords_controller.rb index 8dcaefeb17..bb6ae7ffcb 100644 --- a/app/controllers/users/passwords_controller.rb +++ b/app/controllers/users/passwords_controller.rb @@ -12,12 +12,10 @@ def create resource.errors.add(:base, "Please enter at least one field.") end - # try to find user by email if !email.blank? && !User.find_by(email: email) resource.errors.add(:base, "Email not found") end - # validate and add any errors validation = valid_phone_number(phone_number) if validation[0] User.find_by(phone_number: phone_number) ? "" : resource.errors.add(:base, "Phone number not found") @@ -25,7 +23,7 @@ def create resource.errors.add(:phone_number, validation[1]) end - # re-render and display errors + # re-render and display any errors if resource.errors.any? respond_with(resource) return @@ -33,15 +31,15 @@ def create # otherwise, send reset email and sms @resource = email.blank? ? User.find_by(phone_number: phone_number) : User.find_by(email: email) - # generate a reset token - # call devise mailer if !email.blank? + # generate a reset token and + # call devise mailer reset_token = @resource.send_reset_password_instructions end if !phone_number.blank? # when user enters ONLY a phone number, generate a new reset token to use; - # otherwise, use the same reset token as sent in the email + # otherwise, use the same reset token as sent by devise mailer reset_token ||= @resource.generate_password_reset_token reset_password_link = request.base_url + "/users/password/edit?reset_password_token=#{reset_token}" From be2b80d949cc01ca51c8688ceb735fe16f81d030 Mon Sep 17 00:00:00 2001 From: Xihai Luo Date: Sat, 16 Jul 2022 14:44:06 -0400 Subject: [PATCH 15/25] move spec to views so it is semantically consistent --- spec/system/devise/passwords/new_spec.rb | 4 ---- spec/views/devise/passwords/new.html.erb_spec.rb | 5 +++++ 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/spec/system/devise/passwords/new_spec.rb b/spec/system/devise/passwords/new_spec.rb index b9ceaba45b..bc45cfc194 100644 --- a/spec/system/devise/passwords/new_spec.rb +++ b/spec/system/devise/passwords/new_spec.rb @@ -6,10 +6,6 @@ click_on "Forgot your password?" end - it "displays text above form fields" do - expect(page).to have_text("Please enter email or phone number to recieve reset instructions.") - end - it "displays error messages for non-existent user" do fill_in "Email", with: "tangerine@forward.com" fill_in "Phone number", with: "+16578900012" diff --git a/spec/views/devise/passwords/new.html.erb_spec.rb b/spec/views/devise/passwords/new.html.erb_spec.rb index 9cfe61ea49..026ba43e31 100644 --- a/spec/views/devise/passwords/new.html.erb_spec.rb +++ b/spec/views/devise/passwords/new.html.erb_spec.rb @@ -6,6 +6,11 @@ expect(rendered).to have_text("Forgot your password?") end + it "displays text above form fields" do + render template: "devise/passwords/new" + expect(rendered).to have_text("Please enter email or phone number to recieve reset instructions.") + end + it "displays contact fields for user to reset password" do render template: "devise/passwords/new" expect(rendered).to have_text("Email") From 76d03272357c7d460ae36a531bee6ca9ce34b12f Mon Sep 17 00:00:00 2001 From: Xihai Luo Date: Sat, 16 Jul 2022 14:49:16 -0400 Subject: [PATCH 16/25] add system spec to check for empty form submission --- spec/system/devise/passwords/new_spec.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/spec/system/devise/passwords/new_spec.rb b/spec/system/devise/passwords/new_spec.rb index bc45cfc194..ad09984bbd 100644 --- a/spec/system/devise/passwords/new_spec.rb +++ b/spec/system/devise/passwords/new_spec.rb @@ -26,6 +26,11 @@ expect(page).to have_text("Phone number must be 12 digits including country code (+1)") end + it "displays error if user tries to submit empty form" do + click_on "Send me reset password instructions" + expect(page).to have_text("Please enter at least one field.") + end + it "redirects to sign up page for email" do create(:user, email: "glados@aperture.labs") fill_in "Email", with: "glados@aperture.labs" From 09149e392d2c53a295af19600c7ef4cf03c6a473 Mon Sep 17 00:00:00 2001 From: Xihai Luo Date: Sat, 16 Jul 2022 16:18:54 -0400 Subject: [PATCH 17/25] refactor passwords controller create action to reduce cognitive complexity --- app/controllers/users/passwords_controller.rb | 50 +++++++++++-------- spec/system/devise/passwords/new_spec.rb | 5 +- 2 files changed, 31 insertions(+), 24 deletions(-) diff --git a/app/controllers/users/passwords_controller.rb b/app/controllers/users/passwords_controller.rb index bb6ae7ffcb..7ed6168133 100644 --- a/app/controllers/users/passwords_controller.rb +++ b/app/controllers/users/passwords_controller.rb @@ -7,33 +7,18 @@ def create email = params[resource_name][:email] phone_number = params[resource_name][:phone_number] reset_token = nil - - if email.blank? && phone_number.blank? - resource.errors.add(:base, "Please enter at least one field.") - end - - if !email.blank? && !User.find_by(email: email) - resource.errors.add(:base, "Email not found") - end - - validation = valid_phone_number(phone_number) - if validation[0] - User.find_by(phone_number: phone_number) ? "" : resource.errors.add(:base, "Phone number not found") - else - resource.errors.add(:phone_number, validation[1]) - end + @resource = email.blank? ? User.find_by(phone_number: phone_number) : User.find_by(email: email) # re-render and display any errors - if resource.errors.any? - respond_with(resource) + params_is_valid, error_resource = password_params_is_valid(resource, email, phone_number) + if !params_is_valid + respond_with(error_resource) return end - # otherwise, send reset email and sms - @resource = email.blank? ? User.find_by(phone_number: phone_number) : User.find_by(email: email) + # generate a reset token and + # call devise mailer if !email.blank? - # generate a reset token and - # call devise mailer reset_token = @resource.send_reset_password_instructions end @@ -59,4 +44,27 @@ def create redirect_to after_sending_reset_password_instructions_path_for(resource_name), notice: "You will receive an email or SMS with instructions on how to reset your password in a few minutes." end + + private + + def password_params_is_valid(resource, email, phone_number) + if email.blank? && phone_number.blank? && resource + resource.errors.add(:base, "Please enter at least one field.") + return [false, resource] + end + + phone_number_is_valid, error_message = valid_phone_number(phone_number) + if !phone_number_is_valid && resource + resource.errors.add(:phone_number, error_message) + return [false, resource] + end + + if resource.email != email || resource.phone_number != phone_number + # A new, empty resource is returned (see application helper) + # so to check for nil, we need to check its email/phone fields + resource.errors.add(:base, "User does not exist.") + return [false, resource] + end + return [true, nil] + end end diff --git a/spec/system/devise/passwords/new_spec.rb b/spec/system/devise/passwords/new_spec.rb index ad09984bbd..54793a988b 100644 --- a/spec/system/devise/passwords/new_spec.rb +++ b/spec/system/devise/passwords/new_spec.rb @@ -11,9 +11,8 @@ fill_in "Phone number", with: "+16578900012" click_on "Send me reset password instructions" - expect(page).to have_content "2 errors prohibited this User from being saved:" - expect(page).to have_text("Email not found") - expect(page).to have_text("Phone number not found") + expect(page).to have_content "1 error prohibited this User from being saved:" + expect(page).to have_text("User does not exist.") end it "displays phone number error messages for incorrect formatting" do From f5b9b6ea28c8a06b77dbcda759b67daaa2589532 Mon Sep 17 00:00:00 2001 From: Xihai Luo Date: Sat, 16 Jul 2022 16:23:04 -0400 Subject: [PATCH 18/25] lint files --- app/controllers/users/passwords_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/users/passwords_controller.rb b/app/controllers/users/passwords_controller.rb index 7ed6168133..8fe93e2aef 100644 --- a/app/controllers/users/passwords_controller.rb +++ b/app/controllers/users/passwords_controller.rb @@ -65,6 +65,6 @@ def password_params_is_valid(resource, email, phone_number) resource.errors.add(:base, "User does not exist.") return [false, resource] end - return [true, nil] + [true, nil] end end From a90ec31287166fe7ffb39fe60bbdd2ef7d5ef4ea Mon Sep 17 00:00:00 2001 From: Xihai Luo Date: Sat, 16 Jul 2022 16:54:48 -0400 Subject: [PATCH 19/25] reduce cognitive complexity --- app/controllers/users/passwords_controller.rb | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/app/controllers/users/passwords_controller.rb b/app/controllers/users/passwords_controller.rb index 8fe93e2aef..a2a073c1c8 100644 --- a/app/controllers/users/passwords_controller.rb +++ b/app/controllers/users/passwords_controller.rb @@ -26,35 +26,29 @@ def create # when user enters ONLY a phone number, generate a new reset token to use; # otherwise, use the same reset token as sent by devise mailer reset_token ||= @resource.generate_password_reset_token - reset_password_link = request.base_url + "/users/password/edit?reset_password_token=#{reset_token}" - short_io_service = ShortUrlService.new + short_io_service = ShortUrlService.new.create_short_url(reset_password_link) twilio_service = TwilioService.new(@resource.casa_org.twilio_api_key_sid, @resource.casa_org.twilio_api_key_secret, @resource.casa_org.twilio_account_sid) - - short_io_service.create_short_url(reset_password_link) - body_msg = password_reset_msg(@resource.display_name, short_io_service.short_url) - sms_params = { From: @resource.casa_org.twilio_phone_number, - Body: body_msg, + Body: password_reset_msg(@resource.display_name, short_io_service.short_url), To: phone_number } twilio_service.send_sms(sms_params) end - redirect_to after_sending_reset_password_instructions_path_for(resource_name), notice: "You will receive an email or SMS with instructions on how to reset your password in a few minutes." end private def password_params_is_valid(resource, email, phone_number) - if email.blank? && phone_number.blank? && resource + if email.blank? && phone_number.blank? resource.errors.add(:base, "Please enter at least one field.") return [false, resource] end phone_number_is_valid, error_message = valid_phone_number(phone_number) - if !phone_number_is_valid && resource + if !phone_number_is_valid resource.errors.add(:phone_number, error_message) return [false, resource] end From 90c046822b62f628d0f873b165a17912303df457 Mon Sep 17 00:00:00 2001 From: Xihai Luo Date: Sat, 16 Jul 2022 17:28:34 -0400 Subject: [PATCH 20/25] migrate request spec to controller spec --- app/controllers/users/passwords_controller.rb | 3 ++- .../users/passwords_controller_spec.rb} | 8 +++----- 2 files changed, 5 insertions(+), 6 deletions(-) rename spec/{requests/users/passwords_spec.rb => controllers/users/passwords_controller_spec.rb} (74%) diff --git a/app/controllers/users/passwords_controller.rb b/app/controllers/users/passwords_controller.rb index a2a073c1c8..e06d28541a 100644 --- a/app/controllers/users/passwords_controller.rb +++ b/app/controllers/users/passwords_controller.rb @@ -27,7 +27,8 @@ def create # otherwise, use the same reset token as sent by devise mailer reset_token ||= @resource.generate_password_reset_token reset_password_link = request.base_url + "/users/password/edit?reset_password_token=#{reset_token}" - short_io_service = ShortUrlService.new.create_short_url(reset_password_link) + short_io_service = ShortUrlService.new() + short_io_service.create_short_url(reset_password_link) twilio_service = TwilioService.new(@resource.casa_org.twilio_api_key_sid, @resource.casa_org.twilio_api_key_secret, @resource.casa_org.twilio_account_sid) sms_params = { From: @resource.casa_org.twilio_phone_number, diff --git a/spec/requests/users/passwords_spec.rb b/spec/controllers/users/passwords_controller_spec.rb similarity index 74% rename from spec/requests/users/passwords_spec.rb rename to spec/controllers/users/passwords_controller_spec.rb index e635742323..1e213e3c68 100644 --- a/spec/requests/users/passwords_spec.rb +++ b/spec/controllers/users/passwords_controller_spec.rb @@ -1,8 +1,8 @@ require "rails_helper" require "support/stubbed_requests/webmock_helper" -RSpec.describe "/users/passwords", type: :request do - describe "POST /create" do +RSpec.describe Users::PasswordsController, type: :controller do + describe "create" do before do stubbed_sites = ["api.twilio.com", "api.short.io"] web_mock = WebMockHelper.new(stubbed_sites) @@ -23,12 +23,10 @@ } } - post "/users/password", params: params + post :create, params: params expect(@short_io_stub).to have_been_requested.times(1) expect(@twilio_stub).to have_been_requested.times(1) expect(response).to have_http_status(:redirect) - follow_redirect! - expect(flash[:notice]).to match(/You will receive an email or SMS with instructions on how to reset your password in a few minutes./) end end end From ee1954eb5b6d90856a5ff68430ec251e4b32e023 Mon Sep 17 00:00:00 2001 From: Xihai Luo Date: Sat, 16 Jul 2022 17:29:44 -0400 Subject: [PATCH 21/25] lint files --- app/controllers/users/passwords_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/users/passwords_controller.rb b/app/controllers/users/passwords_controller.rb index e06d28541a..bad858fb8c 100644 --- a/app/controllers/users/passwords_controller.rb +++ b/app/controllers/users/passwords_controller.rb @@ -27,7 +27,7 @@ def create # otherwise, use the same reset token as sent by devise mailer reset_token ||= @resource.generate_password_reset_token reset_password_link = request.base_url + "/users/password/edit?reset_password_token=#{reset_token}" - short_io_service = ShortUrlService.new() + short_io_service = ShortUrlService.new short_io_service.create_short_url(reset_password_link) twilio_service = TwilioService.new(@resource.casa_org.twilio_api_key_sid, @resource.casa_org.twilio_api_key_secret, @resource.casa_org.twilio_account_sid) sms_params = { From b6089041ce7178142f92bc405825081512538f33 Mon Sep 17 00:00:00 2001 From: Xihai Luo Date: Sat, 16 Jul 2022 17:43:45 -0400 Subject: [PATCH 22/25] refactor password controller for cognitive complexity again --- app/controllers/users/passwords_controller.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/controllers/users/passwords_controller.rb b/app/controllers/users/passwords_controller.rb index bad858fb8c..08880daacb 100644 --- a/app/controllers/users/passwords_controller.rb +++ b/app/controllers/users/passwords_controller.rb @@ -26,9 +26,8 @@ def create # when user enters ONLY a phone number, generate a new reset token to use; # otherwise, use the same reset token as sent by devise mailer reset_token ||= @resource.generate_password_reset_token - reset_password_link = request.base_url + "/users/password/edit?reset_password_token=#{reset_token}" short_io_service = ShortUrlService.new - short_io_service.create_short_url(reset_password_link) + short_io_service.create_short_url(request.base_url + "/users/password/edit?reset_password_token=#{reset_token}") twilio_service = TwilioService.new(@resource.casa_org.twilio_api_key_sid, @resource.casa_org.twilio_api_key_secret, @resource.casa_org.twilio_account_sid) sms_params = { From: @resource.casa_org.twilio_phone_number, From a363799dc6c8547bd33495fdcba579f2425c646e Mon Sep 17 00:00:00 2001 From: Xihai Luo Date: Sun, 17 Jul 2022 11:51:14 -0400 Subject: [PATCH 23/25] refactor req params fetching to reduce brain complexity --- app/controllers/users/passwords_controller.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/controllers/users/passwords_controller.rb b/app/controllers/users/passwords_controller.rb index 08880daacb..295dbbc321 100644 --- a/app/controllers/users/passwords_controller.rb +++ b/app/controllers/users/passwords_controller.rb @@ -4,8 +4,7 @@ class Users::PasswordsController < Devise::PasswordsController include SmsBodyHelper def create - email = params[resource_name][:email] - phone_number = params[resource_name][:phone_number] + email, phone_number = [params[resource_name][:email], params[resource_name][:phone_number]] reset_token = nil @resource = email.blank? ? User.find_by(phone_number: phone_number) : User.find_by(email: email) From 45bcefe52bfc8dd51ce557c3fca7dd7be3674906 Mon Sep 17 00:00:00 2001 From: Xihai Luo Date: Sun, 17 Jul 2022 12:06:27 -0400 Subject: [PATCH 24/25] 99% sure brain complexity is reduced in passwords controller --- app/controllers/users/passwords_controller.rb | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/app/controllers/users/passwords_controller.rb b/app/controllers/users/passwords_controller.rb index 295dbbc321..0108f259ad 100644 --- a/app/controllers/users/passwords_controller.rb +++ b/app/controllers/users/passwords_controller.rb @@ -5,7 +5,6 @@ class Users::PasswordsController < Devise::PasswordsController def create email, phone_number = [params[resource_name][:email], params[resource_name][:phone_number]] - reset_token = nil @resource = email.blank? ? User.find_by(phone_number: phone_number) : User.find_by(email: email) # re-render and display any errors @@ -17,29 +16,38 @@ def create # generate a reset token and # call devise mailer + reset_token = send_email_reset(email) + send_sms_reset(@resource, phone_number, reset_token) + redirect_to after_sending_reset_password_instructions_path_for(resource_name), notice: "You will receive an email or SMS with instructions on how to reset your password in a few minutes." + end + + private + + def send_email_reset(email) + reset_token = nil if !email.blank? reset_token = @resource.send_reset_password_instructions end + reset_token + end + def send_sms_reset(resource, phone_number, reset_token) if !phone_number.blank? # when user enters ONLY a phone number, generate a new reset token to use; # otherwise, use the same reset token as sent by devise mailer - reset_token ||= @resource.generate_password_reset_token + reset_token ||= resource.generate_password_reset_token short_io_service = ShortUrlService.new short_io_service.create_short_url(request.base_url + "/users/password/edit?reset_password_token=#{reset_token}") - twilio_service = TwilioService.new(@resource.casa_org.twilio_api_key_sid, @resource.casa_org.twilio_api_key_secret, @resource.casa_org.twilio_account_sid) + twilio_service = TwilioService.new(resource.casa_org.twilio_api_key_sid, resource.casa_org.twilio_api_key_secret, resource.casa_org.twilio_account_sid) sms_params = { - From: @resource.casa_org.twilio_phone_number, - Body: password_reset_msg(@resource.display_name, short_io_service.short_url), + From: resource.casa_org.twilio_phone_number, + Body: password_reset_msg(resource.display_name, short_io_service.short_url), To: phone_number } twilio_service.send_sms(sms_params) end - redirect_to after_sending_reset_password_instructions_path_for(resource_name), notice: "You will receive an email or SMS with instructions on how to reset your password in a few minutes." end - private - def password_params_is_valid(resource, email, phone_number) if email.blank? && phone_number.blank? resource.errors.add(:base, "Please enter at least one field.") From 3fc478deceb0475555aaca01becf9ec7f856d5c2 Mon Sep 17 00:00:00 2001 From: Xihai Luo Date: Tue, 19 Jul 2022 10:09:07 -0400 Subject: [PATCH 25/25] recreate db so schema is up to date with latest changes - should fix remaining rspec faliures --- app/controllers/users/passwords_controller.rb | 4 +- app/models/casa_admin.rb | 2 +- db/schema.rb | 47 ++++++++++--------- 3 files changed, 27 insertions(+), 26 deletions(-) diff --git a/app/controllers/users/passwords_controller.rb b/app/controllers/users/passwords_controller.rb index 0108f259ad..b18c97ebd4 100644 --- a/app/controllers/users/passwords_controller.rb +++ b/app/controllers/users/passwords_controller.rb @@ -17,6 +17,8 @@ def create # generate a reset token and # call devise mailer reset_token = send_email_reset(email) + # for case where user enters ONLY a phone number, generate a new reset token to use; + # otherwise, use the same reset token as sent by devise mailer send_sms_reset(@resource, phone_number, reset_token) redirect_to after_sending_reset_password_instructions_path_for(resource_name), notice: "You will receive an email or SMS with instructions on how to reset your password in a few minutes." end @@ -33,8 +35,6 @@ def send_email_reset(email) def send_sms_reset(resource, phone_number, reset_token) if !phone_number.blank? - # when user enters ONLY a phone number, generate a new reset token to use; - # otherwise, use the same reset token as sent by devise mailer reset_token ||= resource.generate_password_reset_token short_io_service = ShortUrlService.new short_io_service.create_short_url(request.base_url + "/users/password/edit?reset_password_token=#{reset_token}") diff --git a/app/models/casa_admin.rb b/app/models/casa_admin.rb index 511bc62a3a..c0dbf6ef5b 100644 --- a/app/models/casa_admin.rb +++ b/app/models/casa_admin.rb @@ -46,7 +46,7 @@ def change_to_supervisor! # created_at :datetime not null # updated_at :datetime not null # casa_org_id :bigint not null -# invited_by_id :integer +# invited_by_id :bigint # # Indexes # diff --git a/db/schema.rb b/db/schema.rb index 89d530e0b5..32a76b2236 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -37,7 +37,7 @@ end create_table "active_storage_variant_records", force: :cascade do |t| - t.integer "blob_id", null: false + t.bigint "blob_id", null: false t.string "variation_digest", null: false t.index ["blob_id", "variation_digest"], name: "index_active_storage_variant_records_uniqueness", unique: true end @@ -79,8 +79,8 @@ end create_table "casa_case_contact_types", force: :cascade do |t| - t.integer "contact_type_id", null: false - t.integer "casa_case_id", null: false + t.bigint "contact_type_id", null: false + t.bigint "casa_case_id", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false t.index ["casa_case_id"], name: "index_casa_case_contact_types_on_casa_case_id" @@ -136,17 +136,17 @@ t.string "footer_links", default: [], array: true t.string "slug" t.boolean "show_driving_reimbursement", default: true + t.boolean "show_fund_request", default: false t.string "twilio_phone_number" t.string "twilio_account_sid" t.string "twilio_api_key_sid" t.string "twilio_api_key_secret" - t.boolean "show_fund_request", default: false t.index ["slug"], name: "index_casa_orgs_on_slug", unique: true end create_table "case_assignments", force: :cascade do |t| - t.integer "casa_case_id", null: false - t.integer "volunteer_id", null: false + t.bigint "casa_case_id", null: false + t.bigint "volunteer_id", null: false t.boolean "active", default: true, null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false @@ -156,8 +156,8 @@ end create_table "case_contact_contact_types", force: :cascade do |t| - t.integer "case_contact_id", null: false - t.integer "contact_type_id", null: false + t.bigint "case_contact_id", null: false + t.bigint "contact_type_id", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false t.index ["case_contact_id"], name: "index_case_contact_contact_types_on_case_contact_id" @@ -165,8 +165,8 @@ end create_table "case_contacts", force: :cascade do |t| - t.integer "creator_id", null: false - t.integer "casa_case_id", null: false + t.bigint "creator_id", null: false + t.bigint "casa_case_id", null: false t.integer "duration_minutes", null: false t.datetime "occurred_at", precision: nil, null: false t.datetime "created_at", null: false @@ -206,7 +206,7 @@ end create_table "contact_type_groups", force: :cascade do |t| - t.integer "casa_org_id", null: false + t.bigint "casa_org_id", null: false t.string "name", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false @@ -215,7 +215,7 @@ end create_table "contact_types", force: :cascade do |t| - t.integer "contact_type_group_id", null: false + t.bigint "contact_type_group_id", null: false t.string "name", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false @@ -225,7 +225,7 @@ create_table "court_dates", force: :cascade do |t| t.datetime "date", precision: nil, null: false - t.integer "casa_case_id", null: false + t.bigint "casa_case_id", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false t.bigint "hearing_type_id" @@ -259,7 +259,7 @@ end create_table "emancipation_options", force: :cascade do |t| - t.integer "emancipation_category_id", null: false + t.bigint "emancipation_category_id", null: false t.string "name", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false @@ -309,7 +309,7 @@ end create_table "hearing_types", force: :cascade do |t| - t.integer "casa_org_id", null: false + t.bigint "casa_org_id", null: false t.string "name", null: false t.boolean "active", default: true, null: false t.string "checklist_updated_date", default: "None", null: false @@ -317,7 +317,7 @@ end create_table "judges", force: :cascade do |t| - t.integer "casa_org_id", null: false + t.bigint "casa_org_id", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false t.boolean "active", default: true @@ -433,8 +433,8 @@ end create_table "supervisor_volunteers", force: :cascade do |t| - t.integer "supervisor_id", null: false - t.integer "volunteer_id", null: false + t.bigint "supervisor_id", null: false + t.bigint "volunteer_id", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false t.boolean "is_active", default: true @@ -446,12 +446,13 @@ t.string "version", null: false end - create_table "user_case_contact_types_reminders", force: :cascade do |t| + create_table "user_reminder_times", force: :cascade do |t| t.bigint "user_id", null: false - t.datetime "reminder_sent" + t.datetime "case_contact_types" + t.datetime "no_contact_made" t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.index ["user_id"], name: "index_user_case_contact_types_reminders_on_user_id" + t.index ["user_id"], name: "index_user_reminder_times_on_user_id" end create_table "user_sms_notification_events", force: :cascade do |t| @@ -478,7 +479,7 @@ t.datetime "invitation_accepted_at", precision: nil t.integer "invitation_limit" t.string "invited_by_type" - t.integer "invited_by_id" + t.bigint "invited_by_id" t.integer "invitations_count", default: 0 t.string "type" t.boolean "active", default: true @@ -526,7 +527,7 @@ add_foreign_key "sent_emails", "users" add_foreign_key "supervisor_volunteers", "users", column: "supervisor_id" add_foreign_key "supervisor_volunteers", "users", column: "volunteer_id" - add_foreign_key "user_case_contact_types_reminders", "users" + add_foreign_key "user_reminder_times", "users" add_foreign_key "user_sms_notification_events", "sms_notification_events" add_foreign_key "user_sms_notification_events", "users" add_foreign_key "users", "casa_orgs"