From 48108eefc89ede44ec2ba1b56bbdf904c8c3fc15 Mon Sep 17 00:00:00 2001 From: Kostas Sklias <> Date: Tue, 12 Dec 2023 16:21:09 +0200 Subject: [PATCH] DRYs up activities controller --- app/controllers/activities_controller.rb | 24 ++++- app/models/activity.rb | 9 +- app/views/activities/_filters.html.erb | 2 +- .../activity_pages/activity_pages_spec.rb | 6 +- spec/models/activity_spec.rb | 88 +++++++++---------- 5 files changed, 71 insertions(+), 58 deletions(-) diff --git a/app/controllers/activities_controller.rb b/app/controllers/activities_controller.rb index 903260bfa6..8f016b6b7d 100644 --- a/app/controllers/activities_controller.rb +++ b/app/controllers/activities_controller.rb @@ -8,9 +8,7 @@ def index @page = params[:page].present? ? params[:page].to_i : 1 activities = current_project.activities.includes(:trackable) - activities = activities.filter_by_user_id(params[:user]) if params[:user].present? - activities = activities.filter_by_type(params[:type]) if params[:type].present? - activities = activities.filter_by_date(period_start, period_end) if params[:period_start].present? + activities = filter_activities(activities) activities = activities.order(created_at: :desc).page(@page) @activities_groups = activities.group_by do |activity| @@ -45,4 +43,24 @@ def period_end DateTime.parse params[:period_end] end end + + def filtering_params + params.slice(:user, :type, :period_start, :period_end) + end + + def filter_activities(activities) + filtering_params.each do |k, value| + next if value.blank? + + if k == 'period_start' + value = DateTime.parse(value).beginning_of_day + elsif k == 'period_end' + value = DateTime.parse(value).end_of_day + end + + activities = activities.send("by_#{k}", value) + end + + activities + end end diff --git a/app/models/activity.rb b/app/models/activity.rb index 7f5666ef7c..3577c39076 100644 --- a/app/models/activity.rb +++ b/app/models/activity.rb @@ -26,11 +26,10 @@ def project=(new_project); end scope :latest, -> do includes(:trackable).order('activities.created_at DESC').limit(20) end - scope :filter_by_user_id, -> (id) { where user_id: id } - scope :filter_by_type, -> (type) { where trackable_type: type } - scope :filter_by_date, -> (period_start, period_end) { - where('created_at >= ? AND created_at <= ?', period_start.beginning_of_day, period_end.end_of_day) - } + scope :by_user, -> (id) { where user_id: id } + scope :by_type, -> (type) { where trackable_type: type } + scope :by_period_start, -> (period_start) { where('created_at >= ?', period_start) } + scope :by_period_end, -> (period_end) { where('created_at <= ?', period_end) } # -- Callbacks ------------------------------------------------------------ diff --git a/app/views/activities/_filters.html.erb b/app/views/activities/_filters.html.erb index 916db94faf..d58d41b791 100644 --- a/app/views/activities/_filters.html.erb +++ b/app/views/activities/_filters.html.erb @@ -1,4 +1,4 @@ -<%= form_with url: project_activities_path(current_project.id), method: :get, remote: true, class: 'd-flex flex-xl-row flex-column gap-3', data: { behavior: 'activity-filters' } do |f| %> +<%= form_with url: project_activities_path(current_project), method: :get, remote: true, class: 'd-flex flex-xl-row flex-column gap-3', data: { behavior: 'activity-filters' } do |f| %>
<%= f.label :user, class: 'form-label' %> diff --git a/spec/features/activity_pages/activity_pages_spec.rb b/spec/features/activity_pages/activity_pages_spec.rb index 47902bf488..b76a5fe6d1 100644 --- a/spec/features/activity_pages/activity_pages_spec.rb +++ b/spec/features/activity_pages/activity_pages_spec.rb @@ -105,7 +105,7 @@ expect(page).to have_selector('#user', count: 1) end - it 'user filter works' do + it 'by user' do visit project_activities_path(current_project, user: second_user.id) expect(page).to have_selector('.activity', count: 10) end @@ -114,7 +114,7 @@ expect(page).to have_selector('#type', count: 1) end - it 'type filter works' do + it 'by type' do visit project_activities_path(current_project, type: 'Card') expect(page).to have_selector('.activity', count: 15) end @@ -124,7 +124,7 @@ expect(page).to have_selector('#period_end', count: 1) end - it 'daterange filter works' do + it 'by date' do period_start = Time.current - 3.days period_end = Time.current diff --git a/spec/models/activity_spec.rb b/spec/models/activity_spec.rb index 1d99cbe154..2144070a62 100644 --- a/spec/models/activity_spec.rb +++ b/spec/models/activity_spec.rb @@ -25,71 +25,67 @@ end end - describe '#filter_by_user_id' do + describe "filtering" do before do + card = create(:card) issue = create(:issue) @user = create(:user) - activity = create(:activity, trackable: issue, user: @user) - activity2 = create(:activity, trackable: issue, user: @user) - activity3 = create(:activity, trackable: issue) + activity = create(:activity, trackable: issue, user: @user, created_at: DateTime.now.beginning_of_year) + activity2 = create(:activity, trackable: card, user: @user, created_at: DateTime.now.beginning_of_year) + activity3 = create(:activity, trackable: card, created_at: DateTime.now.beginning_of_year - 1.year) end - context 'when passed a valid user id' do - it 'returns activities of this user' do - expect(Activity.filter_by_user_id(@user.id).count).to eq 2 - expect(Activity.all.count).to eq 3 + describe '#by_user' do + context 'when passed a valid user id' do + it 'returns activities of this user' do + expect(Activity.by_user(@user.id).count).to eq 2 + expect(Activity.all.count).to eq 3 + end end - end - context 'when passed a non valid user id' do - it 'returns an empty collection' do - user_id = User.last.id + 1 - expect(Activity.filter_by_user_id(user_id).count).to eq 0 - expect(Activity.all.count).to eq 3 + context 'when passed a non valid user id' do + it 'returns an empty collection' do + user_id = User.last.id + 1 + expect(Activity.by_user(user_id).count).to eq 0 + expect(Activity.all.count).to eq 3 + end end end - end - describe '#filter_by_type' do - before do - issue = create(:issue) - card = create(:card) - activity = create(:activity, trackable: issue) - activity2 = create(:activity, trackable: card) - activity3 = create(:activity, trackable: card) - end - context 'when passed a valid type' do - it 'returns activities of this user' do - expect(Activity.filter_by_type('Card').count).to eq 2 - expect(Activity.all.count).to eq 3 + describe '#by_type' do + context 'when passed a valid type' do + it 'returns activities of this user' do + expect(Activity.by_type('Card').count).to eq 2 + expect(Activity.all.count).to eq 3 + end end - end - context 'when passed a non valid type' do - it 'returns an empty collection' do - expect(Activity.filter_by_type('galaxy').count).to eq 0 - expect(Activity.all.count).to eq 3 + context 'when passed a non valid type' do + it 'returns an empty collection' do + expect(Activity.by_type('galaxy').count).to eq 0 + expect(Activity.all.count).to eq 3 + end end end - end - describe '#filter_by_date' do - before do - issue = create(:issue) - activity = create(:activity, trackable: issue) - activity2 = create(:activity, trackable: issue, created_at: DateTime.now.beginning_of_year - 1.year) - activity3 = create(:activity, trackable: issue, created_at: DateTime.now.beginning_of_year - 1.year) + describe '#by_period_start' do + context 'when passed a valid date' do + it 'returns activities within the date' do + year_start = DateTime.now.beginning_of_year + + expect(Activity.by_period_start(year_start).count).to eq 2 + end + end end - context 'when passed a valid date' do - it 'returns activities within the date' do - year_start = DateTime.now.beginning_of_year - last_year_start = year_start - 1.year - period_end = DateTime.now.end_of_day + describe '#by_period_end' do + context 'when passed a valid date' do + it 'returns activities within the date' do + period_end = DateTime.now.end_of_day - 1.year - expect(Activity.filter_by_date(year_start, period_end).count).to eq 1 - expect(Activity.filter_by_date(last_year_start, period_end).count).to eq 3 + expect(Activity.by_period_end(period_end).count).to eq 1 + end end end end