From 0656380071ccd870e8f7fd484f2589ae5e720a08 Mon Sep 17 00:00:00 2001 From: yuenmichelle1 Date: Thu, 27 Jul 2023 16:21:46 -0500 Subject: [PATCH 1/3] update top_project_contributions to project_contributions --- .../user_classification_count_controller.rb | 8 ++-- app/queries/count_user_classifications.rb | 6 +-- .../user_classification_counts_serializer.rb | 17 ++++---- ...er_classification_count_controller_spec.rb | 25 ++++++----- .../count_user_classifications_spec.rb | 10 ++--- ...r_classification_counts_serializer_spec.rb | 41 ++++++++----------- 6 files changed, 53 insertions(+), 54 deletions(-) diff --git a/app/controllers/user_classification_count_controller.rb b/app/controllers/user_classification_count_controller.rb index c7c0523..4cf9a76 100644 --- a/app/controllers/user_classification_count_controller.rb +++ b/app/controllers/user_classification_count_controller.rb @@ -16,21 +16,21 @@ def query def validate_params super - raise ValidationError, 'Cannot query top projects and query by project/workflow' if params[:top_project_contributions] && (params[:workflow_id] || params[:project_id]) + raise ValidationError, 'Cannot query top projects and query by project/workflow' if params[:project_contributions] && (params[:workflow_id] || params[:project_id]) end def sanitize_params - params[:top_project_contributions] = params[:top_project_contributions].to_i if params[:top_project_contributions] + params[:project_contributions] = params[:project_contributions].casecmp?('true') if params[:project_contributions] params[:time_spent] = params[:time_spent].casecmp?('true') if params[:time_spent] end def serializer_opts_from_params { period: params[:period], time_spent: params[:time_spent], - top_project_contributions: params[:top_project_contributions] } + project_contributions: params[:project_contributions] } end def user_classification_count_params - params.permit(:id, :start_date, :end_date, :period, :workflow_id, :project_id, :top_project_contributions, :time_spent) + params.permit(:id, :start_date, :end_date, :period, :workflow_id, :project_id, :project_contributions, :time_spent) end end diff --git a/app/queries/count_user_classifications.rb b/app/queries/count_user_classifications.rb index 82555e9..ddd6146 100644 --- a/app/queries/count_user_classifications.rb +++ b/app/queries/count_user_classifications.rb @@ -24,19 +24,19 @@ def initial_scope(relation, params) end def group_by_clause(params) - params[:top_project_contributions] ? 'period, project_id' : 'period' + params[:project_contributions] ? 'period, project_id' : 'period' end def select_clause(params) period = params[:period] clause = select_by(period, 'classification') clause += ', SUM(total_session_time)::float AS session_time' if params[:time_spent] - clause += ', project_id' if params[:top_project_contributions] + clause += ', project_id' if params[:project_contributions] clause end def relation(params) - if params[:project_id] || params[:top_project_contributions] + if params[:project_id] || params[:project_contributions] UserClassificationCounts::DailyUserProjectClassificationCount elsif params[:workflow_id] UserClassificationCounts::DailyUserWorkflowClassificationCount diff --git a/app/serializers/user_classification_counts_serializer.rb b/app/serializers/user_classification_counts_serializer.rb index e05be94..43107cb 100644 --- a/app/serializers/user_classification_counts_serializer.rb +++ b/app/serializers/user_classification_counts_serializer.rb @@ -10,11 +10,12 @@ def initialize(counts_scope) def as_json(options) serializer_options = options[:serializer_options] show_time_spent = serializer_options[:time_spent] + show_project_contributions = serializer_options[:project_contributions] total_count = user_classification_counts.sum(&:count).to_i response = { total_count: } calculate_time_spent(user_classification_counts, response) if show_time_spent - show_proj_contributions(response, serializer_options[:top_project_contributions]) if serializer_options[:top_project_contributions] - response[:data] = response_data(user_classification_counts, serializer_options[:top_project_contributions], show_time_spent:) if serializer_options[:period] + display_project_contributions(response) if show_project_contributions + response[:data] = response_data(user_classification_counts, show_project_contributions:, show_time_spent:) if serializer_options[:period] response end @@ -25,11 +26,11 @@ def calculate_time_spent(counts, response) response[:time_spent] = total_time_spent end - def response_data(user_counts, num_top_projects, show_time_spent:) + def response_data(user_counts, show_project_contributions:, show_time_spent:) # when calculating top projects, our records returned from query will be counts (and session time) per user per project bucketed by time # eg. { period: '01-01-2020', count: 38, project_id: 1 }, { period: '01-01-2020', count: 40, project_id: 2} # vs. Our desired response format which is counts (and session time) grouped by bucketed time. { period: '01-02-2020', count: 78 } - if num_top_projects + if show_project_contributions counts_grouped_by_period = user_counts.group_by { |user_proj_class_count| user_proj_class_count[:period] }.transform_values do |counts_in_period| total_in_period = { count: counts_in_period.sum(&:count) } total_in_period[:session_time] = counts_in_period.sum(&:session_time) if show_time_spent @@ -41,19 +42,19 @@ def response_data(user_counts, num_top_projects, show_time_spent:) end end - def show_proj_contributions(response, num_top_projects_to_show) + def display_project_contributions(response) response[:unique_project_contributions] = unique_projects_count - response[:top_project_contributions] = top_project_contributions(num_top_projects_to_show) + response[:project_contributions] = project_contributions end def unique_projects_count @user_classification_counts.map(&:project_id).uniq.count end - def top_project_contributions(num_top_projects) + def project_contributions project_contributions = @user_classification_counts.group_by(&:project_id).transform_values do |counts| counts.sum(&:count) end - project_contributions.map { |project_id, count| { project_id:, count: } }.sort_by { |proj_contribution| proj_contribution[:count] }.reverse.first(num_top_projects) + project_contributions.map { |project_id, count| { project_id:, count: } }.sort_by { |proj_contribution| proj_contribution[:count] }.reverse end end diff --git a/spec/controllers/user_classification_count_controller_spec.rb b/spec/controllers/user_classification_count_controller_spec.rb index e8418f6..b9c1da8 100644 --- a/spec/controllers/user_classification_count_controller_spec.rb +++ b/spec/controllers/user_classification_count_controller_spec.rb @@ -40,13 +40,13 @@ expect(response_body['data'][0]['session_time']).to eq(classification_event.session_time) end - it 'returns top contributions and unique project contributions if querying for top_project_contributions' do - get :query, params: { id: classification_event.user_id, top_project_contributions: 10 } + it 'returns unique project contributions count if querying for project_contributions' do + get :query, params: { id: classification_event.user_id, project_contributions: true } expect(response.status).to eq(200) response_body = JSON.parse(response.body) expect(response_body['unique_project_contributions']).to eq(1) - expect(response_body['top_project_contributions'].length).to eq(1) - expect(response_body['top_project_contributions'][0]['project_id']).to eq(classification_event.project_id) + expect(response_body['project_contributions'].length).to eq(1) + expect(response_body['project_contributions'][0]['project_id']).to eq(classification_event.project_id) end end @@ -97,21 +97,26 @@ context 'param validations' do it_behaves_like 'ensure valid query params', :query, id: 1 - it 'ensures you cannot query by workflow and top_project_contributions' do - get :query, params: { id: 1, top_project_contributions: 10, workflow_id: 1 } + it 'ensures you cannot query by workflow and project_contributions' do + get :query, params: { id: 1, project_contributions: true, workflow_id: 1 } expect(response.status).to eq(400) expect(response.body).to include('Cannot query top projects and query by project/workflow') end it 'ensures you cannot query by project and top project contributions' do - get :query, params: { id: 1, top_project_contributions: 10, project_id: 1 } + get :query, params: { id: 1, project_contributions: true, project_id: 1 } expect(response.status).to eq(400) expect(response.body).to include('Cannot query top projects and query by project/workflow') end - it 'ensures top_project_contributions is an integer' do - get :query, params: { id: 1, top_project_contributions: '20' } - expect(controller.params[:top_project_contributions]).to eq(20) + it 'ensures project_contributions is an boolean' do + get :query, params: { id: 1, project_contributions: '100' } + expect(controller.params[:project_contributions]).to eq(false) + end + + it 'ensures project_contributions is true if given string true' do + get :query, params: { id: 1, project_contributions: 'true' } + expect(controller.params[:project_contributions]).to eq(true) end it 'ensures time_spent is a boolean' do diff --git a/spec/queries/count_user_classifications_spec.rb b/spec/queries/count_user_classifications_spec.rb index 344ff69..84ad57f 100644 --- a/spec/queries/count_user_classifications_spec.rb +++ b/spec/queries/count_user_classifications_spec.rb @@ -6,7 +6,7 @@ let(:params) { {} } let(:count_user_classifications) { described_class.new(params) } describe 'relation' do - it 'returns DailyUserClassificationCount if not given workflow, project ids or top_project_contributions' do + it 'returns DailyUserClassificationCount if not given workflow, project ids or project_contributions' do expect(count_user_classifications.counts.model).to be UserClassificationCounts::DailyUserClassificationCount end @@ -20,8 +20,8 @@ expect(count_user_classifications.counts.model).to be UserClassificationCounts::DailyUserProjectClassificationCount end - it 'returns DailyUserProjectClassificationCount if querying top_project_contributions' do - params[:top_project_contributions] = 2 + it 'returns DailyUserProjectClassificationCount if querying project_contributions' do + params[:project_contributions] = true expect(count_user_classifications.counts.model).to be UserClassificationCounts::DailyUserProjectClassificationCount end end @@ -47,8 +47,8 @@ expect(counts.to_sql).to eq(expected_select_query) end - it 'queries for project_id if querying for top_project_contributions' do - params[:top_project_contributions] = 10 + it 'queries for project_id if querying for project_contributions' do + params[:project_contributions] = true counts = count_user_classifications.call(params) expected_select_query = "SELECT time_bucket('1 year', day) AS period, SUM(classification_count)::integer AS count, project_id FROM \"daily_user_classification_count_and_time_per_project\" GROUP BY period, project_id ORDER BY period" expect(counts.to_sql).to eq(expected_select_query) diff --git a/spec/serializers/user_classification_counts_serializer_spec.rb b/spec/serializers/user_classification_counts_serializer_spec.rb index 9fe72ec..ddf9294 100644 --- a/spec/serializers/user_classification_counts_serializer_spec.rb +++ b/spec/serializers/user_classification_counts_serializer_spec.rb @@ -12,7 +12,7 @@ expect(serialized).not_to have_key(:data) expect(serialized).not_to have_key(:time_spent) expect(serialized).not_to have_key(:unique_project_contributions) - expect(serialized).not_to have_key(:top_project_contributions) + expect(serialized).not_to have_key(:project_contributions) expect(serialized[:total_count]).to eq(user_classification_count.count) end @@ -46,45 +46,38 @@ expect(serialized[:time_spent]).to eq(classification_counts.sum(&:session_time)) end - it 'returns unique_project_contributions and top_project_contributions if top_project_contributions' do - serialized = count_serializer.as_json(serializer_options: { top_project_contributions: 10 }) + it 'returns unique_project_contributions count and project_contributions if project_contributions' do + serialized = count_serializer.as_json(serializer_options: { project_contributions: true }) expect(serialized).to have_key(:total_count) expect(serialized).to have_key(:unique_project_contributions) - expect(serialized).to have_key(:top_project_contributions) + expect(serialized).to have_key(:project_contributions) expect(serialized[:unique_project_contributions]).to eq(1) - expect(serialized[:top_project_contributions].length).to eq(1) + expect(serialized[:project_contributions].length).to eq(1) expected_project_contributions = { project_id: user_classification_count.project_id, count: user_classification_count.count } - expect(serialized[:top_project_contributions][0]).to eq(expected_project_contributions) + expect(serialized[:project_contributions][0]).to eq(expected_project_contributions) end - context 'top_project_contributions param calculations' do + context 'project_contributions param calculations' do let(:user_diff_proj_count) { build(:user_diff_proj_classification_count) } let(:user_diff_period_classification_count) { build(:user_diff_period_classification_count) } let(:serializer) { described_class.new([user_diff_period_classification_count, user_classification_count, user_diff_proj_count]) } it 'correctly counts unique_projects' do - serialized = serializer.as_json(serializer_options: { top_project_contributions: 10 }) + serialized = serializer.as_json(serializer_options: { project_contributions: true }) expect(serialized[:unique_project_contributions]).to eq(2) end - it 'shows top_project_contributions ordered desc by count' do - serialized = serializer.as_json(serializer_options: { top_project_contributions: 10 }) - expect(serialized[:top_project_contributions].length).to eq(2) - expect(serialized[:top_project_contributions][0][:project_id]).to eq(user_classification_count.project_id) - expect(serialized[:top_project_contributions][0][:count]).to eq(user_classification_count.count + user_diff_period_classification_count.count) - expect(serialized[:top_project_contributions][1][:project_id]).to eq(user_diff_proj_count.project_id) - expect(serialized[:top_project_contributions][1][:count]).to eq(user_diff_proj_count.count) - end - - it 'shows the first N top_poject_contributions' do - serialized = serializer.as_json(serializer_options: { top_project_contributions: 1 }) - expect(serialized[:top_project_contributions].length).to eq(1) - expect(serialized[:top_project_contributions][0][:project_id]).to eq(user_classification_count.project_id) - expect(serialized[:top_project_contributions][0][:count]).to eq(user_classification_count.count + user_diff_period_classification_count.count) + it 'shows project_contributions ordered desc by count' do + serialized = serializer.as_json(serializer_options: { project_contributions: true }) + expect(serialized[:project_contributions].length).to eq(2) + expect(serialized[:project_contributions][0][:project_id]).to eq(user_classification_count.project_id) + expect(serialized[:project_contributions][0][:count]).to eq(user_classification_count.count + user_diff_period_classification_count.count) + expect(serialized[:project_contributions][1][:project_id]).to eq(user_diff_proj_count.project_id) + expect(serialized[:project_contributions][1][:count]).to eq(user_diff_proj_count.count) end it 'shows response data bucketed by period when querying top_projects' do - serialized = serializer.as_json(serializer_options: { top_project_contributions: 10, period: 'day' }) + serialized = serializer.as_json(serializer_options: { project_contributions: true, period: 'day' }) expect(serialized[:data].length).to eq(2) expect(serialized[:data][0][:period]).to eq(user_diff_period_classification_count.period) expect(serialized[:data][0][:count]).to eq(user_diff_period_classification_count.count) @@ -94,7 +87,7 @@ end it 'shows response data with session_times bucketed by period' do - serialized = serializer.as_json(serializer_options: { top_project_contributions: 10, period: 'day', time_spent: true }) + serialized = serializer.as_json(serializer_options: { project_contributions: true, period: 'day', time_spent: true }) expect(serialized[:data].length).to eq(2) expect(serialized[:data][0][:session_time]).to eq(user_diff_period_classification_count.session_time) expect(serialized[:data][1][:session_time]).to eq(user_classification_count.session_time + user_diff_proj_count.session_time) From 2d639562246dd82c8f29b4c92152354f971312ff Mon Sep 17 00:00:00 2001 From: yuenmichelle1 Date: Thu, 27 Jul 2023 16:42:32 -0500 Subject: [PATCH 2/3] Removing the attribute unique_project_contributions count, since one can just get the length of the array of project_contributions --- .../user_classification_counts_serializer.rb | 11 +---------- .../user_classification_count_controller_spec.rb | 1 - .../user_classification_counts_serializer_spec.rb | 10 +--------- 3 files changed, 2 insertions(+), 20 deletions(-) diff --git a/app/serializers/user_classification_counts_serializer.rb b/app/serializers/user_classification_counts_serializer.rb index 43107cb..a4ec43d 100644 --- a/app/serializers/user_classification_counts_serializer.rb +++ b/app/serializers/user_classification_counts_serializer.rb @@ -14,7 +14,7 @@ def as_json(options) total_count = user_classification_counts.sum(&:count).to_i response = { total_count: } calculate_time_spent(user_classification_counts, response) if show_time_spent - display_project_contributions(response) if show_project_contributions + response[:project_contributions] = project_contributions if show_project_contributions response[:data] = response_data(user_classification_counts, show_project_contributions:, show_time_spent:) if serializer_options[:period] response end @@ -42,15 +42,6 @@ def response_data(user_counts, show_project_contributions:, show_time_spent:) end end - def display_project_contributions(response) - response[:unique_project_contributions] = unique_projects_count - response[:project_contributions] = project_contributions - end - - def unique_projects_count - @user_classification_counts.map(&:project_id).uniq.count - end - def project_contributions project_contributions = @user_classification_counts.group_by(&:project_id).transform_values do |counts| counts.sum(&:count) diff --git a/spec/controllers/user_classification_count_controller_spec.rb b/spec/controllers/user_classification_count_controller_spec.rb index b9c1da8..aafd33d 100644 --- a/spec/controllers/user_classification_count_controller_spec.rb +++ b/spec/controllers/user_classification_count_controller_spec.rb @@ -44,7 +44,6 @@ get :query, params: { id: classification_event.user_id, project_contributions: true } expect(response.status).to eq(200) response_body = JSON.parse(response.body) - expect(response_body['unique_project_contributions']).to eq(1) expect(response_body['project_contributions'].length).to eq(1) expect(response_body['project_contributions'][0]['project_id']).to eq(classification_event.project_id) end diff --git a/spec/serializers/user_classification_counts_serializer_spec.rb b/spec/serializers/user_classification_counts_serializer_spec.rb index ddf9294..a3fce9a 100644 --- a/spec/serializers/user_classification_counts_serializer_spec.rb +++ b/spec/serializers/user_classification_counts_serializer_spec.rb @@ -11,7 +11,6 @@ expect(serialized).to have_key(:total_count) expect(serialized).not_to have_key(:data) expect(serialized).not_to have_key(:time_spent) - expect(serialized).not_to have_key(:unique_project_contributions) expect(serialized).not_to have_key(:project_contributions) expect(serialized[:total_count]).to eq(user_classification_count.count) end @@ -46,12 +45,10 @@ expect(serialized[:time_spent]).to eq(classification_counts.sum(&:session_time)) end - it 'returns unique_project_contributions count and project_contributions if project_contributions' do + it 'returns project_contributions if project_contributions' do serialized = count_serializer.as_json(serializer_options: { project_contributions: true }) expect(serialized).to have_key(:total_count) - expect(serialized).to have_key(:unique_project_contributions) expect(serialized).to have_key(:project_contributions) - expect(serialized[:unique_project_contributions]).to eq(1) expect(serialized[:project_contributions].length).to eq(1) expected_project_contributions = { project_id: user_classification_count.project_id, count: user_classification_count.count } expect(serialized[:project_contributions][0]).to eq(expected_project_contributions) @@ -62,11 +59,6 @@ let(:user_diff_period_classification_count) { build(:user_diff_period_classification_count) } let(:serializer) { described_class.new([user_diff_period_classification_count, user_classification_count, user_diff_proj_count]) } - it 'correctly counts unique_projects' do - serialized = serializer.as_json(serializer_options: { project_contributions: true }) - expect(serialized[:unique_project_contributions]).to eq(2) - end - it 'shows project_contributions ordered desc by count' do serialized = serializer.as_json(serializer_options: { project_contributions: true }) expect(serialized[:project_contributions].length).to eq(2) From 8656864320e663a05d3b967c0323c6927ef03f0d Mon Sep 17 00:00:00 2001 From: yuenmichelle1 Date: Thu, 27 Jul 2023 16:43:24 -0500 Subject: [PATCH 3/3] remove extra spacing per hound sniff --- app/controllers/user_classification_count_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/user_classification_count_controller.rb b/app/controllers/user_classification_count_controller.rb index 4cf9a76..3a08926 100644 --- a/app/controllers/user_classification_count_controller.rb +++ b/app/controllers/user_classification_count_controller.rb @@ -20,7 +20,7 @@ def validate_params end def sanitize_params - params[:project_contributions] = params[:project_contributions].casecmp?('true') if params[:project_contributions] + params[:project_contributions] = params[:project_contributions].casecmp?('true') if params[:project_contributions] params[:time_spent] = params[:time_spent].casecmp?('true') if params[:time_spent] end