From 761f2a8ede09a7a361827d835e173bd118a624ca Mon Sep 17 00:00:00 2001 From: yuenmichelle1 Date: Thu, 10 Aug 2023 11:16:25 -0500 Subject: [PATCH 01/20] individual stats breakdown per group. add session time to user group projects breakdown. --- ...r_group_classification_count_controller.rb | 5 +-- app/queries/count_group_member_breakdown.rb | 30 ++++++++++++++++++ .../count_group_project_contributions.rb | 2 +- ...group_member_stats_breakdown_serializer.rb | 31 +++++++++++++++++++ 4 files changed, 65 insertions(+), 3 deletions(-) create mode 100644 app/queries/count_group_member_breakdown.rb create mode 100644 app/serializers/user_group_member_stats_breakdown_serializer.rb diff --git a/app/controllers/user_group_classification_count_controller.rb b/app/controllers/user_group_classification_count_controller.rb index 3521640..fa15afc 100644 --- a/app/controllers/user_group_classification_count_controller.rb +++ b/app/controllers/user_group_classification_count_controller.rb @@ -12,8 +12,9 @@ def query # authorize :queried_user_group_context, :show? skip_authorization if params[:individual_stats_breakdown] - # TODO: in a separate PR - # Plan is to query from DailyGroupClassificationCountAndTimePerUserPerProject + group_member_classification_counts = CountGroupMemberBreakdown.new.call(group_classification_count_params) + + render json: UserGroupMemberStatsBreakdownSerializer.new(group_member_classification_counts) else group_classification_counts = CountGroupClassifications.new(group_classification_count_params).call(group_classification_count_params) group_active_user_classification_counts = CountGroupActiveUserClassifications.new(group_classification_count_params).call(group_classification_count_params) diff --git a/app/queries/count_group_member_breakdown.rb b/app/queries/count_group_member_breakdown.rb new file mode 100644 index 0000000..c130a90 --- /dev/null +++ b/app/queries/count_group_member_breakdown.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +class CountGroupMemberBreakdown + include Filterable + attr_reader :counts + + def initialize + @counts = initial_scope(relation) + end + + def call(params={}) + scoped = @counts + scoped = filter_by_user_group_id(scoped, params[:id]) + filter_by_date_range(scoped, params[:start_date], params[:end_date]) + end + + private + + def initial_scope(relation) + relation.select(select_clause).group('user_id, project_id') + end + + def select_clause + 'user_id, project_id, SUM(classification_count)::integer AS count, SUM(total_session_time)::float AS session_time' + end + + def relation + UserGroupClassificationCounts::DailyGroupUserProjectClassificationCount + end +end diff --git a/app/queries/count_group_project_contributions.rb b/app/queries/count_group_project_contributions.rb index c5401c7..75844aa 100644 --- a/app/queries/count_group_project_contributions.rb +++ b/app/queries/count_group_project_contributions.rb @@ -21,7 +21,7 @@ def initial_scope(relation) end def select_clause - 'project_id, SUM(classification_count)::integer AS count' + 'project_id, SUM(classification_count)::integer AS count, SUM(total_session_time)::float AS session_time' end def relation diff --git a/app/serializers/user_group_member_stats_breakdown_serializer.rb b/app/serializers/user_group_member_stats_breakdown_serializer.rb new file mode 100644 index 0000000..299b840 --- /dev/null +++ b/app/serializers/user_group_member_stats_breakdown_serializer.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +class UserGroupMemberStatsBreakdownSerializer + attr_reader :group_member_classification_counts + + def initialize(counts_scope) + @group_member_classification_counts = counts_scope + end + + def as_json(_options) + { + group_member_stats_breakdown: counts_grouped_by_user + } + end + + private + + def counts_grouped_by_user + counts_grouped_by_member = group_member_classification_counts.group_by { |member_proj_contribution| member_proj_contribution[:user_id] }.transform_values do |member_counts_per_project| + total_per_member = { count: member_counts_per_project.sum(&:count) } + total_per_member[:session_time] = member_counts_per_project.sum(&:session_time) + total_per_member[:project_contributions] = individual_member_project_contributions(member_counts_per_project) + total_per_member + end + counts_grouped_by_member.map { |user_id, totals| { user_id: }.merge(totals) }.sort_by(&:count) + end + + def individual_member_project_contributions(member_counts_per_project) + member_counts_per_project.sort_by(&:count).reverse.map { |member_count| member_count.as_json.except('user_id') } + end +end From 8df82f15febbd6f893e505febd8e3e804e254bf1 Mon Sep 17 00:00:00 2001 From: yuenmichelle1 Date: Thu, 10 Aug 2023 11:41:47 -0500 Subject: [PATCH 02/20] update sort of group member stats breakdown --- .../user_group_member_stats_breakdown_serializer.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/serializers/user_group_member_stats_breakdown_serializer.rb b/app/serializers/user_group_member_stats_breakdown_serializer.rb index 299b840..0f1f296 100644 --- a/app/serializers/user_group_member_stats_breakdown_serializer.rb +++ b/app/serializers/user_group_member_stats_breakdown_serializer.rb @@ -9,7 +9,7 @@ def initialize(counts_scope) def as_json(_options) { - group_member_stats_breakdown: counts_grouped_by_user + group_member_stats_breakdown: counts_grouped_by_user.sort_by { |c| c[:count] }.reverse } end @@ -22,7 +22,7 @@ def counts_grouped_by_user total_per_member[:project_contributions] = individual_member_project_contributions(member_counts_per_project) total_per_member end - counts_grouped_by_member.map { |user_id, totals| { user_id: }.merge(totals) }.sort_by(&:count) + counts_grouped_by_member.map { |user_id, totals| { user_id: }.merge(totals) } end def individual_member_project_contributions(member_counts_per_project) From 08ab8a1ee81c8a985a4b3678c3e0ae844507738a Mon Sep 17 00:00:00 2001 From: yuenmichelle1 Date: Thu, 10 Aug 2023 12:35:17 -0500 Subject: [PATCH 03/20] Update user_group_member_stats_breakdown_serializer.rb --- app/serializers/user_group_member_stats_breakdown_serializer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/serializers/user_group_member_stats_breakdown_serializer.rb b/app/serializers/user_group_member_stats_breakdown_serializer.rb index 0f1f296..f8a556e 100644 --- a/app/serializers/user_group_member_stats_breakdown_serializer.rb +++ b/app/serializers/user_group_member_stats_breakdown_serializer.rb @@ -9,7 +9,7 @@ def initialize(counts_scope) def as_json(_options) { - group_member_stats_breakdown: counts_grouped_by_user.sort_by { |c| c[:count] }.reverse + group_member_stats_breakdown: counts_grouped_by_user.sort_by { |member_stat| member_stat[:count] }.reverse } end From 9cf117e3e57876911e879fe14f38f511e75e1669 Mon Sep 17 00:00:00 2001 From: yuenmichelle1 Date: Thu, 10 Aug 2023 14:24:11 -0500 Subject: [PATCH 04/20] add specs user group classification count controller for individual stats breakdown stats --- ...up_classification_count_controller_spec.rb | 49 ++++++++++++------- .../count_group_project_contributions_spec.rb | 2 +- 2 files changed, 31 insertions(+), 20 deletions(-) diff --git a/spec/controllers/user_group_classification_count_controller_spec.rb b/spec/controllers/user_group_classification_count_controller_spec.rb index 5e29ca4..8a0ad13 100644 --- a/spec/controllers/user_group_classification_count_controller_spec.rb +++ b/spec/controllers/user_group_classification_count_controller_spec.rb @@ -6,28 +6,39 @@ describe 'GET query' do let!(:classification_user_group) { create(:classification_user_group) } - it 'returns total_count, time_spent, active_users, and project_contributions of user group' do - get :query, params: { id: classification_user_group.user_group_id.to_s } - expect(response.status).to eq(200) - response_body = JSON.parse(response.body) - expect(response_body['total_count']).to eq(1) - expect(response_body['time_spent']).to eq(classification_user_group.session_time) - expect(response_body['active_users']).to eq(1) - expect(response_body['project_contributions'].length).to eq(1) - end + context 'individual_stats_breakdown is false/not a param' do + it 'returns total_count, time_spent, active_users, and project_contributions of user group' do + get :query, params: { id: classification_user_group.user_group_id.to_s } + expect(response.status).to eq(200) + response_body = JSON.parse(response.body) + expect(response_body['total_count']).to eq(1) + expect(response_body['time_spent']).to eq(classification_user_group.session_time) + expect(response_body['active_users']).to eq(1) + expect(response_body['project_contributions'].length).to eq(1) + end + + it 'does not compute project_contributions when params[:project_id] given' do + get :query, params: { id: classification_user_group.user_group_id.to_s, project_id: 2 } + expect(response.status).to eq(200) + response_body = JSON.parse(response.body) + expect(response_body).not_to have_key('project_contributions') + end - it 'does not compute project_contributions when params[:project_id] given' do - get :query, params: { id: classification_user_group.user_group_id.to_s, project_id: 2 } - expect(response.status).to eq(200) - response_body = JSON.parse(response.body) - expect(response_body).not_to have_key(:project_contributions) + it 'does not compute project_contributions when params[:workflow_id] given' do + get :query, params: { id: classification_user_group.user_group_id.to_s, workflow_id: 2 } + expect(response.status).to eq(200) + response_body = JSON.parse(response.body) + expect(response_body).not_to have_key('project_contributions') + end end - it 'does not compute project_contributions when params[:workflow_id] given' do - get :query, params: { id: classification_user_group.user_group_id.to_s, workflow_id: 2 } - expect(response.status).to eq(200) - response_body = JSON.parse(response.body) - expect(response_body).not_to have_key(:project_contributions) + context 'individual_stats_breakdown is true' do + it 'returns group_member_stats_breakdown' do + get :query, params: { id: classification_user_group.user_group_id.to_s, individual_stats_breakdown: true } + expect(response.status).to eq(200) + response_body = JSON.parse(response.body) + expect(response_body).to have_key('group_member_stats_breakdown') + end end context 'param validations' do diff --git a/spec/queries/count_group_project_contributions_spec.rb b/spec/queries/count_group_project_contributions_spec.rb index 8c205dd..ae493c0 100644 --- a/spec/queries/count_group_project_contributions_spec.rb +++ b/spec/queries/count_group_project_contributions_spec.rb @@ -14,7 +14,7 @@ describe 'select_clause' do it 'selects project_id and orders by count' do counts = group_classifications_query.call(params) - expected_select_query = 'SELECT project_id, SUM(classification_count)::integer AS count FROM "daily_group_classification_count_and_time_per_project" ' + expected_select_query = 'SELECT project_id, SUM(classification_count)::integer AS count, SUM(total_session_time)::float AS session_time FROM "daily_group_classification_count_and_time_per_project" ' expected_select_query += 'GROUP BY "daily_group_classification_count_and_time_per_project"."project_id" ORDER BY count DESC' expect(counts.to_sql).to eq(expected_select_query) end From 34eb2b13f16c5b601f19df365e0b5956c3a2b9da Mon Sep 17 00:00:00 2001 From: yuenmichelle1 Date: Thu, 10 Aug 2023 16:44:46 -0500 Subject: [PATCH 05/20] add specs for group member breakdown query stats --- .../count_group_member_breakdown_spec.rb | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 spec/queries/count_group_member_breakdown_spec.rb diff --git a/spec/queries/count_group_member_breakdown_spec.rb b/spec/queries/count_group_member_breakdown_spec.rb new file mode 100644 index 0000000..c1b1d52 --- /dev/null +++ b/spec/queries/count_group_member_breakdown_spec.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe CountGroupMemberBreakdown do + let(:params) { {} } + let(:group_member_breakdown_query) { described_class.new } + describe 'relation' do + it 'returns DailyGroupUserProjectClassificationCount' do + expect(group_member_breakdown_query.counts.model).to be UserGroupClassificationCounts::DailyGroupUserProjectClassificationCount + end + end + + describe 'select_clause' do + it 'selects user_id, project_id, sum of counts and time' do + counts = group_member_breakdown_query.call(params) + expected_select_query = 'SELECT user_id, project_id, SUM(classification_count)::integer AS count, SUM(total_session_time)::float AS session_time ' + expected_select_query += 'FROM "daily_group_classification_count_and_time_per_user_per_project" ' + expected_select_query += 'GROUP BY user_id, project_id' + expect(counts.to_sql).to eq(expected_select_query) + end + end + + describe '#call' do + let!(:classification_user_group) { create(:classification_user_group) } + let!(:diff_workflow_event) { create(:cug_with_diff_workflow) } + let!(:diff_project_event) { create(:cug_with_diff_project) } + let!(:diff_time_event) { create(:cug_created_yesterday) } + let!(:diff_user_classification) { create(:cug_with_diff_user) } + let!(:diff_user_group_classification) { create(:cug_with_diff_user_group) } + + before(:each) do + params[:id] = classification_user_group.user_group_id.to_s + end + + it_behaves_like 'is filterable by date range' do + let(:counts_query) { described_class.new } + end + + it 'filters by given user_group_id' do + counts = group_member_breakdown_query.call(params) + expect(counts.to_sql).to include(".\"user_group_id\" = #{classification_user_group.user_id}") + end + + it 'returns classification counts of given user group grouped by user and project' do + counts = group_member_breakdown_query.call(params) + # because default is grouped by project_id and user_id, we expect results to look something like: + # [ + # , + # , + # + # ] + expect(counts.length).to eq(3) + # the 3 for user_id: 1, project_id:1 being + # [classification_user_group, diff_workflow_event, diff_time_event] + expect(counts[0].count).to eq(3) + expect(counts[1].count).to eq(1) + expect(counts[1].count).to eq(1) + end + + it 'returns counts of events within given date range' do + last_week = Date.today - 7 + yesterday = Date.today - 1 + params[:start_date] = last_week.to_s + params[:end_date] = yesterday.to_s + counts = group_member_breakdown_query.call(params) + expect(counts.length).to eq(1) + expect(counts[0].count).to eq(1) + expect(counts[0].project_id).to eq(diff_time_event.project_id) + expect(counts[0].user_id).to eq(diff_time_event.user_id) + end + end +end From 459445c0fe93df42b7408555efa9144a2cfabfc8 Mon Sep 17 00:00:00 2001 From: yuenmichelle1 Date: Thu, 10 Aug 2023 17:57:42 -0500 Subject: [PATCH 06/20] add spec for serializer --- ..._member_stats_breakdown_serializer_spec.rb | 75 +++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100644 spec/serializers/user_group_member_stats_breakdown_serializer_spec.rb diff --git a/spec/serializers/user_group_member_stats_breakdown_serializer_spec.rb b/spec/serializers/user_group_member_stats_breakdown_serializer_spec.rb new file mode 100644 index 0000000..c964d5a --- /dev/null +++ b/spec/serializers/user_group_member_stats_breakdown_serializer_spec.rb @@ -0,0 +1,75 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe UserGroupMemberStatsBreakdownSerializer do + let(:user_project_classification_count) { build(:daily_user_project_classification_count) } + + let(:count_serializer) { described_class.new([user_project_classification_count]) } + + it 'returns group_member_stats as array' do + serialized = count_serializer.as_json({}) + expect(serialized).to have_key(:group_member_stats_breakdown) + expect(serialized[:group_member_stats_breakdown].length).to eq(1) + expect(serialized[:group_member_stats_breakdown][0]).to have_key(:user_id) + expect(serialized[:group_member_stats_breakdown][0]).to have_key(:count) + expect(serialized[:group_member_stats_breakdown][0]).to have_key(:session_time) + expect(serialized[:group_member_stats_breakdown][0]).to have_key(:project_contributions) + end + + it 'sums up total_count per user correctly' do + count2 = build(:user_diff_proj_classification_count) + classification_counts = [user_project_classification_count, count2] + serializer = described_class.new(classification_counts) + serialized = serializer.as_json({}) + expect(serialized[:group_member_stats_breakdown][0][:count]).to eq(classification_counts.sum(&:count)) + end + + it 'sums up time_spent per user correctly' do + count2 = build(:user_diff_proj_classification_count) + classification_counts = [user_project_classification_count, count2] + serializer = described_class.new(classification_counts) + serialized = serializer.as_json({}) + expect(serialized[:group_member_stats_breakdown][0][:session_time]).to eq(classification_counts.sum(&:session_time)) + end + + it 'shows project contributions per user correctly' do + count2 = build(:user_diff_proj_classification_count) + classification_counts = [user_project_classification_count, count2] + serializer = described_class.new(classification_counts) + serialized = serializer.as_json({}) + member_project_contributions = serialized[:group_member_stats_breakdown][0][:project_contributions] + expect(member_project_contributions.length).to eq(2) + expect(member_project_contributions[0]).not_to have_key('user_id') + expect(member_project_contributions[0]).to have_key('project_id') + expect(member_project_contributions[0]).to have_key('count') + expect(member_project_contributions[0]).to have_key('session_time') + end + + it 'shows user project contributions in order by count desc' do + count2 = build(:user_diff_proj_classification_count) + count2.count = user_project_classification_count.count + 100 + classification_counts = [user_project_classification_count, count2] + serializer = described_class.new(classification_counts) + serialized = serializer.as_json({}) + member_project_contributions = serialized[:group_member_stats_breakdown][0][:project_contributions] + expect(member_project_contributions[0]['project_id']).to eq(count2.project_id) + expect(member_project_contributions[0]['count']).to eq(count2.count) + expect(member_project_contributions[0]['session_time']).to eq(count2.session_time) + expect(member_project_contributions[1]['project_id']).to eq(user_project_classification_count.project_id) + expect(member_project_contributions[1]['count']).to eq(user_project_classification_count.count) + expect(member_project_contributions[1]['session_time']).to eq(user_project_classification_count.session_time) + end + + it 'shows group_memer_stats_breakdown in order by top contributors' do + diff_group_member_stats = build(:daily_user_project_classification_count) + diff_group_member_stats.user_id = 2 + diff_group_member_stats.count = user_project_classification_count.count + 100 + classification_counts = [user_project_classification_count, diff_group_member_stats] + serializer = described_class.new(classification_counts) + serialized = serializer.as_json({}) + expect(serialized[:group_member_stats_breakdown].length).to eq(2) + expect(serialized[:group_member_stats_breakdown][0][:user_id]).to eq(diff_group_member_stats.user_id) + expect(serialized[:group_member_stats_breakdown][1][:user_id]).to eq(user_project_classification_count.user_id) + end +end From 6fd8cd457e3202730109bce2a151034aeb50b214 Mon Sep 17 00:00:00 2001 From: yuenmichelle1 Date: Thu, 10 Aug 2023 18:43:52 -0500 Subject: [PATCH 07/20] update session time to be included with top contributors --- app/queries/count_group_active_user_classifications.rb | 2 +- spec/queries/count_group_active_user_classifications_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/queries/count_group_active_user_classifications.rb b/app/queries/count_group_active_user_classifications.rb index c25765d..27ccae3 100644 --- a/app/queries/count_group_active_user_classifications.rb +++ b/app/queries/count_group_active_user_classifications.rb @@ -23,7 +23,7 @@ def initial_scope(relation) end def select_clause - 'user_id, SUM(classification_count)::integer AS count' + 'user_id, SUM(classification_count)::integer AS count, SUM(total_session_time)::float AS session_time' end def relation(params) diff --git a/spec/queries/count_group_active_user_classifications_spec.rb b/spec/queries/count_group_active_user_classifications_spec.rb index 93b856e..fce64b6 100644 --- a/spec/queries/count_group_active_user_classifications_spec.rb +++ b/spec/queries/count_group_active_user_classifications_spec.rb @@ -24,7 +24,7 @@ describe 'select_clause' do it 'selects user_id and orders users by count' do counts = group_active_users_query.call(params) - expected_select_query = 'SELECT user_id, SUM(classification_count)::integer AS count FROM "daily_group_classification_count_and_time_per_user" ' + expected_select_query = 'SELECT user_id, SUM(classification_count)::integer AS count, SUM(total_session_time)::float AS session_time FROM "daily_group_classification_count_and_time_per_user" ' expected_select_query += 'GROUP BY "daily_group_classification_count_and_time_per_user"."user_id" ORDER BY count DESC' expect(counts.to_sql).to eq(expected_select_query) end From 981e25175f820fbf6fd0710e9fef730c9a24a2ef Mon Sep 17 00:00:00 2001 From: yuenmichelle1 Date: Tue, 22 Aug 2023 15:44:07 -0500 Subject: [PATCH 08/20] update staging credentials to include staging panoptes client credentials. add initial logic for queried user group context policy --- ...r_group_classification_count_controller.rb | 22 +++++-- .../queried_user_group_context_policy.rb | 66 +++++++++++++++++++ config/credentials/staging.yml.enc | 2 +- 3 files changed, 82 insertions(+), 8 deletions(-) create mode 100644 app/policies/queried_user_group_context_policy.rb diff --git a/app/controllers/user_group_classification_count_controller.rb b/app/controllers/user_group_classification_count_controller.rb index fa15afc..a143c33 100644 --- a/app/controllers/user_group_classification_count_controller.rb +++ b/app/controllers/user_group_classification_count_controller.rb @@ -3,14 +3,13 @@ class UserGroupClassificationCountController < ApplicationController before_action :validate_params before_action :sanitize_params - # before_action :require_login + before_action :require_login def query - # TODO: Skipping Auth for now, Will introduce this in a separate PR - # pundit policy for user groups, will probably look something like below: - # current_user['queried_user_group_id'] = params[:id] - # authorize :queried_user_group_context, :show? - skip_authorization + current_user['user_group_stats_visibility'] = queried_user_group['stats_visibility'] + current_user['individual_stats_breakdown'] = params[:individual_stats_breakdown] + current_user['user_membership'] = current_user_membership + authorize :queried_user_group_context, :show? if params[:individual_stats_breakdown] group_member_classification_counts = CountGroupMemberBreakdown.new.call(group_classification_count_params) @@ -23,12 +22,21 @@ def query project_contributions = CountGroupProjectContributions.new.call(group_classification_count_params) unless params[:project_id] || params[:workflow_id] render json: UserGroupClassificationCountsSerializer.new(group_classification_counts, group_active_user_classification_counts, project_contributions), serializer_options: serializer_opts_from_params - end end private + def current_user_membership + url = "/memberships?user_id=#{current_user['id']&.to_i}&user_group_id=#{params[:id]}" + client.panoptes.get(url)['memberships'][0] + end + + def queried_user_group + url = "/user_groups/#{params[:id]}" + panoptes_application_client.panoptes.get(url)['user_groups'][0] + end + def validate_params super raise ValidationError, 'Cannot query individual stats breakdown with anything else EXCEPT start_date and end_date' if params[:individual_stats_breakdown] && non_date_range_params diff --git a/app/policies/queried_user_group_context_policy.rb b/app/policies/queried_user_group_context_policy.rb new file mode 100644 index 0000000..4a38d02 --- /dev/null +++ b/app/policies/queried_user_group_context_policy.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +class QueriedUserGroupContextPolicy < ApplicationPolicy + attr_reader :user + + def initialize(user, _record) + super + @user = user + end + + def show? + if individual_stats_breakdown_requested? + show_ind_stats_breakdown + else + show_group_aggregate_stats + end + end + + def show_group_aggregate_stats + case group_stats_visibility + when 'public_show_all', 'public_agg_show_ind_if_member', 'public_agg_only' + true + when 'private_show_agg_and_ind', 'private_agg_only' + group_member? + else + false + end + end + + def show_ind_stats_breakdown + case group_stats_visibility + when 'public_show_all' + true + when 'public_agg_show_ind_if_member', 'private_show_agg_and_ind' + group_member? + when 'public_agg_only', 'private_agg_only' + group_admin? + else + false + end + end + + def group_member? + current_user_membership && !current_user_membership['roles'].empty? + end + + def group_admin? + current_user_membership && current_user_roles.include?('admin') + end + + def current_user_roles + current_user_membership['roles'] + end + + def current_user_membership + user['user_membership'] + end + + def individual_stats_breakdown_requested? + user['individual_stats_breakdown'] || false + end + + def group_stats_visibility + user['user_group_stats_visibility'] + end +end diff --git a/config/credentials/staging.yml.enc b/config/credentials/staging.yml.enc index 52245dd..37baafa 100644 --- a/config/credentials/staging.yml.enc +++ b/config/credentials/staging.yml.enc @@ -1 +1 @@ -II+6JZW9k90zgr2aCaLZI2Xql7gsWKQTpsekQ8EijEC9d4ftBGo9PojC4TlyiGxU80tIpQZ7ZyGfazDOq/l2FtqppIWMJHRYrUfwaklVoeBvIGRtWhWIL5UqchuKBHVLH6XXuzhQ10ZnTolCDnKKVL6/F78j1HtPFf/zyZ8CjKH3gn/+Ot8Z5hGvzPBDJwpUgkEPk1PC7TrcfaBnyLX8J/ukT2Tl8XpGY27VnanXwNWP6GBDfnEobciJMYEDuHkaKuB/fiP1hxl7OHk86JIUO2TBZoQt4yOFWk7ugBjpHryn5m+cYhgjJYvlmfXYndKqKtd/3JYmerbZdO8N6eqFPrih/sinbfyy5n1WxD0oKRUFaKKEuE4U/IMHv3rEPIgA//zM1WK1XdZXZ/i4e9d2jhlNWJZf7OC/WXOw0wmfhqfMXbpRSBWUW8pHAbwtieDgLusyynGHrPhCNlzfOxmcO4a6GmAxR7Ji0sA+Dvm/ysjk+RqS1tDsWYWsH0hH993UitKCw83fy5R6RTgRH8eaicm1+3c85hp4lupJlKyNqOb8KOk413O88hJz7OH17yCOeg6HrHrClkmwj8XNKO4YbKRdZVsGZlOULfCdgFUrqQ==--tKUnDNwupKCZZfnq--N4g56vr1FAxDxrvnp9D57A== \ No newline at end of file +XHp82h6+kwrSEnfmbaoTpaA41yJIKoZxukYTtnwVMoeShy0colRrr1+y7Ld/I6R7Qks/2As8HI+ggU7BU+T5vXfuyPHNC6zffDy8O06WdfNpdZsGAKVFMj9d6SfL8jV0T557LrBSfzJc+0nelFT5XDt0HX8br5VG1OsFFnu6rPx7gpiadNdkDEG05m+RwYm/2GUqX2NFSHQtchS8pZ1b9dAkRqg0TdnZoS59xSLMwPT2MnJcLoP6k+1tgreNkS7SBHBFush5MhGfI4LV7hig3SUuuPCjn/J5w1lvlHcmMGzbngtNb7Jpm+MkxQQtt+/qJBNwL0F2ta65/e425qQKRnLQNFuvsS1tVmR2bFr1mS7FnZj/6t1hUVForNCqDzr/M2ysO7HseeyoQdlwLTVcBlvXptG/gTLaRYbPmLAwa62wBX6d9d4dpjupsh03lFkYAdpF0KD1JCTiPIOaEmxhtnHzY3JAQjCCxaOocvWI3cAY+3Hq62EuKzCs+3TXManA26fXztbk7s0G1nXOOip3tYQUAm5ya1gqUu1jreYWCq/zlNoCbYCobQI+4mTOJcrGzJC4vABasU5Cu5IRmeYogwHITRPX5Tc/TFVf9M9I1kNldJbWkzeCwfc8TrWHWdnRYVSk0qzBYo9zi+gxQsJEoSu8BDdC1SOfjf58RQwqwzK5f4HNRwwSWVO5xf3kQA7V52eIl0yg2f8BEZU6wgKhDcaB/6x3UmybNEc69b5san62F3RxNSGMM+nK7wxyApF8uZTr68b9rlE9w1EFfeZ7QRU9aL+6IDIIduxa0SdFdgNSFlpcNDNhRo8+HZJ0zlgYeO98QiQOMbezjbfk--bnPRfx2pmuRY9/UL--WIQHXg1DuW653O1ABeWaog== \ No newline at end of file From 802870a11c0d59a4e36c4b0abd926ee00c8f497c Mon Sep 17 00:00:00 2001 From: yuenmichelle1 Date: Tue, 22 Aug 2023 15:46:33 -0500 Subject: [PATCH 09/20] add comment to show reference of where to find diff types of user group stats visibilities --- app/policies/queried_user_group_context_policy.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/policies/queried_user_group_context_policy.rb b/app/policies/queried_user_group_context_policy.rb index 4a38d02..45d12aa 100644 --- a/app/policies/queried_user_group_context_policy.rb +++ b/app/policies/queried_user_group_context_policy.rb @@ -17,6 +17,8 @@ def show? end def show_group_aggregate_stats + # For types of group stats visibilities see: https://github.com/zooniverse/eras/wiki/(Panoptes)-User-Groups-Stats-Visibilities + case group_stats_visibility when 'public_show_all', 'public_agg_show_ind_if_member', 'public_agg_only' true From 51961e6f6a8469612a19d8bea623dfa7d6745135 Mon Sep 17 00:00:00 2001 From: yuenmichelle1 Date: Tue, 22 Aug 2023 16:55:06 -0500 Subject: [PATCH 10/20] show stats if panoptes admin --- app/policies/queried_user_group_context_policy.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/policies/queried_user_group_context_policy.rb b/app/policies/queried_user_group_context_policy.rb index 45d12aa..58ee60a 100644 --- a/app/policies/queried_user_group_context_policy.rb +++ b/app/policies/queried_user_group_context_policy.rb @@ -9,11 +9,12 @@ def initialize(user, _record) end def show? - if individual_stats_breakdown_requested? + show_group_stats = if individual_stats_breakdown_requested? show_ind_stats_breakdown else show_group_aggregate_stats end + show_group_stats || panoptes_admin? end def show_group_aggregate_stats @@ -43,11 +44,11 @@ def show_ind_stats_breakdown end def group_member? - current_user_membership && !current_user_membership['roles'].empty? + current_user_membership && !current_user_roles.empty? end def group_admin? - current_user_membership && current_user_roles.include?('admin') + group_member? && current_user_roles.include?('admin') end def current_user_roles From 536ef9b7a786341a9d86f02a02305f546bd8b5f1 Mon Sep 17 00:00:00 2001 From: yuenmichelle1 Date: Tue, 22 Aug 2023 18:37:23 -0500 Subject: [PATCH 11/20] Update queried_user_group_context_policy.rb --- app/policies/queried_user_group_context_policy.rb | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/app/policies/queried_user_group_context_policy.rb b/app/policies/queried_user_group_context_policy.rb index 58ee60a..eaf8881 100644 --- a/app/policies/queried_user_group_context_policy.rb +++ b/app/policies/queried_user_group_context_policy.rb @@ -9,15 +9,16 @@ def initialize(user, _record) end def show? - show_group_stats = if individual_stats_breakdown_requested? - show_ind_stats_breakdown + return true if panoptes_admin? + + if individual_stats_breakdown_requested? + show_ind_stats_breakdown? else - show_group_aggregate_stats + show_group_aggregate_stats? end - show_group_stats || panoptes_admin? end - def show_group_aggregate_stats + def show_group_aggregate_stats? # For types of group stats visibilities see: https://github.com/zooniverse/eras/wiki/(Panoptes)-User-Groups-Stats-Visibilities case group_stats_visibility @@ -30,7 +31,7 @@ def show_group_aggregate_stats end end - def show_ind_stats_breakdown + def show_ind_stats_breakdown? case group_stats_visibility when 'public_show_all' true From 974134984bf1f59a7f708a4b0516edf042937671 Mon Sep 17 00:00:00 2001 From: yuenmichelle1 Date: Wed, 23 Aug 2023 11:57:21 -0500 Subject: [PATCH 12/20] update group admin check to check for group_admin --- app/policies/queried_user_group_context_policy.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/policies/queried_user_group_context_policy.rb b/app/policies/queried_user_group_context_policy.rb index eaf8881..f51fabc 100644 --- a/app/policies/queried_user_group_context_policy.rb +++ b/app/policies/queried_user_group_context_policy.rb @@ -49,7 +49,7 @@ def group_member? end def group_admin? - group_member? && current_user_roles.include?('admin') + group_member? && current_user_roles.include?('group_admin') end def current_user_roles From b8b12913fcc941c8a8e56159bbad93989d2ef139 Mon Sep 17 00:00:00 2001 From: yuenmichelle1 Date: Thu, 24 Aug 2023 15:11:24 -0500 Subject: [PATCH 13/20] update user group controller specs --- ...up_classification_count_controller_spec.rb | 365 +++++++++++++++++- spec/support/authentication_helpers.rb | 32 ++ 2 files changed, 383 insertions(+), 14 deletions(-) diff --git a/spec/controllers/user_group_classification_count_controller_spec.rb b/spec/controllers/user_group_classification_count_controller_spec.rb index 8a0ad13..5778c83 100644 --- a/spec/controllers/user_group_classification_count_controller_spec.rb +++ b/spec/controllers/user_group_classification_count_controller_spec.rb @@ -3,10 +3,12 @@ require 'rails_helper' RSpec.describe UserGroupClassificationCountController do + include AuthenticationHelpers + describe 'GET query' do let!(:classification_user_group) { create(:classification_user_group) } - context 'individual_stats_breakdown is false/not a param' do + shared_examples 'shows group aggregate stats' do it 'returns total_count, time_spent, active_users, and project_contributions of user group' do get :query, params: { id: classification_user_group.user_group_id.to_s } expect(response.status).to eq(200) @@ -16,28 +18,353 @@ expect(response_body['active_users']).to eq(1) expect(response_body['project_contributions'].length).to eq(1) end + end - it 'does not compute project_contributions when params[:project_id] given' do - get :query, params: { id: classification_user_group.user_group_id.to_s, project_id: 2 } + shared_examples 'shows group individual stats breakdown' do + it 'returns group_member_stats_breakdown' do + get :query, params: { id: classification_user_group.user_group_id.to_s, individual_stats_breakdown: true } expect(response.status).to eq(200) response_body = JSON.parse(response.body) - expect(response_body).not_to have_key('project_contributions') + expect(response_body).to have_key('group_member_stats_breakdown') end + end - it 'does not compute project_contributions when params[:workflow_id] given' do - get :query, params: { id: classification_user_group.user_group_id.to_s, workflow_id: 2 } - expect(response.status).to eq(200) - response_body = JSON.parse(response.body) - expect(response_body).not_to have_key('project_contributions') + shared_context 'user_group member' do + before(:each) { + authenticate_with_membership!(classification_user_group, [membership(classification_user_group)]) + } + end + + shared_context 'user_group admin' do + before(:each) { + authenticate_with_membership!(classification_user_group, [membership(classification_user_group, 'group_admin')]) + } + end + + shared_context 'zooniverse admin' do + before(:each) { + authenticate_with_membership!(classification_user_group, [], is_panoptes_admin: true) + } + end + + shared_context 'user group with stats_visibility' do |stats_visibility| + before(:each) { + user_groups_url = "/user_groups/#{classification_user_group.user_group_id}" + allow(panoptes_application_client).to receive_message_chain(:panoptes, :get).with(user_groups_url).and_return('user_groups' => [user_group(classification_user_group, stats_visibility)]) + } + end + + before(:each) { + allow(controller).to receive(:panoptes_application_client).and_return(panoptes_application_client) + } + + context 'individual_stats_breakdown is false/not a param' do + context 'public_show_all' do + include_context 'user group with stats_visibility', 'public_show_all' + context 'querying user is not a member' do + before(:each) { + authenticate_with_membership!(classification_user_group, []) + } + + it_behaves_like 'shows group aggregate stats' + + it 'does not compute project_contributions when params[:project_id] given' do + get :query, params: { id: classification_user_group.user_group_id.to_s, project_id: 2 } + expect(response.status).to eq(200) + response_body = JSON.parse(response.body) + expect(response_body).not_to have_key('project_contributions') + end + + it 'does not compute project_contributions when params[:workflow_id] given' do + get :query, params: { id: classification_user_group.user_group_id.to_s, workflow_id: 2 } + expect(response.status).to eq(200) + response_body = JSON.parse(response.body) + expect(response_body).not_to have_key('project_contributions') + end + end + + context 'querying user is a group member' do + include_context 'user_group member' + it_behaves_like 'shows group aggregate stats' + end + + context 'querying user is a group admin' do + include_context 'user_group admin' + it_behaves_like 'shows group aggregate stats' + end + + context 'querying user is a zooniverse admin' do + include_context 'zooniverse admin' + it_behaves_like 'shows group aggregate stats' + end + end + + context 'public_agg_show_ind_if_member' do + include_context 'user group with stats_visibility', 'public_agg_show_ind_if_member' + + context 'querying user is not a member' do + before(:each) { + authenticate_with_membership!(classification_user_group, []) + } + + it_behaves_like 'shows group aggregate stats' + end + + context 'querying user is a group member' do + include_context 'user_group member' + it_behaves_like 'shows group aggregate stats' + end + + context 'querying user is a group admin' do + include_context 'user_group admin' + it_behaves_like 'shows group aggregate stats' + end + + context 'querying user is a zooniverse admin' do + include_context 'zooniverse admin' + it_behaves_like 'shows group aggregate stats' + end + end + + context 'public_agg_only' do + include_context 'user group with stats_visibility', 'public_agg_only' + + context 'querying user is not a member' do + before(:each) { + authenticate_with_membership!(classification_user_group, []) + } + + it_behaves_like 'shows group aggregate stats' + end + + context 'querying user is a group member' do + include_context 'user_group member' + it_behaves_like 'shows group aggregate stats' + end + + context 'querying user is a group admin' do + include_context 'user_group admin' + it_behaves_like 'shows group aggregate stats' + end + + context 'querying user is a zooniverse admin' do + include_context 'zooniverse admin' + it_behaves_like 'shows group aggregate stats' + end + end + + context 'private_show_agg_and_ind' do + include_context 'user group with stats_visibility', 'private_show_agg_and_ind' + + context 'querying user is not a member' do + before(:each) { + authenticate_with_membership!(classification_user_group, []) + } + + it 'does not show group aggregate stats' do + get :query, params: { id: classification_user_group.user_group_id.to_s } + expect(response.status).to eq(403) + end + end + + context 'querying user is a group member' do + include_context 'user_group member' + it_behaves_like 'shows group aggregate stats' + end + + context 'querying user is a group admin' do + include_context 'user_group admin' + it_behaves_like 'shows group aggregate stats' + end + + context 'querying user is a zooniverse admin' do + include_context 'zooniverse admin' + it_behaves_like 'shows group aggregate stats' + end + end + + context 'private_agg_only' do + include_context 'user group with stats_visibility', 'private_agg_only' + + context 'querying user is not a member' do + before(:each) { + authenticate_with_membership!(classification_user_group, []) + } + + it 'does not show group aggregate stats' do + get :query, params: { id: classification_user_group.user_group_id.to_s } + expect(response.status).to eq(403) + end + end + + context 'querying user is a group member' do + include_context 'user_group member' + it_behaves_like 'shows group aggregate stats' + end + + context 'querying user is a group admin' do + include_context 'user_group admin' + it_behaves_like 'shows group aggregate stats' + end + + context 'querying user is a zooniverse admin' do + include_context 'zooniverse admin' + it_behaves_like 'shows group aggregate stats' + end end end context 'individual_stats_breakdown is true' do - it 'returns group_member_stats_breakdown' do - get :query, params: { id: classification_user_group.user_group_id.to_s, individual_stats_breakdown: true } - expect(response.status).to eq(200) - response_body = JSON.parse(response.body) - expect(response_body).to have_key('group_member_stats_breakdown') + context 'public_show_all' do + include_context 'user group with stats_visibility', 'public_show_all' + + context 'querying user is not a member' do + before(:each) { + authenticate_with_membership!(classification_user_group, []) + } + it_behaves_like 'shows group individual stats breakdown' + end + + context 'querying user is a group member' do + include_context 'user_group member' + it_behaves_like 'shows group individual stats breakdown' + end + + context 'querying user is a group admin' do + include_context 'user_group admin' + it_behaves_like 'shows group individual stats breakdown' + end + + context 'querying user is a zooniverse admin' do + include_context 'zooniverse admin' + it_behaves_like 'shows group individual stats breakdown' + end + end + + context 'public_agg_show_ind_if_member' do + include_context 'user group with stats_visibility', 'public_agg_show_ind_if_member' + + context 'querying user is not a member' do + before(:each) { + authenticate_with_membership!(classification_user_group, []) + } + + it 'does not show individual stats breakdown' do + get :query, params: { id: classification_user_group.user_group_id.to_s, individual_stats_breakdown: true } + expect(response.status).to eq(403) + end + end + + context 'querying user is a group member' do + include_context 'user_group member' + it_behaves_like 'shows group individual stats breakdown' + end + + context 'querying user is a group admin' do + include_context 'user_group admin' + it_behaves_like 'shows group individual stats breakdown' + end + + context 'querying user is a zooniverse admin' do + include_context 'zooniverse admin' + it_behaves_like 'shows group individual stats breakdown' + end + end + + context 'public_agg_only' do + include_context 'user group with stats_visibility', 'public_agg_only' + + context 'querying user is not a member' do + before(:each) { + authenticate_with_membership!(classification_user_group, []) + } + + it 'does not show individual stats breakdown' do + get :query, params: { id: classification_user_group.user_group_id.to_s, individual_stats_breakdown: true } + expect(response.status).to eq(403) + end + end + + context 'querying user is a group member' do + include_context 'user_group member' + it 'does not show individual stats breakdown' do + get :query, params: { id: classification_user_group.user_group_id.to_s, individual_stats_breakdown: true } + expect(response.status).to eq(403) + end + end + + context 'querying user is a group admin' do + include_context 'user_group admin' + it_behaves_like 'shows group individual stats breakdown' + end + + context 'querying user is a zooniverse admin' do + include_context 'zooniverse admin' + it_behaves_like 'shows group individual stats breakdown' + end + end + + context 'private_show_agg_and_ind' do + include_context 'user group with stats_visibility', 'private_show_agg_and_ind' + + context 'querying user is not a member' do + before(:each) { + authenticate_with_membership!(classification_user_group, []) + } + + it 'does not show individual stats breakdown' do + get :query, params: { id: classification_user_group.user_group_id.to_s, individual_stats_breakdown: true } + expect(response.status).to eq(403) + end + end + + context 'querying user is a group member' do + include_context 'user_group member' + it_behaves_like 'shows group individual stats breakdown' + end + + context 'querying user is a group admin' do + include_context 'user_group admin' + it_behaves_like 'shows group individual stats breakdown' + end + + context 'querying user is a zooniverse admin' do + include_context 'zooniverse admin' + it_behaves_like 'shows group individual stats breakdown' + end + end + + context 'private_agg_only' do + include_context 'user group with stats_visibility', 'private_agg_only' + + context 'querying user is not a member' do + before(:each) { + authenticate_with_membership!(classification_user_group, []) + } + + it 'does not show individual stats breakdown' do + get :query, params: { id: classification_user_group.user_group_id.to_s, individual_stats_breakdown: true } + expect(response.status).to eq(403) + end + end + + context 'querying user is a group member' do + include_context 'user_group member' + it 'does not show individual stats breakdown' do + get :query, params: { id: classification_user_group.user_group_id.to_s, individual_stats_breakdown: true } + expect(response.status).to eq(403) + end + end + + context 'querying user is a group admin' do + include_context 'user_group admin' + it_behaves_like 'shows group individual stats breakdown' + end + + context 'querying user is a zooniverse admin' do + include_context 'zooniverse admin' + it_behaves_like 'shows group individual stats breakdown' + end end end @@ -66,4 +393,14 @@ end end end + + def membership(classification_user_group, role_type='group_member') + { 'id' => 123, + 'user_group_id' => classification_user_group.user_group_id, 'user_id' => classification_user_group.user_id, + 'roles' => [role_type] } + end + + def user_group(classification_user_group, stats_visibility) + { 'id' => classification_user_group.user_group_id, 'stats_visibility' => stats_visibility } + end end diff --git a/spec/support/authentication_helpers.rb b/spec/support/authentication_helpers.rb index c234de2..e5746d5 100644 --- a/spec/support/authentication_helpers.rb +++ b/spec/support/authentication_helpers.rb @@ -5,6 +5,38 @@ def authenticate!(user_id='9999', is_panoptes_admin: false) allow(controller).to receive(:client).and_return(user_client(user_id, is_panoptes_admin)) end + def authenticate_with_membership!(classification_user_group, memberships,is_panoptes_admin: false) + allow(controller).to receive(:client).and_return(user_client_with_membership(classification_user_group, memberships, is_panoptes_admin)) + end + + def panoptes_application_client + return @panoptes_application_client if @panoptes_application_client + + @panoptes_application_client = instance_double(Panoptes::Client) + + @panoptes_application_client + end + + def user_client_with_membership(classification_user_group, memberships, is_panoptes_admin) + return @user_client_with_membership if @user_client_with_membership + + me_hash = { + 'id' => classification_user_group.user_id, + 'login' => 'login', + 'display_name' => 'display_name', + 'admin' => is_panoptes_admin + } + memberships_url = "/memberships?user_id=#{classification_user_group.user_id}&user_group_id=#{classification_user_group.user_group_id}" + + @user_client_with_membership = double(Panoptes::Client, me: me_hash).tap do |client| + allow(client).to receive(:is_a?).and_return(false) + allow(client).to receive(:is_a?).with(Panoptes::Client).and_return(true) + allow(client).to receive_message_chain(:panoptes, :get).with(memberships_url).and_return('memberships' => memberships) + end + + @user_client_with_membership + end + def user_client(user_id, is_panoptes_admin) return @user_client if @user_client From 874b5f092d1a43359a02eaaa19cefeac9493a07c Mon Sep 17 00:00:00 2001 From: yuenmichelle1 Date: Thu, 24 Aug 2023 16:14:03 -0500 Subject: [PATCH 14/20] add queried user group context policy spec --- .../queried_user_group_context_policy_spec.rb | 160 ++++++++++++++++++ 1 file changed, 160 insertions(+) create mode 100644 spec/policies/queried_user_group_context_policy_spec.rb diff --git a/spec/policies/queried_user_group_context_policy_spec.rb b/spec/policies/queried_user_group_context_policy_spec.rb new file mode 100644 index 0000000..18b1cb3 --- /dev/null +++ b/spec/policies/queried_user_group_context_policy_spec.rb @@ -0,0 +1,160 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe QueriedUserGroupContextPolicy do + let(:querying_user) { + { + 'id' => '1234', + 'login' => 'login', + 'display_name' => 'display_name' + } + } + + permissions :show? do + it 'permits querying_user to query if panoptes admin' do + querying_user['admin'] = true + expect(described_class).to permit(querying_user) + end + + context 'individual stats breakdown requested' do + before(:each) do + querying_user['individual_stats_breakdown'] = true + end + + it 'permits if group_stats_visibility is public_show_all' do + querying_user['user_group_stats_visibility'] = 'public_show_all' + expect(described_class).to permit(querying_user) + end + + context 'public_agg_show_ind_if_member' do + before(:each) do + querying_user['user_group_stats_visibility'] = 'public_agg_show_ind_if_member' + end + it 'forbids if user is not a member' do + expect(described_class).not_to permit(querying_user) + end + + it 'permits if user is a member' do + querying_user['user_membership'] = membership + expect(described_class).to permit(querying_user) + end + end + + context 'private_show_agg_and_ind' do + before(:each) do + querying_user['user_group_stats_visibility'] = 'private_show_agg_and_ind' + end + it 'forbids if user is not a member' do + expect(described_class).not_to permit(querying_user) + end + + it 'permits if user is a member' do + querying_user['user_membership'] = membership + expect(described_class).to permit(querying_user) + end + end + + context 'public_agg_only' do + before(:each) do + querying_user['user_group_stats_visibility'] = 'public_agg_only' + end + it 'forbids if user is not a member' do + expect(described_class).not_to permit(querying_user) + end + + it 'forbids if user is a member' do + querying_user['user_membership'] = membership + expect(described_class).not_to permit(querying_user) + end + + it 'permits if user is an admin' do + querying_user['user_membership'] = membership('group_admin') + expect(described_class).to permit(querying_user) + end + end + + context 'private_agg_only' do + before(:each) do + querying_user['user_group_stats_visibility'] = 'private_agg_only' + end + it 'forbids if user is not a member' do + expect(described_class).not_to permit(querying_user) + end + + it 'forbids if user is a member' do + querying_user['user_membership'] = membership + expect(described_class).not_to permit(querying_user) + end + + it 'permits if user is an admin' do + querying_user['user_membership'] = membership('group_admin') + expect(described_class).to permit(querying_user) + end + end + + it 'forbids if group visibility is not of listed types' do + querying_user['user_group_stats_visibility'] = 'other' + expect(described_class).not_to permit(querying_user) + end + end + + context 'group aggregate stats' do + it 'permits if group_stats_visibility is public_show_all' do + querying_user['user_group_stats_visibility'] = 'public_show_all' + expect(described_class).to permit(querying_user) + end + + it 'permits if group_stats_visibility is public_agg_show_ind_if_member' do + querying_user['user_group_stats_visibility'] = 'public_agg_show_ind_if_member' + expect(described_class).to permit(querying_user) + end + + it 'permits if group_stats_visibility is public_agg_only' do + querying_user['user_group_stats_visibility'] = 'public_agg_only' + expect(described_class).to permit(querying_user) + end + + context 'private_show_agg_and_ind' do + before(:each) do + querying_user['user_group_stats_visibility'] = 'private_show_agg_and_ind' + end + + it 'forbids if user is not a member' do + expect(described_class).not_to permit(querying_user) + end + + it 'permits if user is a group member' do + querying_user['user_membership'] = membership + expect(described_class).to permit(querying_user) + end + end + + context 'private_agg_only' do + before(:each) do + querying_user['user_group_stats_visibility'] = 'private_agg_only' + end + + it 'forbids if user is not a group member' do + expect(described_class).not_to permit(querying_user) + end + + it 'permits if user is a group member' do + querying_user['user_membership'] = membership + expect(described_class).to permit(querying_user) + end + end + end + + it 'forbids unauthorized users' do + expect(described_class).not_to permit(querying_user) + end + end + + def membership(role_type='group_member') + { 'id' => 123, + 'user_group_id' => 123, + 'user_id' => 123, + 'roles' => [role_type] } + end +end From c6b327632cef6ce51a47e12962a78401fb4eef5e Mon Sep 17 00:00:00 2001 From: yuenmichelle1 Date: Fri, 25 Aug 2023 09:17:37 -0500 Subject: [PATCH 15/20] Update user_group_classification_count_controller.rb --- app/controllers/user_group_classification_count_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/user_group_classification_count_controller.rb b/app/controllers/user_group_classification_count_controller.rb index a143c33..c1560fb 100644 --- a/app/controllers/user_group_classification_count_controller.rb +++ b/app/controllers/user_group_classification_count_controller.rb @@ -28,7 +28,7 @@ def query private def current_user_membership - url = "/memberships?user_id=#{current_user['id']&.to_i}&user_group_id=#{params[:id]}" + url = "/memberships?user_id=#{current_user['id']}&user_group_id=#{params[:id]}" client.panoptes.get(url)['memberships'][0] end From 87b22b9753d36d0e3dcd0afd7353050617bdc7ae Mon Sep 17 00:00:00 2001 From: yuenmichelle1 Date: Fri, 25 Aug 2023 12:25:23 -0500 Subject: [PATCH 16/20] update validations to allow non logged in users to view publicly visible stats and update specs --- ...r_group_classification_count_controller.rb | 31 +++++++-- ...er_classification_count_controller_spec.rb | 17 ++--- ...up_classification_count_controller_spec.rb | 66 +++++++++++++++++++ spec/support/auth_header_validator.rb | 16 +++++ spec/support/authentication_helpers.rb | 2 +- 5 files changed, 113 insertions(+), 19 deletions(-) create mode 100644 spec/support/auth_header_validator.rb diff --git a/app/controllers/user_group_classification_count_controller.rb b/app/controllers/user_group_classification_count_controller.rb index c1560fb..a4218e0 100644 --- a/app/controllers/user_group_classification_count_controller.rb +++ b/app/controllers/user_group_classification_count_controller.rb @@ -3,13 +3,15 @@ class UserGroupClassificationCountController < ApplicationController before_action :validate_params before_action :sanitize_params - before_action :require_login + before_action :group_require_login def query - current_user['user_group_stats_visibility'] = queried_user_group['stats_visibility'] - current_user['individual_stats_breakdown'] = params[:individual_stats_breakdown] - current_user['user_membership'] = current_user_membership - authorize :queried_user_group_context, :show? + if authorization_required_for_group? + authorize_user_to_query_group_stats + else + skip_authorization + end + if params[:individual_stats_breakdown] group_member_classification_counts = CountGroupMemberBreakdown.new.call(group_classification_count_params) @@ -27,6 +29,21 @@ def query private + def authorize_user_to_query_group_stats + current_user['user_group_stats_visibility'] = group_stats_visibility + current_user['individual_stats_breakdown'] = params[:individual_stats_breakdown] + current_user['user_membership'] = current_user_membership + authorize :queried_user_group_context, :show? + end + + def authorization_required_for_group? + (params[:individual_stats_breakdown] && group_stats_visibility != 'public_show_all') || group_stats_visibility.include?('private') + end + + def group_require_login + require_login if authorization_required_for_group? + end + def current_user_membership url = "/memberships?user_id=#{current_user['id']}&user_group_id=#{params[:id]}" client.panoptes.get(url)['memberships'][0] @@ -37,6 +54,10 @@ def queried_user_group panoptes_application_client.panoptes.get(url)['user_groups'][0] end + def group_stats_visibility + queried_user_group['stats_visibility'] + end + def validate_params super raise ValidationError, 'Cannot query individual stats breakdown with anything else EXCEPT start_date and end_date' if params[:individual_stats_breakdown] && non_date_range_params diff --git a/spec/controllers/user_classification_count_controller_spec.rb b/spec/controllers/user_classification_count_controller_spec.rb index eb49985..d8e754e 100644 --- a/spec/controllers/user_classification_count_controller_spec.rb +++ b/spec/controllers/user_classification_count_controller_spec.rb @@ -69,19 +69,10 @@ end context 'missing token' do - it 'returns a 403 missing authorization header' do - get :query, params: { id: classification_event.user_id.to_s } - expected_response = { error: 'Missing Authorization header' } - expect(response.status).to eq(403) - expect(response.body).to eq(expected_response.to_json) - end - - it 'returns a 403 missing when missing bearer token' do - request.headers['Authorization'] = 'asjdhaskdhsa' - get :query, params: { id: classification_event.user_id.to_s } - expected_response = { error: 'Missing Bearer token' } - expect(response.status).to eq(403) - expect(response.body).to eq(expected_response.to_json) + it_behaves_like 'returns 403 when authorization header is invalid' do + before(:each) { + get :query, params: { id: classification_event.user_id.to_s } + } end end diff --git a/spec/controllers/user_group_classification_count_controller_spec.rb b/spec/controllers/user_group_classification_count_controller_spec.rb index 5778c83..be49b98 100644 --- a/spec/controllers/user_group_classification_count_controller_spec.rb +++ b/spec/controllers/user_group_classification_count_controller_spec.rb @@ -83,6 +83,10 @@ end end + context 'querying user is not logged in' do + it_behaves_like 'shows group aggregate stats' + end + context 'querying user is a group member' do include_context 'user_group member' it_behaves_like 'shows group aggregate stats' @@ -110,6 +114,10 @@ it_behaves_like 'shows group aggregate stats' end + context 'querying user is not logged in' do + it_behaves_like 'shows group aggregate stats' + end + context 'querying user is a group member' do include_context 'user_group member' it_behaves_like 'shows group aggregate stats' @@ -137,6 +145,10 @@ it_behaves_like 'shows group aggregate stats' end + context 'querying user is not logged in' do + it_behaves_like 'shows group aggregate stats' + end + context 'querying user is a group member' do include_context 'user_group member' it_behaves_like 'shows group aggregate stats' @@ -167,6 +179,14 @@ end end + context 'querying user is not logged in' do + it_behaves_like 'returns 403 when authorization header is invalid' do + before(:each) { + get :query, params: { id: classification_user_group.user_group_id.to_s } + } + end + end + context 'querying user is a group member' do include_context 'user_group member' it_behaves_like 'shows group aggregate stats' @@ -197,6 +217,14 @@ end end + context 'querying user is not logged in' do + it_behaves_like 'returns 403 when authorization header is invalid' do + before(:each) { + get :query, params: { id: classification_user_group.user_group_id.to_s } + } + end + end + context 'querying user is a group member' do include_context 'user_group member' it_behaves_like 'shows group aggregate stats' @@ -225,6 +253,10 @@ it_behaves_like 'shows group individual stats breakdown' end + context 'querying user is not logged in' do + it_behaves_like 'shows group aggregate stats' + end + context 'querying user is a group member' do include_context 'user_group member' it_behaves_like 'shows group individual stats breakdown' @@ -255,6 +287,14 @@ end end + context 'querying user is not logged in' do + it_behaves_like 'returns 403 when authorization header is invalid' do + before(:each) { + get :query, params: { id: classification_user_group.user_group_id.to_s, individual_stats_breakdown: true } + expect(response.status).to eq(403) } + end + end + context 'querying user is a group member' do include_context 'user_group member' it_behaves_like 'shows group individual stats breakdown' @@ -285,6 +325,14 @@ end end + context 'querying user is not logged in' do + it_behaves_like 'returns 403 when authorization header is invalid' do + before(:each) { + get :query, params: { id: classification_user_group.user_group_id.to_s, individual_stats_breakdown: true } + expect(response.status).to eq(403) } + end + end + context 'querying user is a group member' do include_context 'user_group member' it 'does not show individual stats breakdown' do @@ -318,6 +366,14 @@ end end + context 'querying user is not logged in' do + it_behaves_like 'returns 403 when authorization header is invalid' do + before(:each) { + get :query, params: { id: classification_user_group.user_group_id.to_s, individual_stats_breakdown: true } + expect(response.status).to eq(403) } + end + end + context 'querying user is a group member' do include_context 'user_group member' it_behaves_like 'shows group individual stats breakdown' @@ -348,6 +404,14 @@ end end + context 'querying user is not logged in' do + it_behaves_like 'returns 403 when authorization header is invalid' do + before(:each) { + get :query, params: { id: classification_user_group.user_group_id.to_s, individual_stats_breakdown: true } + expect(response.status).to eq(403) } + end + end + context 'querying user is a group member' do include_context 'user_group member' it 'does not show individual stats breakdown' do @@ -369,6 +433,8 @@ end context 'param validations' do + include_context 'user group with stats_visibility', 'public_show_all' + it_behaves_like 'ensure valid query params', :query, id: 1 it 'ensures you cannot query individual_stats_breakdown and any non date range param' do diff --git a/spec/support/auth_header_validator.rb b/spec/support/auth_header_validator.rb new file mode 100644 index 0000000..f48bdb1 --- /dev/null +++ b/spec/support/auth_header_validator.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'returns 403 when authorization header is invalid' do + it 'returns a 403 missing authorization header' do + expected_response = { error: 'Missing Authorization header' } + expect(response.status).to eq(403) + expect(response.body).to eq(expected_response.to_json) + end + + it 'returns a 403 missing authorization header' do + request.headers['Authorization'] = 'asjdhaskdhsa' + expected_response = { error: 'Missing Authorization header' } + expect(response.status).to eq(403) + expect(response.body).to eq(expected_response.to_json) + end +end diff --git a/spec/support/authentication_helpers.rb b/spec/support/authentication_helpers.rb index e5746d5..09d1a7a 100644 --- a/spec/support/authentication_helpers.rb +++ b/spec/support/authentication_helpers.rb @@ -5,7 +5,7 @@ def authenticate!(user_id='9999', is_panoptes_admin: false) allow(controller).to receive(:client).and_return(user_client(user_id, is_panoptes_admin)) end - def authenticate_with_membership!(classification_user_group, memberships,is_panoptes_admin: false) + def authenticate_with_membership!(classification_user_group, memberships, is_panoptes_admin: false) allow(controller).to receive(:client).and_return(user_client_with_membership(classification_user_group, memberships, is_panoptes_admin)) end From 6c5c679b4983699b839b7b1d228064fe0813452e Mon Sep 17 00:00:00 2001 From: yuenmichelle1 Date: Fri, 25 Aug 2023 12:36:05 -0500 Subject: [PATCH 17/20] fix accidental spacing issue --- ...ser_group_classification_count_controller_spec.rb | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/spec/controllers/user_group_classification_count_controller_spec.rb b/spec/controllers/user_group_classification_count_controller_spec.rb index be49b98..5470ae0 100644 --- a/spec/controllers/user_group_classification_count_controller_spec.rb +++ b/spec/controllers/user_group_classification_count_controller_spec.rb @@ -291,7 +291,8 @@ it_behaves_like 'returns 403 when authorization header is invalid' do before(:each) { get :query, params: { id: classification_user_group.user_group_id.to_s, individual_stats_breakdown: true } - expect(response.status).to eq(403) } + expect(response.status).to eq(403) + } end end @@ -329,7 +330,8 @@ it_behaves_like 'returns 403 when authorization header is invalid' do before(:each) { get :query, params: { id: classification_user_group.user_group_id.to_s, individual_stats_breakdown: true } - expect(response.status).to eq(403) } + expect(response.status).to eq(403) + } end end @@ -370,7 +372,8 @@ it_behaves_like 'returns 403 when authorization header is invalid' do before(:each) { get :query, params: { id: classification_user_group.user_group_id.to_s, individual_stats_breakdown: true } - expect(response.status).to eq(403) } + expect(response.status).to eq(403) + } end end @@ -408,7 +411,8 @@ it_behaves_like 'returns 403 when authorization header is invalid' do before(:each) { get :query, params: { id: classification_user_group.user_group_id.to_s, individual_stats_breakdown: true } - expect(response.status).to eq(403) } + expect(response.status).to eq(403) + } end end From fedfefed92dd8685fe3885e7d29c6f79caeac7e6 Mon Sep 17 00:00:00 2001 From: yuenmichelle1 Date: Mon, 28 Aug 2023 09:16:23 -0500 Subject: [PATCH 18/20] Update staging.yml.enc --- config/credentials/staging.yml.enc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/credentials/staging.yml.enc b/config/credentials/staging.yml.enc index b3a4459..6fa9665 100644 --- a/config/credentials/staging.yml.enc +++ b/config/credentials/staging.yml.enc @@ -1 +1 @@ -DE7jRIWwMnbmRiQRJ0SmsOae9uRB5sniKrwXf7F8ZuvfmLeo16iJbHLX+QtRkTG3hsBdJtjYA1fek7cfsLi78ThSon8Ovzaw+opS6h+mMBOT2takrxtlD1kYDYjupHbsTt5zb3MJEzLYXy+8utpy2wOunq/e8jXg28i0asWONbevnd0ieq/E8aVpSmNLNPlE6b4JeQA0Gk+aa1gKVnNPK9ByqWxaXieDimNiBQa90rmO6hdpvIVcDl6FbbZC3+SqOTab5yOdkZudiRvrcHnDc4fdb03V/hvPbk0vXSoL0dr1SsLlCjTgzkciU56NzJ75z7k9pd8GJ5qQnmJnNIgtOjJyK85t8jcVOhqARI3UihE6QXXbHS5BmhB2ssJ7gi031qpLtM6Iax60zhg9TgWU8mothUE3/b8IONQgEGnLwnnF5KoW4QXkQe50vM+ar0Os6bWEh4adfmjXV5MJGknZ+kBa4tWifDXMWS38+uLDaPmtaB4uWx+iQB0UAxYn/+wFlXWi59nOFf+s+85iIuVX6wHyUNSMdnnMZrGhhfBqQ7GC82Jmelu9dVA9TkuMz6UY8Ts1NATn6HYOVu9VWYcxqLpynbIdbNClKNO4w/lGLw==--P8SvT9334PWTh9EK--oeM/Hj0aEswh7ipr+Rv2QQ== +PGbmdbmi4qGqkpAPoEEdruRtfzTdqrFUuxKZyKkHogeNKi3V3XRYY7xCIpiVzaOk9x2P88UnPSi3uQJX00YFC5g11ta00WuH9WmPEjr9zSrpXTQ5Vou+dKoTszP2nPauNwUPkrYTM2q3Ew2691GspeXK7D6I+t0Oi0uVckI2C7RJknpcaqxDUwOVV4idGczyenMKPYPoOQ9rBogwG2JvG1YBsS6xul87lHyYqmVB7a2KmAxMLPNFbTwT3ASuwreDZyrG6e2oyXMBG1tFtnyX3wcPkj9A02oTN4mLxoaBVRVQS4T43sFqbGawlDPUdHZbI5B6+ekWpqgMIBQXkvqORyUTmfMGhbs3TEuNYEfKBkO1fo3+B2XtGNOknJPIavwVvaUFt2KYRh2Pmdk2ew7VA9FTt6mzTy6QewA6lA4vPfs01flrRzGDx/+DJCydL+UXX04izUzyH2eul0NWAPTKJ0btocok0GCcJb5mXktgECJXBKWO9j/90UVeupaxetpGp/SsfMjtjildK9M0X33ehmfzeWTqmFp+2mnfQo5R8p9XPsX/eVpvGwogSS6dDXIjYixlM0lhOeRq6loWguGRPLY5Fv7Uir7iMoc5vh/mk/Ivaqm+Xf7MkWCZ5QRj+FM9UYczp8C5nX3ndL7WJyfUDRPZR8gRLRZoIIKKznC/icw05IqysCcLBI8wQpl7FGLN760IrlWHWwz4lT4DbeIn7sZ7OoLimYkw/x7QGmOZzpSLGed589D3BsERsHhNXPBVKVaWhReenYgME/u/nfV+dwmE0cPJVxmYGllDdZKEeqtPOSwWK5vEksvCAVowwwADjZNCDVaeEdQ6c+y1--MUB9zAKbMkjYaijP--jfzUVjic0hyW9gGZ6Wv58g== \ No newline at end of file From 4708b5d0464cdd4014c5cc5f6bdded0b297b6ffa Mon Sep 17 00:00:00 2001 From: yuenmichelle1 Date: Thu, 7 Sep 2023 10:06:00 -0500 Subject: [PATCH 19/20] Update spec/support/authentication_helpers.rb Co-authored-by: Zach Wolfenbarger --- spec/support/authentication_helpers.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/spec/support/authentication_helpers.rb b/spec/support/authentication_helpers.rb index 09d1a7a..e8337da 100644 --- a/spec/support/authentication_helpers.rb +++ b/spec/support/authentication_helpers.rb @@ -10,11 +10,7 @@ def authenticate_with_membership!(classification_user_group, memberships, is_pan end def panoptes_application_client - return @panoptes_application_client if @panoptes_application_client - - @panoptes_application_client = instance_double(Panoptes::Client) - - @panoptes_application_client + @panoptes_application_client ||= instance_double(Panoptes::Client) end def user_client_with_membership(classification_user_group, memberships, is_panoptes_admin) From dc935104333816367c1627e0092d0c10d8a8f68e Mon Sep 17 00:00:00 2001 From: yuenmichelle1 Date: Thu, 7 Sep 2023 10:27:12 -0500 Subject: [PATCH 20/20] Update authentication_helpers.rb --- spec/support/authentication_helpers.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/spec/support/authentication_helpers.rb b/spec/support/authentication_helpers.rb index e8337da..2b41097 100644 --- a/spec/support/authentication_helpers.rb +++ b/spec/support/authentication_helpers.rb @@ -25,7 +25,6 @@ def user_client_with_membership(classification_user_group, memberships, is_panop memberships_url = "/memberships?user_id=#{classification_user_group.user_id}&user_group_id=#{classification_user_group.user_group_id}" @user_client_with_membership = double(Panoptes::Client, me: me_hash).tap do |client| - allow(client).to receive(:is_a?).and_return(false) allow(client).to receive(:is_a?).with(Panoptes::Client).and_return(true) allow(client).to receive_message_chain(:panoptes, :get).with(memberships_url).and_return('memberships' => memberships) end @@ -44,7 +43,6 @@ def user_client(user_id, is_panoptes_admin) } @user_client = double(Panoptes::Client, me: me_hash).tap do |client| - allow(client).to receive(:is_a?).and_return(false) allow(client).to receive(:is_a?).with(Panoptes::Client).and_return(true) end