diff --git a/app/controllers/casa_admins_controller.rb b/app/controllers/casa_admins_controller.rb index c97930f9a7..c6d193ab35 100644 --- a/app/controllers/casa_admins_controller.rb +++ b/app/controllers/casa_admins_controller.rb @@ -123,6 +123,6 @@ def set_admin end def update_casa_admin_params - CasaAdminParameters.new(params).with_only(:email, :display_name) + CasaAdminParameters.new(params).with_only(:email, :display_name, :phone_number) end end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 9b64eb65ec..78c7b5c3d4 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -60,9 +60,9 @@ def update_user_password def user_params if current_user.casa_admin? - params.require(:user).permit(:email, :display_name, :receive_sms_notifications, :receive_email_notifications) + params.require(:user).permit(:email, :display_name, :phone_number, :receive_sms_notifications, :receive_email_notifications) else - params.require(:user).permit(:display_name, :receive_sms_notifications, :receive_email_notifications) + params.require(:user).permit(:display_name, :phone_number, :receive_sms_notifications, :receive_email_notifications) end end 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 1a8c7f3816..3ce92ffd98 100644 --- a/app/models/casa_case_contact_type.rb +++ b/app/models/casa_case_contact_type.rb @@ -13,8 +13,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 4898c870ac..0bc81d1fdb 100644 --- a/app/models/case_assignment.rb +++ b/app/models/case_assignment.rb @@ -37,8 +37,8 @@ def casa_case_and_volunteer_must_belong_to_same_casa_org # active :boolean default(TRUE), not null # 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 8427ccb701..00da20f61c 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 205ea49de2..57da9ba9f4 100644 --- a/app/models/case_contact_contact_type.rb +++ b/app/models/case_contact_contact_type.rb @@ -13,8 +13,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 6c0001402f..fe862eb4a7 100644 --- a/app/models/contact_type.rb +++ b/app/models/contact_type.rb @@ -22,7 +22,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 f977a2f1ee..7f81398ecc 100644 --- a/app/models/contact_type_group.rb +++ b/app/models/contact_type_group.rb @@ -22,7 +22,7 @@ class ContactTypeGroup < ApplicationRecord # 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 11cdbf1768..509c34e04e 100644 --- a/app/models/hearing_type.rb +++ b/app/models/hearing_type.rb @@ -17,7 +17,7 @@ class HearingType < ApplicationRecord # id :bigint not null, primary key # active :boolean default(TRUE), 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 dda04bc1e2..b6a458497e 100644 --- a/app/models/judge.rb +++ b/app/models/judge.rb @@ -18,7 +18,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 73c0c415c7..3a8f387453 100644 --- a/app/models/supervisor_volunteer.rb +++ b/app/models/supervisor_volunteer.rb @@ -25,8 +25,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 22c4c8897e..dba2c6a58c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -12,6 +12,8 @@ class User < ApplicationRecord validates :email, presence: true validates :display_name, presence: true + + validates_with UserValidator validate :at_least_one_communication_preference_selected belongs_to :casa_org @@ -192,7 +194,7 @@ def last_deactivated_by # 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 2a384ead3c..27ef1380fc 100644 --- a/app/models/volunteer.rb +++ b/app/models/volunteer.rb @@ -153,7 +153,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/app/policies/user_policy.rb b/app/policies/user_policy.rb index 20efc3c3ce..8fcd480ae1 100644 --- a/app/policies/user_policy.rb +++ b/app/policies/user_policy.rb @@ -25,6 +25,14 @@ def update_supervisor_name? update_supervisor_email? end + def update_user_setting? + if is_supervisor_same_org? + # allow access to own record or volunteer record + return record == user || record.volunteer? + end + is_admin? + end + def edit_name?(viewed_user) is_admin? || viewed_user == user end diff --git a/app/validators/user_validator.rb b/app/validators/user_validator.rb index 89aee34a4e..a9f9f0b91c 100644 --- a/app/validators/user_validator.rb +++ b/app/validators/user_validator.rb @@ -1,29 +1,33 @@ class UserValidator < ActiveModel::Validator VALID_PHONE_NUMBER_LENGTH = 12 VALID_COUNTRY_CODE = "+1" - PHONE_NUMBER_ERROR = "Phone number is not in correct format: 1XXXXXXXXXX" def validate(record) - if !valid_phone_number_contents(record.phone_number) - record.errors.add(:phone_number, PHONE_NUMBER_ERROR) - end + valid_phone_number_contents(record.phone_number, record) end private - def valid_phone_number_contents(number) + def valid_phone_number_contents(number, record) if number.empty? return true end if number.length != VALID_PHONE_NUMBER_LENGTH + record.errors.add(:phone_number, " must be 12 digits including country code (+1)") return false end country_code = number[0..1] phone_number = number[2..number.length] - if country_code != VALID_COUNTRY_CODE || !phone_number.scan(/\D/).empty? + if country_code != VALID_COUNTRY_CODE + record.errors.add(:phone_number, " must have a valid country code (+1)") + return false + end + + if !phone_number.scan(/\D/).empty? + record.errors.add(:phone_number, " must have correct format") return false end diff --git a/app/values/user_parameters.rb b/app/values/user_parameters.rb index 39523adeb3..52de01b7f8 100644 --- a/app/values/user_parameters.rb +++ b/app/values/user_parameters.rb @@ -5,6 +5,7 @@ def initialize(params, key = :user) :email, :casa_org_id, :display_name, + :phone_number, :password, :active, :type diff --git a/app/views/casa_admins/_form.html.erb b/app/views/casa_admins/_form.html.erb index 7cda79e709..5d1d72f92b 100644 --- a/app/views/casa_admins/_form.html.erb +++ b/app/views/casa_admins/_form.html.erb @@ -5,15 +5,7 @@ <%= form_with(model: casa_admin, local: true) do |form| %> <%= render "/shared/error_messages", resource: casa_admin %> -
- <%= form.label :email, t("common.email") %> - <%= form.text_field :email, class: "form-control" %> -
- -
- <%= form.label :display_name, t("common.display_name") %> - <%= form.text_field :display_name, class: "form-control" %> -
+ <%= render "/shared/edit_form", resource: casa_admin, f: form %> <%= render "/shared/invite_login", resource: casa_admin %> diff --git a/app/views/shared/_edit_form.html.erb b/app/views/shared/_edit_form.html.erb new file mode 100644 index 0000000000..d5f4c9b6fa --- /dev/null +++ b/app/views/shared/_edit_form.html.erb @@ -0,0 +1,26 @@ +
+ <%= f.label :email, t("common.email") %> + <% if policy(resource).update_user_setting? %> + <%= f.text_field :email, class: "form-control" %> + <% else %> + + <% end %> +
+ +
+ <%= f.label :display_name, t("common.display_name") %> + <% if policy(resource).update_user_setting? %> + <%= f.text_field :display_name, class: "form-control" %> + <% else %> + + <% end %> +
+ +
+ <%= f.label :phone_number, t("common.phone_number") %> + <% if policy(resource).update_user_setting? %> + <%= f.text_field :phone_number, class: "form-control" %> + <% else %> + + <% end %> +
diff --git a/app/views/supervisors/edit.html.erb b/app/views/supervisors/edit.html.erb index da2e78c3e7..c8de7fc78c 100644 --- a/app/views/supervisors/edit.html.erb +++ b/app/views/supervisors/edit.html.erb @@ -5,23 +5,7 @@ <%= form_for @supervisor, as: :supervisor, url: supervisor_path(@supervisor) do |form| %> <%= render "/shared/error_messages", resource: @supervisor %> -
- <%= form.label :email, t("common.email") %> - <% if policy(@supervisor).update_supervisor_email? %> - <%= form.text_field :email, class: "form-control" %> - <% else %> - - <% end %> -
- -
- <%= form.label :display_name, t("common.display_name") %> - <% if policy(@supervisor).update_supervisor_name? %> - <%= form.text_field :display_name, class: "form-control" %> - <% else %> - - <% end %> -
+ <%= render "/shared/edit_form", resource: @supervisor, f: form %>

