Skip to content

Commit

Permalink
User changes - contactable opt-in
Browse files Browse the repository at this point in the history
* adds contactable consent field to user model
updates related functionality (filter, strong params) for updated user model
adds test cases to users model and users requests

updates error handling in lib/gems/baw-app/lib/types.rb

autoformat **only** changes:
spec/factories/user_factory.rb
spec/controllers/user_accounts_controller_spec.rb

* addresses PR comments
changes values of consent enum

Fixes #692
  • Loading branch information
andrew-1234 authored and atruskie committed Jan 16, 2025
1 parent bdf451e commit a8084b5
Show file tree
Hide file tree
Showing 10 changed files with 269 additions and 131 deletions.
44 changes: 22 additions & 22 deletions app/controllers/user_accounts_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,8 @@ def projects
do_authorize_instance

@user_projects = Access::ByPermission.projects(@user).includes(:creator).references(:creator)
.order('projects.name ASC')
.page(paging_params[:page].blank? ? 1 : paging_params[:page])
.order('projects.name ASC')
.page((paging_params[:page].presence || 1))
respond_to do |format|
format.html
end
Expand All @@ -118,8 +118,8 @@ def sites
do_authorize_instance

@user_sites = Access::ByPermission.sites(@user).includes(:creator, :projects).references(:creator, :project)
.order('sites.name ASC')
.page(paging_params[:page].blank? ? 1 : paging_params[:page])
.order('sites.name ASC')
.page((paging_params[:page].presence || 1))

respond_to do |format|
format.html
Expand All @@ -132,8 +132,8 @@ def bookmarks
do_authorize_instance

@user_bookmarks = Access::ByUserModified.bookmarks(@user)
.order('bookmarks.updated_at DESC')
.page(paging_params[:page].blank? ? 1 : paging_params[:page])
.order('bookmarks.updated_at DESC')
.page((paging_params[:page].presence || 1))
respond_to do |format|
format.html
end
Expand All @@ -145,8 +145,8 @@ def audio_event_comments
do_authorize_instance

@user_audio_event_comments = Access::ByUserModified.audio_event_comments(@user)
.order('audio_event_comments.updated_at DESC')
.page(paging_params[:page].blank? ? 1 : paging_params[:page])
.order('audio_event_comments.updated_at DESC')
.page((paging_params[:page].presence || 1))
respond_to do |format|
format.html
end
Expand All @@ -160,8 +160,8 @@ def audio_events
@user_annotations = Access::ByUserModified.audio_events(@user).includes(audio_recording: [:site]).references(
:audio_recordings, :sites
)
.order('audio_events.updated_at DESC')
.page(paging_params[:page].blank? ? 1 : paging_params[:page])
.order('audio_events.updated_at DESC')
.page((paging_params[:page].presence || 1))
respond_to do |format|
format.html
end
Expand All @@ -173,11 +173,11 @@ def saved_searches
do_authorize_instance

@user_saved_searches = Access::ByUserModified.saved_searches(@user)
.order('saved_searches.created_at DESC')
.paginate(
page: paging_params[:page].blank? ? 1 : paging_params[:page],
per_page: 30
)
.order('saved_searches.created_at DESC')
.paginate(
page: paging_params[:page].presence || 1,
per_page: 30
)
respond_to do |format|
format.html
end
Expand All @@ -189,11 +189,11 @@ def analysis_jobs
do_authorize_instance

@user_analysis_jobs = Access::ByUserModified.analysis_jobs(@user)
.order('analysis_jobs.updated_at DESC')
.paginate(
page: paging_params[:page].blank? ? 1 : paging_params[:page],
per_page: 30
)
.order('analysis_jobs.updated_at DESC')
.paginate(
page: paging_params[:page].presence || 1,
per_page: 30
)
respond_to do |format|
format.html
end
Expand Down Expand Up @@ -223,15 +223,15 @@ def user_params
params.require(:user).permit(
:user_name, :email, :password, :password_confirmation, :remember_me,
:roles, :roles_mask, :preferences,
:image, :login
:image, :login, :contactable
)
end

def user_update_params
params.require(:user).permit(
:id, :user_name, :email, :tzinfo_tz,
:password, :password_confirmation,
:roles_mask, :image
:roles_mask, :image, :contactable
)
end

Expand Down
4 changes: 2 additions & 2 deletions app/models/ability.rb
Original file line number Diff line number Diff line change
Expand Up @@ -667,7 +667,7 @@ def to_saved_search(user, is_guest)
can [:index, :filter], SavedSearch
end

