Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
d3176fa
add rspec tests for model validations
xihai01 Apr 3, 2022
e22169f
modify rspec model validation test to check for 12 digit phone number…
xihai01 Apr 3, 2022
a66f98e
model phone number field for users table in db
xihai01 Apr 3, 2022
6867630
refactor volunteer_controller specs
xihai01 Apr 5, 2022
f68c099
create request rspec test for volunteer update as admin
xihai01 Apr 7, 2022
56cf925
create request rspec test for supervisor update as admin
xihai01 Apr 7, 2022
95dbd57
create request rspec test for admin update as admin
xihai01 Apr 7, 2022
6bcbcd2
add integration tests for form fields with phone number
xihai01 Apr 8, 2022
2b22a20
add phone number input to views
xihai01 Apr 9, 2022
c3f9871
refactor user_validator to spit out clearer errors for phone number
xihai01 Apr 9, 2022
69ad254
add locales for en.common.phone_number
xihai01 Apr 9, 2022
d193df4
update error message for invalid phone number
xihai01 Apr 9, 2022
e964f01
fix tests
xihai01 Apr 11, 2022
68468bb
include phone number as a param for casa admin controller
xihai01 Apr 11, 2022
e7110cb
create system spec for users to check for invalid phone number form e…
xihai01 Apr 11, 2022
9bd38ae
lint files
xihai01 Apr 14, 2022
b372f86
remove duplicate test and used fake phone numbers for tests
xihai01 Apr 15, 2022
2692b68
refactor system specs to use shared_examples
xihai01 Apr 15, 2022
cc08751
refactor system spec for users to use shared_examples
xihai01 Apr 15, 2022
170d10e
create partial for the user form inputs to avoid duplication in views
xihai01 Apr 15, 2022
d375fb2
create a policy for previous partial
xihai01 Apr 15, 2022
7e6c6da
lint files
xihai01 Apr 15, 2022
3e795c3
delete duplicate migration for phone number column
xihai01 Apr 15, 2022
47c7789
Revert "delete duplicate migration for phone number column"
xihai01 Apr 15, 2022
b1218af
delete duplicate migration
xihai01 Apr 15, 2022
d434741
Merge branch 'main' into f/1785
xihai01 Apr 15, 2022
174d360
Merge pull request #1 from xihai01/f/1785
xihai01 Apr 15, 2022
77b7030
feature 1785 ready to PR
xihai01 Apr 15, 2022
d10a2a4
fix import spec with new error msgs
xihai01 Apr 15, 2022
870c8f8
fix policy permissions and correct some people's names
xihai01 Apr 17, 2022
a6a9245
remove admin has to be from same org for policy permissions
xihai01 Apr 17, 2022
4a0aeef
update db schema
xihai01 Apr 17, 2022
b4709d9
create view spec for supervisors/edit to check for editable/disabled …
xihai01 Apr 17, 2022
1d6b675
edit supervisors/edit view with permissions for phone number field
xihai01 Apr 18, 2022
dec5145
Merge pull request #2 from xihai01/f/1786
xihai01 Apr 18, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/controllers/casa_admins_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 2 additions & 2 deletions app/controllers/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion app/models/casa_admin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
#
Expand Down
4 changes: 2 additions & 2 deletions app/models/casa_case_contact_type.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
#
Expand Down
4 changes: 2 additions & 2 deletions app/models/case_assignment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
#
Expand Down
4 changes: 2 additions & 2 deletions app/models/case_contact.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
#
Expand Down
4 changes: 2 additions & 2 deletions app/models/case_contact_contact_type.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
#
Expand Down
2 changes: 1 addition & 1 deletion app/models/contact_type.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
#
Expand Down
2 changes: 1 addition & 1 deletion app/models/contact_type_group.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
#
Expand Down
2 changes: 1 addition & 1 deletion app/models/court_date.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
#
Expand Down
2 changes: 1 addition & 1 deletion app/models/emancipation_option.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
#
Expand Down
2 changes: 1 addition & 1 deletion app/models/hearing_type.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
#
Expand Down
2 changes: 1 addition & 1 deletion app/models/judge.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
#
Expand Down
2 changes: 1 addition & 1 deletion app/models/supervisor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
#
Expand Down
4 changes: 2 additions & 2 deletions app/models/supervisor_volunteer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
#
Expand Down
4 changes: 3 additions & 1 deletion app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ class User < ApplicationRecord

validates :email, presence: true
validates :display_name, presence: true

validates_with UserValidator
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we're using the fancy validator, should we put all the existing validators in it also and remove them here?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would be a great issue to create for

validate :at_least_one_communication_preference_selected

belongs_to :casa_org
Expand Down Expand Up @@ -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
#
Expand Down
2 changes: 1 addition & 1 deletion app/models/volunteer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
#
Expand Down
8 changes: 8 additions & 0 deletions app/policies/user_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

