From 9fd939cae128e26e047599b4c947d74599c2ad08 Mon Sep 17 00:00:00 2001 From: yuenmichelle1 Date: Fri, 27 Oct 2023 23:48:43 -0500 Subject: [PATCH 1/4] allow setting and updating stats visibility also adding error if stats visibility is invalid to bypass Rails enum attribute validation that returns ArgumentError --- .../api/v1/user_groups_controller.rb | 4 +- app/models/user_group.rb | 17 +++++- .../api/v1/user_groups_controller_spec.rb | 58 +++++++++++++++++++ spec/models/user_group_spec.rb | 26 +++++++++ 4 files changed, 102 insertions(+), 3 deletions(-) diff --git a/app/controllers/api/v1/user_groups_controller.rb b/app/controllers/api/v1/user_groups_controller.rb index 08a3dc48e..328105da0 100644 --- a/app/controllers/api/v1/user_groups_controller.rb +++ b/app/controllers/api/v1/user_groups_controller.rb @@ -9,8 +9,8 @@ class Api::V1::UserGroupsController < Api::ApiController alias_method :user_group, :controlled_resource - allowed_params :create, :name, :display_name, links: [ users: [] ] - allowed_params :update, :name, :display_name + allowed_params :create, :name, :display_name, :stats_visibility, links: [ users: [] ] + allowed_params :update, :name, :stats_visibility, :display_name search_by do |name, query| query.search_name(name.join(" ")) diff --git a/app/models/user_group.rb b/app/models/user_group.rb index 9d51da007..dcdc2c914 100644 --- a/app/models/user_group.rb +++ b/app/models/user_group.rb @@ -31,13 +31,20 @@ class UserGroup < ApplicationRecord # # public_show_all: Anyone can view aggregate stats of the user group and can view individual stats of the user group. ## - enum stats_visibility: { + STATS_VISIBILITY_LEVELS = { private_agg_only: 0, private_show_agg_and_ind: 1, public_agg_only: 2, public_agg_show_ind_if_member: 3, public_show_all: 4 } + enum stats_visibility: STATS_VISIBILITY_LEVELS + + validate do + if @invalid_stats_visibility + errors.add(:stats_visibility, "Not stats_visibility type, please select from the list: #{STATS_VISIBILITY_LEVELS.keys}") + end + end validates :display_name, presence: true validates :name, presence: true, @@ -86,6 +93,14 @@ def verify_join_token(token_to_verify) join_token.present? && join_token == token_to_verify end + def stats_visibility=(value) + if !STATS_VISIBILITY_LEVELS.stringify_keys.keys.include?(value) && !STATS_VISIBILITY_LEVELS.values.include?(value) + @invalid_stats_visibility = true + else + super value + end + end + private def default_display_name diff --git a/spec/controllers/api/v1/user_groups_controller_spec.rb b/spec/controllers/api/v1/user_groups_controller_spec.rb index 3b22a6d43..954dfa394 100644 --- a/spec/controllers/api/v1/user_groups_controller_spec.rb +++ b/spec/controllers/api/v1/user_groups_controller_spec.rb @@ -69,6 +69,37 @@ end it_behaves_like 'is updatable' + + describe 'updating stats_visibility' do + it 'updates stats_visibility' do + default_request scopes: scopes, user_id: authorized_user.id + params = { + id: resource.id, + user_groups: { + display_name: 'A-Different-Name', + stats_visibility: 'public_agg_only' + } + } + put :update, params: params + expect(response.status).to eq(200) + + group = UserGroup.find(resource.id) + expect(group.stats_visibility).to eq('public_agg_only') + end + + it 'does not update user_group if invalid stats_visibility' do + default_request scopes: scopes, user_id: authorized_user.id + params = { + id: resource.id, + user_groups: { + display_name: 'A-Different-Name', + stats_visibility: 'fake_stats_visibility' + } + } + put :update, params: params + expect(response.status).to eq(400) + end + end end describe '#show' do @@ -120,6 +151,33 @@ end end + describe 'setting stats_visibility' do + it 'sets the stats_visiblity when sending in stats_visiblity as string' do + default_request scopes: scopes, user_id: authorized_user.id + post :create, params: { user_groups: { name: 'GalaxyZoo', stats_visibility: 'public_agg_show_ind_if_member' } } + expect(response.status).to eq(201) + + group = UserGroup.find(created_instance_id('user_groups')) + expect(group.stats_visibility).to eq('public_agg_show_ind_if_member') + end + + it 'sets the stats_visibility when sending related integer corresponding to visibility level' do + default_request scopes: scopes, user_id: authorized_user.id + post :create, params: { user_groups: { name: 'GalaxyZoo', stats_visibility: 3 } } + expect(response.status).to eq(201) + + # see app/models/user_group.rb L22-L40 for explanations of stats_visibliity levels + group = UserGroup.find(created_instance_id('user_groups')) + expect(group.stats_visibility).to eq('public_agg_show_ind_if_member') + end + + it 'does not create group if stats_visibility is invalid' do + default_request scopes: scopes, user_id: authorized_user.id + post :create, params: { user_groups: { name: 'GalaxyZoo', stats_visibility: 7 } } + expect(response.status).to eq(400) + end + end + describe 'when only a name is provided' do it 'sets the display name' do default_request scopes: scopes, user_id: authorized_user.id diff --git a/spec/models/user_group_spec.rb b/spec/models/user_group_spec.rb index 350cbbad4..fcf7b3ac2 100644 --- a/spec/models/user_group_spec.rb +++ b/spec/models/user_group_spec.rb @@ -71,6 +71,32 @@ end end + describe '#stats_visibility' do + it 'validates that it is one of the STATS_VISIBILITY levels' do + ug = build(:user_group, name: "abc") + ug.stats_visibility = 'public_agg_only' + expect(ug).to be_valid + end + + it 'allows stats_visibility to be integer corresponding to STATS_VISIBILITY level' do + ug = build(:user_group, name: "abc") + ug.stats_visibility = 4 + expect(ug).to be_valid + end + + it 'does not allow stats_visibility to be outside STATS_VISIBILITY levels' do + ug = build(:user_group, name: "abc") + ug.stats_visibility = 'fake_stats_level' + expect(ug).to_not be_valid + end + + it 'does not allow stats_visibility to be outside STATS_VISIBILITY range' do + ug = build(:user_group, name: "abc") + ug.stats_visibility = 9 + expect(ug).to_not be_valid + end + end + describe "#users" do let(:user_group) { create(:user_group_with_users) } From e86811a6283e5376cdb003da617a67701595817d Mon Sep 17 00:00:00 2001 From: yuenmichelle1 Date: Sat, 28 Oct 2023 00:10:23 -0500 Subject: [PATCH 2/4] hounded " to and remove extra spaces, and .freeze constants --- app/controllers/api/v1/user_groups_controller.rb | 2 +- app/models/user_group.rb | 4 ++-- spec/models/user_group_spec.rb | 12 ++++++------ 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/app/controllers/api/v1/user_groups_controller.rb b/app/controllers/api/v1/user_groups_controller.rb index 328105da0..1234d486b 100644 --- a/app/controllers/api/v1/user_groups_controller.rb +++ b/app/controllers/api/v1/user_groups_controller.rb @@ -9,7 +9,7 @@ class Api::V1::UserGroupsController < Api::ApiController alias_method :user_group, :controlled_resource - allowed_params :create, :name, :display_name, :stats_visibility, links: [ users: [] ] + allowed_params :create, :name, :display_name, :stats_visibility, links: [users: []] allowed_params :update, :name, :stats_visibility, :display_name search_by do |name, query| diff --git a/app/models/user_group.rb b/app/models/user_group.rb index dcdc2c914..d274fe71a 100644 --- a/app/models/user_group.rb +++ b/app/models/user_group.rb @@ -37,7 +37,7 @@ class UserGroup < ApplicationRecord public_agg_only: 2, public_agg_show_ind_if_member: 3, public_show_all: 4 - } + }.freeze enum stats_visibility: STATS_VISIBILITY_LEVELS validate do @@ -94,7 +94,7 @@ def verify_join_token(token_to_verify) end def stats_visibility=(value) - if !STATS_VISIBILITY_LEVELS.stringify_keys.keys.include?(value) && !STATS_VISIBILITY_LEVELS.values.include?(value) + if STATS_VISIBILITY_LEVELS.stringify_keys.keys.exclude?(value) && STATS_VISIBILITY_LEVELS.values.exclude?(value) @invalid_stats_visibility = true else super value diff --git a/spec/models/user_group_spec.rb b/spec/models/user_group_spec.rb index fcf7b3ac2..6b8b847d9 100644 --- a/spec/models/user_group_spec.rb +++ b/spec/models/user_group_spec.rb @@ -73,27 +73,27 @@ describe '#stats_visibility' do it 'validates that it is one of the STATS_VISIBILITY levels' do - ug = build(:user_group, name: "abc") + ug = build(:user_group, name: 'abc') ug.stats_visibility = 'public_agg_only' expect(ug).to be_valid end it 'allows stats_visibility to be integer corresponding to STATS_VISIBILITY level' do - ug = build(:user_group, name: "abc") + ug = build(:user_group, name: 'abc') ug.stats_visibility = 4 expect(ug).to be_valid end it 'does not allow stats_visibility to be outside STATS_VISIBILITY levels' do - ug = build(:user_group, name: "abc") + ug = build(:user_group, name: 'abc') ug.stats_visibility = 'fake_stats_level' - expect(ug).to_not be_valid + expect(ug).not_to be_valid end it 'does not allow stats_visibility to be outside STATS_VISIBILITY range' do - ug = build(:user_group, name: "abc") + ug = build(:user_group, name: 'abc') ug.stats_visibility = 9 - expect(ug).to_not be_valid + expect(ug).not_to be_valid end end From 4183b408f255c88e828f0b2675e70b0572291dfa Mon Sep 17 00:00:00 2001 From: yuenmichelle1 Date: Sat, 28 Oct 2023 00:18:49 -0500 Subject: [PATCH 3/4] missed hound review. update validate stats visibility to single line --- app/models/user_group.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/models/user_group.rb b/app/models/user_group.rb index d274fe71a..a9a81882b 100644 --- a/app/models/user_group.rb +++ b/app/models/user_group.rb @@ -41,9 +41,7 @@ class UserGroup < ApplicationRecord enum stats_visibility: STATS_VISIBILITY_LEVELS validate do - if @invalid_stats_visibility - errors.add(:stats_visibility, "Not stats_visibility type, please select from the list: #{STATS_VISIBILITY_LEVELS.keys}") - end + errors.add(:stats_visibility, "Not valid stats_visibility type, please select from the list: #{STATS_VISIBILITY_LEVELS.keys}") if @invalid_stats_visibility end validates :display_name, presence: true From d8c4f963458a097d9dddcd35f8162a7692b0fdd4 Mon Sep 17 00:00:00 2001 From: yuenmichelle1 Date: Fri, 3 Nov 2023 13:09:37 -0500 Subject: [PATCH 4/4] update update specs to ensure group_admins/admin users can update but not group_members --- .../api/v1/user_groups_controller_spec.rb | 103 +++++++++++------- 1 file changed, 64 insertions(+), 39 deletions(-) diff --git a/spec/controllers/api/v1/user_groups_controller_spec.rb b/spec/controllers/api/v1/user_groups_controller_spec.rb index 954dfa394..a1c22ce26 100644 --- a/spec/controllers/api/v1/user_groups_controller_spec.rb +++ b/spec/controllers/api/v1/user_groups_controller_spec.rb @@ -71,33 +71,56 @@ it_behaves_like 'is updatable' describe 'updating stats_visibility' do - it 'updates stats_visibility' do - default_request scopes: scopes, user_id: authorized_user.id - params = { + let(:params) { + { id: resource.id, user_groups: { display_name: 'A-Different-Name', stats_visibility: 'public_agg_only' } } - put :update, params: params - expect(response.status).to eq(200) + } - group = UserGroup.find(resource.id) - expect(group.stats_visibility).to eq('public_agg_only') - end + describe 'as group_admin' do + it 'updates stats_visibility' do + default_request scopes: scopes, user_id: authorized_user.id + put :update, params: params + expect(response.status).to eq(200) - it 'does not update user_group if invalid stats_visibility' do - default_request scopes: scopes, user_id: authorized_user.id - params = { - id: resource.id, - user_groups: { + group = UserGroup.find(resource.id) + expect(group.stats_visibility).to eq('public_agg_only') + end + + it 'does not update user_group if invalid stats_visibility' do + default_request scopes: scopes, user_id: authorized_user.id + user_groups = { display_name: 'A-Different-Name', stats_visibility: 'fake_stats_visibility' } - } - put :update, params: params - expect(response.status).to eq(400) + params[:user_groups] = user_groups + put :update, params: params + expect(response.status).to eq(400) + end + end + + describe 'as admin' do + it 'updates user_group_stats_visibility' do + admin_user = create(:user, admin: true) + default_request scopes: scopes, user_id: admin_user.id + params[:admin] = true + put :update, params: params + expect(response.status).to eq(200) + end + end + + describe 'as group_member' do + it 'does not update user_group stats_visibility' do + group_member_user = create(:user) + create(:membership, user: group_member_user, user_group: resource, roles: ['group_member']) + default_request scopes: scopes, user_id: group_member_user.id + put :update, params: params + expect(response.status).to eq(404) + end end end end @@ -152,29 +175,31 @@ end describe 'setting stats_visibility' do - it 'sets the stats_visiblity when sending in stats_visiblity as string' do - default_request scopes: scopes, user_id: authorized_user.id - post :create, params: { user_groups: { name: 'GalaxyZoo', stats_visibility: 'public_agg_show_ind_if_member' } } - expect(response.status).to eq(201) - - group = UserGroup.find(created_instance_id('user_groups')) - expect(group.stats_visibility).to eq('public_agg_show_ind_if_member') - end - - it 'sets the stats_visibility when sending related integer corresponding to visibility level' do - default_request scopes: scopes, user_id: authorized_user.id - post :create, params: { user_groups: { name: 'GalaxyZoo', stats_visibility: 3 } } - expect(response.status).to eq(201) - - # see app/models/user_group.rb L22-L40 for explanations of stats_visibliity levels - group = UserGroup.find(created_instance_id('user_groups')) - expect(group.stats_visibility).to eq('public_agg_show_ind_if_member') - end - - it 'does not create group if stats_visibility is invalid' do - default_request scopes: scopes, user_id: authorized_user.id - post :create, params: { user_groups: { name: 'GalaxyZoo', stats_visibility: 7 } } - expect(response.status).to eq(400) + describe 'as group_admin' do + it 'sets the stats_visiblity when sending in stats_visiblity as string' do + default_request scopes: scopes, user_id: authorized_user.id + post :create, params: { user_groups: { name: 'GalaxyZoo', stats_visibility: 'public_agg_show_ind_if_member' } } + expect(response.status).to eq(201) + group = UserGroup.find(created_instance_id('user_groups')) + + expect(group.stats_visibility).to eq('public_agg_show_ind_if_member') + end + + it 'sets the stats_visibility when sending related integer corresponding to visibility level' do + default_request scopes: scopes, user_id: authorized_user.id + post :create, params: { user_groups: { name: 'GalaxyZoo', stats_visibility: 3 } } + expect(response.status).to eq(201) + + # see app/models/user_group.rb L22-L40 for explanations of stats_visibliity levels + group = UserGroup.find(created_instance_id('user_groups')) + expect(group.stats_visibility).to eq('public_agg_show_ind_if_member') + end + + it 'does not create group if stats_visibility is invalid' do + default_request scopes: scopes, user_id: authorized_user.id + post :create, params: { user_groups: { name: 'GalaxyZoo', stats_visibility: 7 } } + expect(response.status).to eq(400) + end end end