def to_provenance(_user, is_guest)
def to_provenance(_user, _is_guest)
# other actions are admin only

# available to any user, including guest
Expand Down Expand Up @@ -722,7 +722,7 @@ def to_user(user, is_guest)
# normal users edit their profile using devise/registrations#edit

# users can only view their own:
can [:projects, :sites, :bookmarks, :audio_events, :audio_event_comments], User, id: user.id
can [:projects, :sites, :bookmarks, :audio_events, :audio_event_comments, :update], User, id: user.id

# users get their own account and preferences from these actions
can [:my_account, :modify_preferences], User, id: user.id
Expand Down
104 changes: 68 additions & 36 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,38 +4,39 @@
#
# Table name: users
#
# id :integer not null, primary key
# authentication_token :string
# confirmation_sent_at :datetime
# confirmation_token :string
# confirmed_at :datetime
# current_sign_in_at :datetime
# current_sign_in_ip :string
# email :string not null
# encrypted_password :string not null
# failed_attempts :integer default(0)
# image_content_type :string
# image_file_name :string
# image_file_size :bigint
# image_updated_at :datetime
# invitation_token :string
# last_seen_at :datetime
# last_sign_in_at :datetime
# last_sign_in_ip :string
# locked_at :datetime
# preferences :text
# rails_tz :string(255)
# remember_created_at :datetime
# reset_password_sent_at :datetime
# reset_password_token :string
# roles_mask :integer
# sign_in_count :integer default(0)
# tzinfo_tz :string(255)
# unconfirmed_email :string
# unlock_token :string
# user_name :string not null
# created_at :datetime
# updated_at :datetime
# id :integer not null, primary key
# authentication_token :string
# confirmation_sent_at :datetime
# confirmation_token :string
# confirmed_at :datetime
# contactable(Is the user contactable for email communications) :enum default("unasked"), not null
# current_sign_in_at :datetime
# current_sign_in_ip :string
# email :string not null
# encrypted_password :string not null
# failed_attempts :integer default(0)
# image_content_type :string
# image_file_name :string
# image_file_size :bigint
# image_updated_at :datetime
# invitation_token :string
# last_seen_at :datetime
# last_sign_in_at :datetime
# last_sign_in_ip :string
# locked_at :datetime
# preferences :text
# rails_tz :string(255)
# remember_created_at :datetime
# reset_password_sent_at :datetime
# reset_password_token :string
# roles_mask :integer
# sign_in_count :integer default(0)
# tzinfo_tz :string(255)
# unconfirmed_email :string
# unlock_token :string
# user_name :string not null
# created_at :datetime
# updated_at :datetime
#
# Indexes
#
Expand All @@ -61,6 +62,36 @@ class User < ApplicationRecord
:recoverable, :rememberable, :trackable, :validatable,
:confirmable, :lockable, :timeoutable

# Defines a reusable mapping (CONSENT_ENUM) of consent constants
CONSENT_UNASKED = 'unasked'
CONSENT_YES = 'yes'
CONSENT_NO = 'no'

CONSENT_ENUM = {
CONSENT_UNASKED => CONSENT_UNASKED,
CONSENT_YES => CONSENT_YES,
CONSENT_NO => CONSENT_NO
}.freeze

# @!method contactable_unasked?
# @return [Boolean] true if the user has not been asked for consent for contact
# @!method contactable_unasked!
# @return [void] sets contactable as unasked
# @!method contactable_yes?
# @return [Boolean] true if the user has consented to be contacted
# @!method contactable_yes!
# @return [void] sets the user as contactable
# @!method contactable_no?
# @return [Boolean] true if the user has not consented to be contacted
# @!method contactable_no!
# @return [void] sets the user as not contactable
enum :contactable, CONSENT_ENUM, prefix: :contactable, validate: true

# @return [Boolean] true if the user has consented to be contacted
def contactable?
contactable == CONSENT_YES
end

# http://www.phase2technology.com/blog/authentication-permissions-and-roles-in-rails-with-devise-cancan-and-role-model/
include RoleModel

Expand Down Expand Up @@ -210,8 +241,8 @@ def login
def excluded_login
reserved_user_names = ['admin', 'harvester', 'analysis_runner', 'root', 'superuser', 'administrator', 'admins',
'administrators']
errors.add(:login, 'is reserved') if reserved_user_names.include?(login.downcase)
errors.add(:user_name, 'is reserved') if reserved_user_names.include?(user_name.downcase)
errors.add(:login, 'is reserved') if reserved_user_names.include?(login&.downcase)
errors.add(:user_name, 'is reserved') if reserved_user_names.include?(user_name&.downcase)
end