permissions issue- must make sure that the volunteer belongs to the correct casa org

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

end
is_admin?
end

def edit_name?(viewed_user)
is_admin? || viewed_user == user
end
Expand Down
16 changes: 10 additions & 6 deletions app/validators/user_validator.rb
Original file line number Diff line number Diff line change
@@ -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

Expand Down
1 change: 1 addition & 0 deletions app/values/user_parameters.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ def initialize(params, key = :user)
:email,
:casa_org_id,
:display_name,
:phone_number,
:password,
:active,
:type
Expand Down
10 changes: 1 addition & 9 deletions app/views/casa_admins/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,7 @@
<%= form_with(model: casa_admin, local: true) do |form| %>
<%= render "/shared/error_messages", resource: casa_admin %>

<div class="field form-group">
<%= form.label :email, t("common.email") %>
<%= form.text_field :email, class: "form-control" %>
</div>

<div class="field form-group">
<%= form.label :display_name, t("common.display_name") %>
<%= form.text_field :display_name, class: "form-control" %>
</div>
<%= render "/shared/edit_form", resource: casa_admin, f: form %>

<%= render "/shared/invite_login", resource: casa_admin %>

Expand Down
26 changes: 26 additions & 0 deletions app/views/shared/_edit_form.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<div class="field form-group">
<%= f.label :email, t("common.email") %>
<% if policy(resource).update_user_setting? %>
<%= f.text_field :email, class: "form-control" %>
<% else %>
<input class="form-control" type="text" placeholder="<%= resource.email %>" autocomplete="off" readonly>
<% end %>
</div>

<div class="field form-group">
<%= f.label :display_name, t("common.display_name") %>
<% if policy(resource).update_user_setting? %>
<%= f.text_field :display_name, class: "form-control" %>
<% else %>
<input class="form-control" type="text" placeholder="<%= resource.display_name %>" autocomplete="off" readonly>
<% end %>
</div>

<div class="field form-group">
<%= f.label :phone_number, t("common.phone_number") %>
<% if policy(resource).update_user_setting? %>
<%= f.text_field :phone_number, class: "form-control" %>
<% else %>
<input class="form-control" type="text" placeholder="<%= resource.phone_number %>" autocomplete="off" readonly>
<% end %>
</div>
18 changes: 1 addition & 17 deletions app/views/supervisors/edit.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,7 @@
<%= form_for @supervisor, as: :supervisor, url: supervisor_path(@supervisor) do |form| %>
<%= render "/shared/error_messages", resource: @supervisor %>

<div class="field form-group">
<%= form.label :email, t("common.email") %>
<% if policy(@supervisor).update_supervisor_email? %>
<%= form.text_field :email, class: "form-control" %>
<% else %>
<input class="form-control" type="text" placeholder="<%= @supervisor.email %>" autocomplete="off" readonly>
<% end %>
</div>

<div class="field form-group">
<%= form.label :display_name, t("common.display_name") %>
<% if policy(@supervisor).update_supervisor_name? %>
<%= form.text_field :display_name, class: "form-control" %>
<% else %>
<input class="form-control" type="text" placeholder="<%= @supervisor.display_name %>" autocomplete="off" readonly>
<% end %>
</div>
<%= render "/shared/edit_form", resource: @supervisor, f: form %>

<p>
<%= render "/shared/invite_login", resource: @supervisor %>
Expand Down
5 changes: 5 additions & 0 deletions app/views/users/edit.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@
<%= form.text_field :display_name, class: "form-control" %>
</div>

<div class="field form-group">
<%= form.label :phone_number, t("common.phone_number") %>
<%= form.text_field :phone_number, class: "form-control" %>
</div>

<%= form.hidden_field :casa_org_id, value: current_user.casa_org_id %>

<%= render "/shared/invite_login", resource: current_user %>
Expand Down
14 changes: 1 addition & 13 deletions app/views/volunteers/edit.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,7 @@
<%= form_for @volunteer, url: volunteer_path(@volunteer) do |form| %>
<%= render "/shared/error_messages", resource: @volunteer %>

<div class="field form-group">
<%= form.label :email, t("common.email") %>
<% if policy(:volunteer).update_volunteer_email? %>
<%= form.text_field :email, class: "form-control" %>
<% else %>
<input class="form-control" type="text" placeholder="<%= @volunteer.email %>" autocomplete="off" readonly>
<% end %>
</div>

<div class="field form-group">
<%= form.label :display_name, t("common.display_name") %>
<%= form.text_field :display_name, class: "form-control" %>
</div>
<%= render "/shared/edit_form", resource: @volunteer, f: form %>

<%= render "/shared/invite_login", resource: @volunteer %>

Expand Down
1 change: 1 addition & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ en:
email: Email
name: Name
display_name: Display name
phone_number: Phone number
address: Address
date:
formats:
Expand Down
1 change: 0 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Loading