From 1d3881b6491250cf1fcdb17ee5960941a6e76462 Mon Sep 17 00:00:00 2001 From: zmagod Date: Wed, 3 Jan 2018 13:48:59 +0100 Subject: [PATCH] fixes bug with duplicated activities and wrong date separators [fixes SCI-1866] --- app/assets/javascripts/navigation.js | 8 +-- app/controllers/activities_controller.rb | 58 +++++++------------ .../client_api/activities_controller.rb | 2 +- .../components/GlobalActivitiesModal.jsx | 11 +++- app/views/activities/_index.html.erb | 10 ++-- app/views/activities/_list.html.erb | 13 ++--- spec/factories/user_project.rb | 5 ++ spec/models/user_spec.rb | 15 +++++ 8 files changed, 65 insertions(+), 57 deletions(-) create mode 100644 spec/factories/user_project.rb diff --git a/app/assets/javascripts/navigation.js b/app/assets/javascripts/navigation.js index 4b19db7cc..4e731df63 100644 --- a/app/assets/javascripts/navigation.js +++ b/app/assets/javascripts/navigation.js @@ -109,14 +109,14 @@ // Activity feed modal in main navigation menu var activityModal = $('#activity-modal'); var activityModalBody = activityModal.find('.modal-body'); - var initMoreBtn = function() { activityModalBody.find('.btn-more-activities') .on('ajax:success', function(e, data) { $(data.html).insertBefore($(this).parents('li')); - $(this).attr('href', data.next_url); - if (data.activities_number < data.per_page) { - $(this).remove(); + if(data.more_url) { + $(this).attr('href', data.more_url); + } else { + $(this).remove(); } }); }; diff --git a/app/controllers/activities_controller.rb b/app/controllers/activities_controller.rb index 29629df31..7f2db81a0 100644 --- a/app/controllers/activities_controller.rb +++ b/app/controllers/activities_controller.rb @@ -1,47 +1,33 @@ class ActivitiesController < ApplicationController include ActivityHelper - before_action :load_vars - def index - @per_page = Constants::ACTIVITY_AND_NOTIF_SEARCH_LIMIT - @activities = current_user.last_activities(@last_activity_id, - @per_page + 1) - - @overflown = @activities.length > @per_page - - @activities = current_user.last_activities(@last_activity_id, - @per_page) - - # Whether to hide date labels - @hide_today = params.include? :from - @day = @last_activity.present? ? - days_since_1970(@last_activity.created_at) : - days_since_1970(DateTime.current + 30.days) - - more_url = url_for(activities_url(format: :json, - from: @activities.last.id)) if @overflown respond_to do |format| - format.json { - render :json => { - per_page: @per_page, - activities_number: @activities.length, - next_url: more_url, - html: render_to_string({ - partial: 'index.html.erb', - locals: { - more_activities_url: more_url, - hide_today: @hide_today, - day: @day - } - }) + format.json do + render json: { + more_url: local_vars.fetch(:more_activities_url), + html: render_to_string( + partial: 'index.html.erb', locals: local_vars + ) } - } + end end end - def load_vars - @last_activity_id = params[:from].to_i || 0 - @last_activity = Activity.find_by_id(@last_activity_id) + private + + def local_vars + page = (params[:page] || 1).to_i + activities = current_user.last_activities + .page(page) + .per(Constants::ACTIVITY_AND_NOTIF_SEARCH_LIMIT) + unless activities.last_page? + more_url = url_for(activities_url(format: :json, page: page + 1)) + end + { + activities: activities, + more_activities_url: more_url, + page: page + } end end diff --git a/app/controllers/client_api/activities_controller.rb b/app/controllers/client_api/activities_controller.rb index d83bfd9b5..a92caa3a6 100644 --- a/app/controllers/client_api/activities_controller.rb +++ b/app/controllers/client_api/activities_controller.rb @@ -22,7 +22,7 @@ module ClientApi { activities: activities, page: page, - more: !current_user.last_activities.page(page).last_page?, + more: !activities.last_page?, timezone: current_user.time_zone } end diff --git a/app/javascript/src/components/Navigation/components/GlobalActivitiesModal.jsx b/app/javascript/src/components/Navigation/components/GlobalActivitiesModal.jsx index f56805eb9..999d863de 100644 --- a/app/javascript/src/components/Navigation/components/GlobalActivitiesModal.jsx +++ b/app/javascript/src/components/Navigation/components/GlobalActivitiesModal.jsx @@ -99,7 +99,7 @@ class GlobalActivitiesModal extends Component { // when the backend bug will be fixed const newDate = new Date(activity.createdAt); // returns a label with "today" if the date of the activity is today - if (i === 0) { + if (i === 0 && newDate.toDateString() === new Date().toDateString()) { return GlobalActivitiesModal.renderActivityDateElement( i, activity, @@ -108,8 +108,13 @@ class GlobalActivitiesModal extends Component { } // else checks if the previous activity is newer than current // and displays a label with the date - const prevDate = new Date(arr[i - 1].createdAt); - if (prevDate.getDate() > newDate.getDate()) { + const prevDate = + i !== 0 ? new Date(arr[i - 1].createdAt) : new Date(1901, 1, 1); + // filter only date from createdAt without minutes and seconds + // used to compare dates + const parsePrevDate = new Date(prevDate.toDateString()); + const parseNewDate = new Date(newDate.toDateString()); + if (parsePrevDate.getTime() > parseNewDate.getTime()) { return GlobalActivitiesModal.renderActivityDateElement( i, activity, diff --git a/app/views/activities/_index.html.erb b/app/views/activities/_index.html.erb index 2c7eeac6b..2ec016b50 100644 --- a/app/views/activities/_index.html.erb +++ b/app/views/activities/_index.html.erb @@ -1,12 +1,14 @@
    - <% if @activities.length == 0 then %> + <% if activities.empty? %>
  • <%= t'activities.index.no_activities' %>
    • <% else %> - <%= render 'activities/list.html.erb', activities: @activities, hide_today: hide_today, day: @day %> + <%= render 'activities/list.html.erb', activities: activities%> <% end %> - <% if @last_activity_id < 1 and @overflown %> + <% if more_activities_url.present? && page == 1 %>
  • - + <%= t'activities.index.more_activities' %>
    • <% end %> diff --git a/app/views/activities/_list.html.erb b/app/views/activities/_list.html.erb index 5f633b587..151cf2e41 100644 --- a/app/views/activities/_list.html.erb +++ b/app/views/activities/_list.html.erb @@ -1,18 +1,13 @@ -<% - current_day = days_since_1970(DateTime.current) -%> -<% if !hide_today && activities.count > 0 && days_since_1970(activities[0].created_at) == current_day %> +<% if activities.first.created_at.to_date == Date.today %>
  • <%=t "activities.index.today" %>
    • <% end %> -<% activities.each do |activity| %> - <% activity_day = days_since_1970(activity.created_at) %> - - <% if activity_day < current_day and activity_day < day %> - <% day = days_since_1970(activity.created_at) %> +<% activities.each_with_index do |activity, index| %> + <% prevDate = activities[index - 1] ? activities[index - 1].created_at.to_date : Date.new(1901, 1, 1) %> + <% if activity.created_at.to_date < prevDate %>
  • <%= activity.created_at.strftime('%d.%m.%Y') %> diff --git a/spec/factories/user_project.rb b/spec/factories/user_project.rb new file mode 100644 index 000000000..24a2c6a34 --- /dev/null +++ b/spec/factories/user_project.rb @@ -0,0 +1,5 @@ +FactoryGirl.define do + factory :user_project do + role 'owner' + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 0d254c439..dd25cb334 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -179,4 +179,19 @@ describe User, type: :model do it { is_expected.to respond_to(:recent_email_notification) } it { is_expected.to respond_to(:system_message_email_notification) } end + + describe '#last_activities' do + let!(:user) { create :user } + let!(:project) { create :project } + let!(:user_projects) { create :user_project, project: project, user: user } + let!(:activity_one) { create :activity, user: user, project: project } + let!(:activity_two) { create :activity, user: user, project: project } + + it 'is expected to return an array of user\'s activities' do + activities = user.last_activities + expect(activities.count).to eq 2 + expect(activities).to include activity_one + expect(activities).to include activity_two + end + end end