# format, uniqueness, and presence are validated by devise
Expand Down Expand Up @@ -345,10 +376,11 @@ def admin?
end

# Define filter api settings
# @return [Hash] filter settings
def self.filter_settings
{
valid_fields: [:id, :user_name, :roles_mask, :last_seen_at, :created_at, :updated_at],
render_fields: [:id, :user_name, :roles_mask],
valid_fields: [:id, :user_name, :roles_mask, :last_seen_at, :created_at, :updated_at, :contactable],
render_fields: [:id, :user_name, :roles_mask, :contactable],
text_fields: [:user_name],
custom_fields: lambda { |item, user|
# 'item' is the user being processed, 'user' is the currently logged in user
Expand Down
9 changes: 9 additions & 0 deletions db/migrate/20250113012304_add_contactable_to_users.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# frozen_string_literal: true

class AddContactableToUsers < ActiveRecord::Migration[7.2]
def change
create_enum :consent, ['unasked', 'yes', 'no']
add_column :users, :contactable, :enum, enum_type: :consent, default: 'unasked', null: false,
comment: 'Is the user contactable for email communications'
end
end
22 changes: 21 additions & 1 deletion db/structure.sql
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,17 @@ CREATE TYPE public.analysis_jobs_item_transition AS ENUM (
);


--
-- Name: consent; Type: TYPE; Schema: public; Owner: -
--

CREATE TYPE public.consent AS ENUM (
'unasked',
'yes',
'no'
);


--
-- Name: dirname(text); Type: FUNCTION; Schema: public; Owner: -
--
Expand Down Expand Up @@ -2004,10 +2015,18 @@ CREATE TABLE public.users (
preferences text,
tzinfo_tz character varying(255),
rails_tz character varying(255),
last_seen_at timestamp without time zone
last_seen_at timestamp without time zone,
contactable public.consent DEFAULT 'unasked'::public.consent NOT NULL
);


--
-- Name: COLUMN users.contactable; Type: COMMENT; Schema: public; Owner: -
--

COMMENT ON COLUMN public.users.contactable IS 'Is the user contactable for email communications';


--
-- Name: users_id_seq; Type: SEQUENCE; Schema: public; Owner: -
--
Expand Down Expand Up @@ -4216,6 +4235,7 @@ ALTER TABLE ONLY public.tags
SET search_path TO "$user", public;

INSERT INTO "schema_migrations" (version) VALUES
('20250113012304'),
('20241106015941'),
('20241004055117'),
('20240828062256'),
Expand Down
9 changes: 8 additions & 1 deletion lib/gems/baw-app/lib/types.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,15 @@ module Types
begin
p.mkpath
rescue Errno::EACCES => e
Rails.logger.debug { "ERROR: Could not create directory #{p}, #{e}" }
# These errors happen before the logger is available, don't use rails logger
# rubocop:disable Rails/Output
puts "CreatedDirPathname ERROR: Could not create directory #{p}, #{e}"
# rubocop:enable Rails/Output
raise e if BawApp.dev_or_test?
rescue StandardError => e
# rubocop:disable Rails/Output
puts "CreatedDirPathname ERROR: #{e}"
# rubocop:enable Rails/Output
end
p
}
Expand Down
6 changes: 3 additions & 3 deletions spec/controllers/user_accounts_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
# make sure it can recover from bad input
describe 'proper timezones data' do
let(:user_bad_tz) {
user = FactoryBot.build(:user, tzinfo_tz: 'Australia - Sydney', rails_tz: 'Sydney')
user = build(:user, tzinfo_tz: 'Australia - Sydney', rails_tz: 'Sydney')
user.save!(validate: false)
user
}
Expand Down Expand Up @@ -55,7 +55,7 @@

describe 'bad timezones data' do
let(:user_bad_tz) {
user = FactoryBot.build(:user, tzinfo_tz: '[email protected]', rails_tz: '[email protected]')
user = build(:user, tzinfo_tz: '[email protected]', rails_tz: '[email protected]')
user.save!(validate: false)
user
}
Expand All @@ -71,7 +71,7 @@
response = get :my_account, params: { format: :json }
body = JSON.parse(response.body)

expect(body['timezone_information']).to be(nil)
expect(body['timezone_information']).to be_nil

user_good_tz = User.find(user_bad_tz.id)
new_values = [user_good_tz.tzinfo_tz, user_good_tz.rails_tz]
Expand Down
Loading

0 comments on commit a8084b5

Please sign in to comment.