<%= render "/shared/invite_login", resource: @supervisor %> diff --git a/app/views/users/edit.html.erb b/app/views/users/edit.html.erb index 24025cee64..456af527ff 100644 --- a/app/views/users/edit.html.erb +++ b/app/views/users/edit.html.erb @@ -13,6 +13,11 @@ <%= form.text_field :display_name, class: "form-control" %> +

+ <%= form.label :phone_number, t("common.phone_number") %> + <%= form.text_field :phone_number, class: "form-control" %> +
+ <%= form.hidden_field :casa_org_id, value: current_user.casa_org_id %> <%= render "/shared/invite_login", resource: current_user %> diff --git a/app/views/volunteers/edit.html.erb b/app/views/volunteers/edit.html.erb index 7ab397ac01..fd2cba8b82 100644 --- a/app/views/volunteers/edit.html.erb +++ b/app/views/volunteers/edit.html.erb @@ -15,19 +15,7 @@ <%= form_for @volunteer, url: volunteer_path(@volunteer) do |form| %> <%= render "/shared/error_messages", resource: @volunteer %> -
- <%= form.label :email, t("common.email") %> - <% if policy(:volunteer).update_volunteer_email? %> - <%= form.text_field :email, class: "form-control" %> - <% else %> - - <% end %> -
- -
- <%= form.label :display_name, t("common.display_name") %> - <%= form.text_field :display_name, class: "form-control" %> -
+ <%= render "/shared/edit_form", resource: @volunteer, f: form %> <%= render "/shared/invite_login", resource: @volunteer %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 4526d0137a..d67ab754ae 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -94,6 +94,7 @@ en: email: Email name: Name display_name: Display name + phone_number: Phone number address: Address date: formats: diff --git a/db/schema.rb b/db/schema.rb index a1860475e6..49456d77e8 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -127,7 +127,6 @@ 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.index ["slug"], name: "index_casa_orgs_on_slug", unique: true end diff --git a/spec/controllers/volunteers_controller_spec.rb b/spec/controllers/volunteers_controller_spec.rb index 225cc63ad5..f2edc1a7db 100644 --- a/spec/controllers/volunteers_controller_spec.rb +++ b/spec/controllers/volunteers_controller_spec.rb @@ -2,28 +2,37 @@ RSpec.describe VolunteersController, type: :controller do describe "update" do + before :each do + @east = create :casa_org, name: "East" + @west = create :casa_org, name: "West" + end + + def log_in_as_supervisor + @supervisor = create :supervisor, casa_org: @east + allow(controller).to receive(:current_user).and_return(@supervisor) + end + + def log_in_as_admin + @admin = create :casa_admin + allow(controller).to receive(:current_user).and_return(@admin) + end + context "as a supervisor" do it "only shows volunteers for the same casa org" do - east = create :casa_org, name: "East" - west = create :casa_org, name: "West" - supervisor = create :supervisor, casa_org: east allow(controller).to receive(:authenticate_user!).and_return(:supervisor) - allow(controller).to receive(:current_user).and_return(supervisor) - volunteer_east = create :volunteer, casa_org: east - _volunteer_west = create :volunteer, casa_org: west + log_in_as_supervisor + volunteer_east = create :volunteer, casa_org: @east + _volunteer_west = create :volunteer, casa_org: @west get :edit, params: {id: volunteer_east.id} expect(response).to have_http_status(:ok) - expect(assigns(:supervisors)).to eq([supervisor]) + expect(assigns(:supervisors)).to eq([@supervisor]) expect(assigns(:volunteer)).to eq(volunteer_east) end it "cannot edit volunteer for a different casa org" do - east = create :casa_org, name: "East" - west = create :casa_org, name: "West" - supervisor = create :supervisor, casa_org: east allow(controller).to receive(:authenticate_user!).and_return(:supervisor) - allow(controller).to receive(:current_user).and_return(supervisor) - volunteer_west = create :volunteer, casa_org: west + log_in_as_supervisor + volunteer_west = create :volunteer, casa_org: @west get :edit, params: {id: volunteer_west.id} expect(response).to redirect_to("/") end diff --git a/spec/factories/user.rb b/spec/factories/user.rb index 64e94afc8c..b72d004358 100644 --- a/spec/factories/user.rb +++ b/spec/factories/user.rb @@ -6,6 +6,7 @@ password { "12345678" } password_confirmation { "12345678" } case_assignments { [] } + phone_number { "" } trait :inactive do volunteer diff --git a/spec/lib/importers/supervisor_importer_spec.rb b/spec/lib/importers/supervisor_importer_spec.rb index d4d46d25f0..313c139e58 100644 --- a/spec/lib/importers/supervisor_importer_spec.rb +++ b/spec/lib/importers/supervisor_importer_spec.rb @@ -119,7 +119,7 @@ expect(alert[:type]).to eq(:error) expect(alert[:message]).to eq("Not all rows were imported.") - expect(alert[:exported_rows]).to include("Phone number is not in correct format: 1XXXXXXXXXX") + expect(alert[:exported_rows]).to include("Phone number must be 12 digits including country code (+1)") end end diff --git a/spec/lib/importers/volunteer_importer_spec.rb b/spec/lib/importers/volunteer_importer_spec.rb index 8f50c1523e..daa0b15df9 100644 --- a/spec/lib/importers/volunteer_importer_spec.rb +++ b/spec/lib/importers/volunteer_importer_spec.rb @@ -91,7 +91,7 @@ expect(alert[:type]).to eq(:error) expect(alert[:message]).to eq("Not all rows were imported.") - expect(alert[:exported_rows]).to include("Phone number is not in correct format: 1XXXXXXXXXX") + expect(alert[:exported_rows]).to include("Phone number must be 12 digits including country code (+1)") end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index eb855dcf92..a501e2bb86 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -15,9 +15,31 @@ it { is_expected.to have_many(:notes) } - it "requires display name" do - user = build(:user, display_name: "") - expect(user.valid?).to be false + describe "model validations" do + it "requires display name" do + user = build(:user, display_name: "") + expect(user.valid?).to be false + end + + it "requires email" do + user = build(:user, email: "") + expect(user.valid?).to be false + end + + it "requires 12 digit phone numbers" do + user = build(:user, phone_number: "+1416321809") + expect(user.valid?).to be false + end + + it "requires phone number to only contain numbers" do + user = build(:user, phone_number: "+1416eee4325") + expect(user.valid?).to be false + end + + it "requires phone number with US area code" do + user = build(:user, phone_number: "+76758890432") + expect(user.valid?).to be false + end end describe "#case_contacts_for" do diff --git a/spec/policies/user_policy_spec.rb b/spec/policies/user_policy_spec.rb index 83117297ae..1274a2afb3 100644 --- a/spec/policies/user_policy_spec.rb +++ b/spec/policies/user_policy_spec.rb @@ -7,6 +7,11 @@ let(:volunteer) { build_stubbed(:volunteer) } let(:supervisor) { build_stubbed(:supervisor) } + let(:org_b) { build_stubbed(:casa_org) } + let(:casa_admin_b) { build_stubbed(:casa_admin, casa_org: org_b) } + let(:supervisor_b) { build_stubbed(:supervisor, casa_org: org_b) } + let(:volunteer_b) { build_stubbed(:volunteer, casa_org: org_b) } + permissions :edit?, :update?, :update_password? do it "allows casa_admins" do is_expected.to permit(casa_admin) @@ -20,4 +25,31 @@ is_expected.to permit(volunteer) end end + + permissions :update_user_setting? do + context "when user is an admin" do + it "allows update settings of all roles" do + is_expected.to permit(casa_admin) + end + end + + context "when user is a supervisor" do + it "allows supervisors to update another volunteer settings" do + is_expected.to permit(supervisor, volunteer) + end + + it "does not allow supervisor to update a volunteer in a different casa org" do + is_expected.not_to permit(supervisor, volunteer_b) + end + + it "allows supervisors to update their own settings" do + is_expected.to permit(supervisor, supervisor) + end + + it "does not allow supervisor to update another supervisor settings" do + another_supervisor = build_stubbed(:supervisor) + is_expected.not_to permit(supervisor, another_supervisor) + end + end + end end diff --git a/spec/requests/casa_admins_spec.rb b/spec/requests/casa_admins_spec.rb index ab9fd7c983..b92f75cf67 100644 --- a/spec/requests/casa_admins_spec.rb +++ b/spec/requests/casa_admins_spec.rb @@ -37,6 +37,7 @@ let(:casa_admin) { create(:casa_admin) } let(:expected_display_name) { "Admin 2" } let(:expected_email) { "admin2@casa.com" } + let(:expected_phone_number) { "+14163218092" } before do sign_in_as_admin @@ -46,13 +47,15 @@ put casa_admin_path(casa_admin), params: { casa_admin: { email: expected_email, - display_name: expected_display_name + display_name: expected_display_name, + phone_number: expected_phone_number } } casa_admin.reload expect(casa_admin.email).to eq expected_email expect(casa_admin.display_name).to eq expected_display_name + expect(casa_admin.phone_number).to eq expected_phone_number expect(response).to redirect_to casa_admins_path expect(response.request.flash[:notice]).to eq "New admin created successfully" end @@ -80,11 +83,13 @@ it "cannot update the casa admin", :aggregate_failures do put casa_admin_path(casa_admin), params: { - casa_admin: {email: nil} + casa_admin: {email: nil}, + phone_number: {phone_number: "dsadw323"} } casa_admin.reload expect(casa_admin.email).not_to eq nil + expect(casa_admin.phone_number).not_to eq "dsadw323" expect(response).to render_template :edit end diff --git a/spec/requests/supervisors_spec.rb b/spec/requests/supervisors_spec.rb index b3d49e636c..9e821e9a63 100644 --- a/spec/requests/supervisors_spec.rb +++ b/spec/requests/supervisors_spec.rb @@ -7,7 +7,7 @@ let(:supervisor) { create(:supervisor) } let(:update_supervisor_params) do - {supervisor: {email: "newemail@gmail.com", display_name: "New Name"}} + {supervisor: {email: "newemail@gmail.com", display_name: "New Name", phone_number: "+14163218092"}} end describe "GET /new" do @@ -84,6 +84,7 @@ expect(supervisor.display_name).to eq "New Name" expect(supervisor.email).to eq "newemail@gmail.com" + expect(supervisor.phone_number).to eq "+14163218092" end it "can set the supervisor to be inactive" do diff --git a/spec/requests/volunteers_spec.rb b/spec/requests/volunteers_spec.rb index 6b40065a03..a8ca19417b 100644 --- a/spec/requests/volunteers_spec.rb +++ b/spec/requests/volunteers_spec.rb @@ -154,13 +154,14 @@ context "with valid params" do it "updates the volunteer" do patch volunteer_path(volunteer), params: { - volunteer: {email: "newemail@gmail.com", display_name: "New Name"} + volunteer: {email: "newemail@gmail.com", display_name: "New Name", phone_number: "+15463457898"} } expect(response).to have_http_status(:redirect) volunteer.reload expect(volunteer.display_name).to eq "New Name" expect(volunteer.email).to eq "newemail@gmail.com" + expect(volunteer.phone_number).to eq "+15463457898" end end @@ -171,13 +172,14 @@ volunteer.supervisor = build(:supervisor) patch volunteer_path(volunteer), params: { - volunteer: {email: other_volunteer.email, display_name: "New Name"} + volunteer: {email: other_volunteer.email, display_name: "New Name", phone_number: "+15463457898"} } expect(response).to have_http_status(:success) # Re-renders form volunteer.reload expect(volunteer.display_name).to_not eq "New Name" expect(volunteer.email).to_not eq other_volunteer.email + expect(volunteer.phone_number).to_not eq "+15463457898" end end diff --git a/spec/support/shared_examples/shows_error_for_invalid_phone_numbers.rb b/spec/support/shared_examples/shows_error_for_invalid_phone_numbers.rb new file mode 100644 index 0000000000..140e153b9f --- /dev/null +++ b/spec/support/shared_examples/shows_error_for_invalid_phone_numbers.rb @@ -0,0 +1,25 @@ +shared_examples_for "shows error for invalid phone numbers" do + it "shows error message for phone number < 12 digits" do + role == "admin" || role == "user" ? fill_in("Phone number", with: "+141632489") : fill_in("#{role}_phone_number", with: "+141632489") + role == "user" ? click_on("Update Profile") : click_on("Submit") + expect(page).to have_text "Phone number must be 12 digits including country code (+1)" + end + + it "shows error message for phone number > 12 digits" do + role == "admin" || role == "user" ? fill_in("Phone number", with: "+141632180923") : fill_in("#{role}_phone_number", with: "+141632180923") + role == "user" ? click_on("Update Profile") : click_on("Submit") + expect(page).to have_text "Phone number must be 12 digits including country code (+1)" + end + + it "shows error message for bad phone number" do + role == "admin" || role == "user" ? fill_in("Phone number", with: "+141632u809o") : fill_in("#{role}_phone_number", with: "+141632u809o") + role == "user" ? click_on("Update Profile") : click_on("Submit") + expect(page).to have_text "Phone number must have correct format" + end + + it "shows error message for phone number without country code" do + role == "admin" || role == "user" ? fill_in("Phone number", with: "+24163218092") : fill_in("#{role}_phone_number", with: "+24163218092") + role == "user" ? click_on("Update Profile") : click_on("Submit") + expect(page).to have_text "Phone number must have a valid country code (+1)" + end +end diff --git a/spec/system/casa_admins/edit_spec.rb b/spec/system/casa_admins/edit_spec.rb index ac348ec16c..216d5d4128 100644 --- a/spec/system/casa_admins/edit_spec.rb +++ b/spec/system/casa_admins/edit_spec.rb @@ -9,11 +9,13 @@ it "can successfully edit user email and display name" do expected_email = "root@casa.com" expected_display_name = "Root Admin" + expected_phone_number = "+14398761234" visit edit_casa_admin_path(admin) fill_in "Email", with: expected_email fill_in "Display name", with: expected_display_name + fill_in "Phone number", with: expected_phone_number click_on "Submit" @@ -22,13 +24,21 @@ expect(page).to have_text "New admin created successfully" expect(admin.email).to eq expected_email expect(admin.display_name).to eq expected_display_name + expect(admin.phone_number).to eq expected_phone_number end end context "with invalid data" do - it "shows error message for empty email" do + let(:role) { "admin" } + before do visit edit_casa_admin_path(admin) + fill_in "Email", with: "newemail@example.com" + fill_in "Display name", with: "Kaedehara Kazuha" + end + it_should_behave_like "shows error for invalid phone numbers" + + it "shows error message for empty email" do fill_in "Email", with: "" fill_in "Display name", with: "" diff --git a/spec/system/supervisors/edit_spec.rb b/spec/system/supervisors/edit_spec.rb index d2c60802cf..dc8eb39958 100644 --- a/spec/system/supervisors/edit_spec.rb +++ b/spec/system/supervisors/edit_spec.rb @@ -36,6 +36,16 @@ expect(page).to have_text("Editing Supervisor") end + context "with invalid data" do + let(:role) { "supervisor" } + let(:supervisor) { create(:supervisor, display_name: "Leslie Knope", casa_org: organization) } + before do + sign_in user + visit edit_supervisor_path(supervisor) + end + it_should_behave_like "shows error for invalid phone numbers" + end + it "can go to the supervisor edit page and see red message when there are no active volunteers" do supervisor = create :supervisor, casa_org: organization diff --git a/spec/system/users/edit_spec.rb b/spec/system/users/edit_spec.rb index 5f737392b2..869d67e7d7 100644 --- a/spec/system/users/edit_spec.rb +++ b/spec/system/users/edit_spec.rb @@ -127,6 +127,7 @@ end context "when admin" do + let(:role) { "user" } before do sign_in admin visit edit_users_path @@ -138,6 +139,8 @@ expect(page).to have_text("Display name can't be blank") end + it_should_behave_like "shows error for invalid phone numbers" + it "is able to update the email if user is a admin" do expect(page).to have_field("Email", disabled: false) fill_in "Email", with: "new_admin@example.com" diff --git a/spec/system/volunteers/edit_spec.rb b/spec/system/volunteers/edit_spec.rb index 193c613441..d299f6cc6d 100644 --- a/spec/system/volunteers/edit_spec.rb +++ b/spec/system/volunteers/edit_spec.rb @@ -19,18 +19,22 @@ before do sign_in admin visit edit_volunteer_path(volunteer) + fill_in "volunteer_email", with: "newemail@example.com" + fill_in "volunteer_display_name", with: "Kamisato Ayato" end context "with valid data" do it "updates successfully" do - fill_in "volunteer_email", with: "newemail@example.com" - fill_in "volunteer_display_name", with: "Mickey Mouse" click_on "Submit" expect(page).to have_text "Volunteer was successfully updated." end end context "with invalid data" do + let(:role) { "volunteer" } + + it_should_behave_like "shows error for invalid phone numbers" + it "shows error message for duplicate email" do volunteer.supervisor = build(:supervisor) fill_in "volunteer_email", with: admin.email diff --git a/spec/views/supervisors/edit.html.erb_spec.rb b/spec/views/supervisors/edit.html.erb_spec.rb index 7268f8881d..c1822e9fd4 100644 --- a/spec/views/supervisors/edit.html.erb_spec.rb +++ b/spec/views/supervisors/edit.html.erb_spec.rb @@ -60,6 +60,34 @@ expect(rendered).to have_text("Password reset last sent \n never") end + it "shows profile info form fields as editable for a supervisor editing their own profile" do + enable_pundit(view, supervisor) + + assign :supervisor, supervisor + assign :all_volunteers_ever_assigned, [volunteer] + assign :available_volunteers, [] + + render template: "supervisors/edit" + + expect(rendered).to have_field("supervisor_email") + expect(rendered).to have_field("supervisor_display_name") + expect(rendered).to have_field("supervisor_phone_number") + end + + it "shows profile info form fields as disabled for a supervisor editing another supervisor" do + enable_pundit(view, supervisor) + + assign :supervisor, build_stubbed(:supervisor, casa_org: build_stubbed(:casa_org)) + assign :all_volunteers_ever_assigned, [volunteer] + assign :available_volunteers, [] + + render template: "supervisors/edit" + + expect(rendered).not_to have_field("supervisor_email") + expect(rendered).not_to have_field("supervisor_display_name") + expect(rendered).not_to have_field("supervisor_phone_number") + end + it "shows for an admin editing a supervisor" do enable_pundit(view, supervisor) allow(view).to receive(:current_user).and_return(admin) diff --git a/spec/views/volunteers/edit.html.erb_spec.rb b/spec/views/volunteers/edit.html.erb_spec.rb index dd257ee7d6..916463e35c 100644 --- a/spec/views/volunteers/edit.html.erb_spec.rb +++ b/spec/views/volunteers/edit.html.erb_spec.rb @@ -16,6 +16,19 @@ expect(rendered).to_not have_field("volunteer_email", readonly: true) end + it "allows an administrator to edit a volunteers phone number" do + administrator = build_stubbed :casa_admin + enable_pundit(view, administrator) + allow(view).to receive(:current_user).and_return(administrator) + + assign :volunteer, volunteer + assign :supervisors, [] + + render template: "volunteers/edit" + + expect(rendered).to_not have_field("volunteer_email", readonly: true) + end + it "allows a supervisor to edit a volunteers email address" do supervisor = build_stubbed :supervisor enable_pundit(view, supervisor)