From 101b3b68d3ba3e1ac67b5cb4c0bd4dcc2b49531f Mon Sep 17 00:00:00 2001 From: Georg Gadinger Date: Mon, 20 Apr 2020 23:03:57 +0200 Subject: [PATCH] Use cursored pagination, remove WillPaginate --- Gemfile | 2 - Gemfile.lock | 5 - app/controllers/group_controller.rb | 9 +- app/controllers/inbox_controller.rb | 31 ++-- app/controllers/notifications_controller.rb | 26 ++- app/controllers/public_controller.rb | 5 +- app/controllers/question_controller.rb | 5 +- app/controllers/static_controller.rb | 7 +- app/controllers/user_controller.rb | 16 +- app/models/answer.rb | 2 + app/models/answer/timeline_methods.rb | 14 ++ app/models/concerns/.keep | 0 app/models/concerns/cursor_paginatable.rb | 38 +++++ app/models/group.rb | 7 +- app/models/group/timeline_methods.rb | 12 ++ app/models/notification.rb | 9 + app/models/question.rb | 2 + app/models/question/answer_methods.rb | 13 ++ app/models/user.rb | 11 +- app/models/user/answer_methods.rb | 13 ++ app/models/user/inbox_methods.rb | 14 ++ app/models/user/question_methods.rb | 13 ++ app/models/user/relationship_methods.rb | 16 ++ app/models/user/timeline_methods.rb | 12 ++ app/views/group/index.html.haml | 6 +- app/views/group/index.js.erb | 6 +- app/views/inbox/show.html.haml | 6 +- app/views/inbox/show.js.erb | 6 +- app/views/notifications/index.html.haml | 6 +- app/views/notifications/index.js.erb | 6 +- app/views/public/index.html.haml | 6 +- app/views/public/index.js.erb | 6 +- app/views/question/show.html.haml | 6 +- app/views/question/show.js.erb | 6 +- .../shared/_cursored_pagination_dummy.haml | 11 ++ app/views/static/index.html.haml | 8 +- app/views/static/index.js.erb | 6 +- app/views/user/questions.html.haml | 6 +- app/views/user/questions.js.erb | 4 +- app/views/user/show.html.haml | 6 +- app/views/user/show.js.erb | 6 +- app/views/user/show_follow.html.haml | 6 +- app/views/user/show_follow.js.erb | 4 +- config/initializers/20_will_paginate.rb | 1 - config/initializers/rails_admin.rb | 6 - spec/factories/answer.rb | 12 ++ spec/factories/question.rb | 9 + spec/factories/{users.rb => user.rb} | 4 +- spec/models/user_spec.rb | 158 ++++++++++++++++-- 49 files changed, 470 insertions(+), 119 deletions(-) create mode 100644 app/models/answer/timeline_methods.rb delete mode 100644 app/models/concerns/.keep create mode 100644 app/models/concerns/cursor_paginatable.rb create mode 100644 app/models/group/timeline_methods.rb create mode 100644 app/models/question/answer_methods.rb create mode 100644 app/models/user/answer_methods.rb create mode 100644 app/models/user/inbox_methods.rb create mode 100644 app/models/user/question_methods.rb create mode 100644 app/models/user/relationship_methods.rb create mode 100644 app/models/user/timeline_methods.rb create mode 100644 app/views/shared/_cursored_pagination_dummy.haml delete mode 100644 config/initializers/20_will_paginate.rb create mode 100644 spec/factories/answer.rb create mode 100644 spec/factories/question.rb rename spec/factories/{users.rb => user.rb} (86%) diff --git a/Gemfile b/Gemfile index 4f148a2f..fc7ad28b 100644 --- a/Gemfile +++ b/Gemfile @@ -22,8 +22,6 @@ gem 'haml', '~> 5.0' gem 'bootstrap-sass', '~> 3.4.0' gem 'bootswatch-rails' gem 'sweetalert-rails' -gem 'will_paginate' -gem 'will_paginate-bootstrap' gem 'devise', '~> 4.0' gem 'devise-i18n' gem 'devise-async' diff --git a/Gemfile.lock b/Gemfile.lock index 3c142622..4ab36387 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -499,9 +499,6 @@ GEM websocket-driver (0.7.1) websocket-extensions (>= 0.1.0) websocket-extensions (0.1.4) - will_paginate (3.3.0) - will_paginate-bootstrap (1.0.2) - will_paginate (>= 3.0.3) xpath (3.2.0) nokogiri (~> 1.8) @@ -584,8 +581,6 @@ DEPENDENCIES uglifier (>= 1.3.0) unicorn web-console (< 4.0.0) - will_paginate - will_paginate-bootstrap BUNDLED WITH 2.1.4 diff --git a/app/controllers/group_controller.rb b/app/controllers/group_controller.rb index 7fc45a42..95f772d6 100644 --- a/app/controllers/group_controller.rb +++ b/app/controllers/group_controller.rb @@ -3,6 +3,13 @@ class GroupController < ApplicationController def index @group = current_user.groups.find_by_name!(params[:group_name]) - @timeline = @group.timeline.paginate(page: params[:page]) + @timeline = @group.cursored_timeline(last_id: params[:last_id]) + @timeline_last_id = @timeline.map(&:id).min + @more_data_available = !@group.cursored_timeline(last_id: @timeline_last_id, size: 1).count.zero? + + respond_to do |format| + format.html + format.js + end end end diff --git a/app/controllers/inbox_controller.rb b/app/controllers/inbox_controller.rb index 5b40b455..24849f32 100644 --- a/app/controllers/inbox_controller.rb +++ b/app/controllers/inbox_controller.rb @@ -2,28 +2,37 @@ class InboxController < ApplicationController before_action :authenticate_user! def show - @inbox = Inbox.where(user: current_user) - .order(:created_at).reverse_order - .paginate(page: params[:page]) - @inbox_count = Inbox.where(user: current_user).count + @inbox = current_user.cursored_inbox(last_id: params[:last_id]) + @inbox_last_id = @inbox.map(&:id).min + @more_data_available = !current_user.cursored_inbox(last_id: @inbox_last_id, size: 1).count.zero? + @inbox_count = current_user.inboxes.count + if params[:author].present? begin @author = true @target_user = User.where('LOWER(screen_name) = ?', params[:author].downcase).first! - @inbox_author = current_user.inboxes.joins(:question) - .where(questions: { user_id: @target_user.id, author_is_anonymous: false }) - .paginate(page: params[:page]) - @inbox_author_count = current_user.inboxes.joins(:question) - .where(questions: { user_id: @target_user.id, author_is_anonymous: false }) - .count + @inbox_author = @inbox.joins(:question) + .where(questions: { user_id: @target_user.id, author_is_anonymous: false }) + @inbox_author_count = current_user.inboxes + .joins(:question) + .where(questions: { user_id: @target_user.id, author_is_anonymous: false }) + .count + if @inbox_author.empty? @empty = true flash.now[:info] = "No questions from @#{params[:author]} found, showing default entries instead!" else @inbox = @inbox_author @inbox_count = @inbox_author_count + @inbox_last_id = @inbox.map(&:id).min + @more_data_available = !current_user.cursored_inbox(last_id: @inbox_last_id, size: 1) + .joins(:question) + .where(questions: { user_id: @target_user.id, author_is_anonymous: false }) + .count + .zero? end - rescue + rescue => e + NewRelic::Agent.notice_error(e) flash.now[:error] = "No user with the name @#{params[:author]} found, showing default entries instead!" @not_found = true end diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb index 8e824ebc..006ba5e8 100644 --- a/app/controllers/notifications_controller.rb +++ b/app/controllers/notifications_controller.rb @@ -3,16 +3,28 @@ class NotificationsController < ApplicationController def index @type = params[:type] - @notifications = if @type == 'all' - Notification.for(current_user) - elsif @type == 'new' - Notification.for(current_user).where(new: true) - else - Notification.for(current_user).where('LOWER(target_type) = ?', @type) - end.paginate(page: params[:page]) + @notifications = cursored_notifications_for(type: @type, last_id: params[:last_id]) + @notifications_last_id = @notifications.map(&:id).min + @more_data_available = !cursored_notifications_for(type: @type, last_id: @notifications_last_id, size: 1).count.zero? + respond_to do |format| format.html format.js end end + + private + + def cursored_notifications_for(type:, last_id:, size: nil) + cursor_params = { last_id: last_id, size: size }.compact + + case type + when 'all' + Notification.cursored_for(current_user, **cursor_params) + when 'new' + Notification.cursored_for(current_user, new: true, **cursor_params) + else + Notification.cursored_for_type(current_user, type, **cursor_params) + end + end end diff --git a/app/controllers/public_controller.rb b/app/controllers/public_controller.rb index d5b8af94..9a7b1d97 100644 --- a/app/controllers/public_controller.rb +++ b/app/controllers/public_controller.rb @@ -2,7 +2,10 @@ class PublicController < ApplicationController before_action :authenticate_user! def index - @timeline = Answer.joins(:user).where(users: { privacy_allow_public_timeline: true }).all.reverse_order.paginate(page: params[:page]) + @timeline = Answer.cursored_public_timeline(last_id: params[:last_id]) + @timeline_last_id = @timeline.map(&:id).min + @more_data_available = !Answer.cursored_public_timeline(last_id: @timeline_last_id, size: 1).count.zero? + respond_to do |format| format.html format.js diff --git a/app/controllers/question_controller.rb b/app/controllers/question_controller.rb index 54fac76c..f698bdca 100644 --- a/app/controllers/question_controller.rb +++ b/app/controllers/question_controller.rb @@ -1,7 +1,10 @@ class QuestionController < ApplicationController def show @question = Question.find(params[:id]) - @answers = @question.answers.reverse_order.paginate(page: params[:page]) + @answers = @question.cursored_answers(last_id: params[:last_id]) + @answers_last_id = @answers.map(&:id).min + @more_data_available = !@question.cursored_answers(last_id: @answers_last_id, size: 1).count.zero? + respond_to do |format| format.html format.js diff --git a/app/controllers/static_controller.rb b/app/controllers/static_controller.rb index 1fd3bb62..449a5171 100644 --- a/app/controllers/static_controller.rb +++ b/app/controllers/static_controller.rb @@ -1,7 +1,12 @@ +# frozen_string_literal: true + class StaticController < ApplicationController def index if user_signed_in? - @timeline = current_user.timeline.paginate(page: params[:page]) + @timeline = current_user.cursored_timeline(last_id: params[:last_id]) + @timeline_last_id = @timeline.map(&:id).min + @more_data_available = !current_user.cursored_timeline(last_id: @timeline_last_id, size: 1).count.zero? + respond_to do |format| format.html format.js diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb index acebb416..a35971ed 100644 --- a/app/controllers/user_controller.rb +++ b/app/controllers/user_controller.rb @@ -5,7 +5,9 @@ class UserController < ApplicationController def show @user = User.where('LOWER(screen_name) = ?', params[:username].downcase).first! - @answers = @user.answers.reverse_order.paginate(page: params[:page]) + @answers = @user.cursored_answers(last_id: params[:last_id]) + @answers_last_id = @answers.map(&:id).min + @more_data_available = !@user.cursored_answers(last_id: @answers_last_id, size: 1).count.zero? if user_signed_in? notif = Notification.where(target_type: "Relationship", target_id: @user.active_relationships.where(target_id: current_user.id).pluck(:id), recipient_id: current_user.id, new: true).first @@ -72,7 +74,9 @@ class UserController < ApplicationController def followers @title = 'Followers' @user = User.where('LOWER(screen_name) = ?', params[:username].downcase).first! - @users = @user.followers.reverse_order.paginate(page: params[:page]) + @users = @user.cursored_followers(last_id: params[:last_id]) + @users_last_id = @users.map(&:id).min + @more_data_available = !@user.cursored_followers(last_id: @users_last_id, size: 1).count.zero? @type = :friend render 'show_follow' end @@ -80,7 +84,9 @@ class UserController < ApplicationController def friends @title = 'Following' @user = User.where('LOWER(screen_name) = ?', params[:username].downcase).first! - @users = @user.friends.reverse_order.paginate(page: params[:page]) + @users = @user.cursored_friends(last_id: params[:last_id]) + @users_last_id = @users.map(&:id).min + @more_data_available = !@user.cursored_friends(last_id: @users_last_id, size: 1).count.zero? @type = :friend render 'show_follow' end @@ -88,7 +94,9 @@ class UserController < ApplicationController def questions @title = 'Questions' @user = User.where('LOWER(screen_name) = ?', params[:username].downcase).first! - @questions = @user.questions.where(author_is_anonymous: false).reverse_order.paginate(page: params[:page]) + @questions = @user.cursored_questions(last_id: params[:last_id]) + @questions_last_id = @questions.map(&:id).min + @more_data_available = !@user.cursored_questions(last_id: @questions_last_id, size: 1).count.zero? end def data diff --git a/app/models/answer.rb b/app/models/answer.rb index 652e24d8..5be72352 100644 --- a/app/models/answer.rb +++ b/app/models/answer.rb @@ -1,4 +1,6 @@ class Answer < ApplicationRecord + extend Answer::TimelineMethods + belongs_to :user belongs_to :question has_many :comments, dependent: :destroy diff --git a/app/models/answer/timeline_methods.rb b/app/models/answer/timeline_methods.rb new file mode 100644 index 00000000..732ba2bd --- /dev/null +++ b/app/models/answer/timeline_methods.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +module Answer::TimelineMethods + include CursorPaginatable + + define_cursor_paginator :cursored_public_timeline, :public_timeline + + def public_timeline + joins(:user) + .where(users: { privacy_allow_public_timeline: true }) + .order(:created_at) + .reverse_order + end +end diff --git a/app/models/concerns/.keep b/app/models/concerns/.keep deleted file mode 100644 index e69de29b..00000000 diff --git a/app/models/concerns/cursor_paginatable.rb b/app/models/concerns/cursor_paginatable.rb new file mode 100644 index 00000000..826a172d --- /dev/null +++ b/app/models/concerns/cursor_paginatable.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module CursorPaginatable + extend ActiveSupport::Concern + + module ClassMethods + # Defines a cursor paginator. + # + # This method will define a new method +name+, which accepts the keyword + # arguments +last_id+ for defining the last id the cursor will operate on, + # and +size+ for the amount of records it should return. + # + # @param name [Symbol] The name of the method for the cursor paginator + # @param scope [Symbol] The name of the method which returns an + # ActiveRecord scope. + # + # @example + # class User + # has_many :answers + # + # include CursorPaginatable + # define_cursor_paginator :cursored_answers, :recent_answers + # + # def recent_answers + # self.answers.order(:created_at).reverse_order + # end + # end + def define_cursor_paginator(name, scope, default_size: APP_CONFIG.fetch('items_per_page')) + class_eval <<-RUBY, __FILE__, __LINE__ + 1 + def #{name}(*args, last_id: nil, size: #{default_size}, **kwargs) + s = self.#{scope}(*args, **kwargs).limit(size) + s = s.where(s.arel_table[:id].lt(last_id)) if last_id.present? + s + end + RUBY + end + end +end diff --git a/app/models/group.rb b/app/models/group.rb index 0f04893f..8a90b598 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class Group < ApplicationRecord + include Group::TimelineMethods + belongs_to :user has_many :group_members, dependent: :destroy @@ -22,9 +24,4 @@ class Group < ApplicationRecord def remove_member(user) GroupMember.where(group: self, user: user).first!.destroy end - - # @return [Array] the groups' timeline - def timeline - Answer.where("user_id in (?)", members.pluck(:user_id)).order(:created_at).reverse_order - end end diff --git a/app/models/group/timeline_methods.rb b/app/models/group/timeline_methods.rb new file mode 100644 index 00000000..cf42dcbc --- /dev/null +++ b/app/models/group/timeline_methods.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +module Group::TimelineMethods + include CursorPaginatable + + define_cursor_paginator :cursored_timeline, :timeline + + # @return [Array] the groups' timeline + def timeline + Answer.where('user_id in (?)', members.pluck(:user_id)).order(:created_at).reverse_order + end +end diff --git a/app/models/notification.rb b/app/models/notification.rb index 18d29bd2..97670ae2 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -3,10 +3,19 @@ class Notification < ApplicationRecord belongs_to :target, polymorphic: true class << self + include CursorPaginatable + + define_cursor_paginator :cursored_for, :for + define_cursor_paginator :cursored_for_type, :for_type + def for(recipient, options={}) self.where(options.merge!(recipient: recipient)).order(:created_at).reverse_order end + def for_type(recipient, type, options={}) + self.where(options.merge!(recipient: recipient)).where('LOWER(target_type) = ?', type).order(:created_at).reverse_order + end + def notify(recipient, target) return nil unless target.respond_to? :notification_type diff --git a/app/models/question.rb b/app/models/question.rb index 29146367..65daf030 100644 --- a/app/models/question.rb +++ b/app/models/question.rb @@ -1,4 +1,6 @@ class Question < ApplicationRecord + include Question::AnswerMethods + belongs_to :user has_many :answers, dependent: :destroy has_many :inboxes, dependent: :destroy diff --git a/app/models/question/answer_methods.rb b/app/models/question/answer_methods.rb new file mode 100644 index 00000000..2de7f3b2 --- /dev/null +++ b/app/models/question/answer_methods.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module Question::AnswerMethods + include CursorPaginatable + + define_cursor_paginator :cursored_answers, :ordered_answers + + def ordered_answers + answers + .order(:created_at) + .reverse_order + end +end diff --git a/app/models/user.rb b/app/models/user.rb index 119f8d81..033579c6 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,4 +1,10 @@ class User < ApplicationRecord + include User::AnswerMethods + include User::InboxMethods + include User::QuestionMethods + include User::RelationshipMethods + include User::TimelineMethods + # Include default devise modules. Others available are: # :confirmable, :lockable, :timeoutable and :omniauthable devise :database_authenticatable, :async, :registerable, @@ -100,11 +106,6 @@ class User < ApplicationRecord end end - # @return [Array] the users' timeline - def timeline - Answer.where("user_id in (?) OR user_id = ?", friend_ids, id).order(:created_at).reverse_order - end - # follows an user. def follow(target_user) active_relationships.create(target: target_user) diff --git a/app/models/user/answer_methods.rb b/app/models/user/answer_methods.rb new file mode 100644 index 00000000..3955a8cb --- /dev/null +++ b/app/models/user/answer_methods.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module User::AnswerMethods + include CursorPaginatable + + define_cursor_paginator :cursored_answers, :ordered_answers + + def ordered_answers + answers + .order(:created_at) + .reverse_order + end +end diff --git a/app/models/user/inbox_methods.rb b/app/models/user/inbox_methods.rb new file mode 100644 index 00000000..dea2fb36 --- /dev/null +++ b/app/models/user/inbox_methods.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +module User::InboxMethods + include CursorPaginatable + + define_cursor_paginator :cursored_inbox, :ordered_inbox + + def ordered_inbox + inboxes + .includes(:question, :user) + .order(:created_at) + .reverse_order + end +end diff --git a/app/models/user/question_methods.rb b/app/models/user/question_methods.rb new file mode 100644 index 00000000..608efe6b --- /dev/null +++ b/app/models/user/question_methods.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module User::QuestionMethods + include CursorPaginatable + + define_cursor_paginator :cursored_questions, :ordered_questions + + def ordered_questions + questions + .order(:created_at) + .reverse_order + end +end diff --git a/app/models/user/relationship_methods.rb b/app/models/user/relationship_methods.rb new file mode 100644 index 00000000..8093b6bc --- /dev/null +++ b/app/models/user/relationship_methods.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module User::RelationshipMethods + include CursorPaginatable + + define_cursor_paginator :cursored_friends, :ordered_friends + define_cursor_paginator :cursored_followers, :ordered_followers + + def ordered_friends + friends.reverse_order + end + + def ordered_followers + followers.reverse_order + end +end diff --git a/app/models/user/timeline_methods.rb b/app/models/user/timeline_methods.rb new file mode 100644 index 00000000..364d756f --- /dev/null +++ b/app/models/user/timeline_methods.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +module User::TimelineMethods + include CursorPaginatable + + define_cursor_paginator :cursored_timeline, :timeline + + # @return [Array] the users' timeline + def timeline + Answer.where('user_id in (?) OR user_id = ?', friend_ids, id).order(:created_at).reverse_order + end +end diff --git a/app/views/group/index.html.haml b/app/views/group/index.html.haml index 8c17ebca..26e38046 100644 --- a/app/views/group/index.html.haml +++ b/app/views/group/index.html.haml @@ -10,9 +10,9 @@ - @timeline.each do |answer| = render 'shared/answerbox', a: answer - #pagination= will_paginate @timeline, renderer: BootstrapPagination::Rails, page_links: false + = render 'shared/cursored_pagination_dummy', more_data_available: @more_data_available, last_id: @timeline_last_id - - if @timeline.next_page - %button#load-more-btn.btn.btn-default{type: :button, data: { current_page: @timeline.current_page }} + - if @more_data_available + %button#load-more-btn.btn.btn-default{type: :button, data: { last_id: @timeline_last_id }} = t 'views.actions.load' .visible-xs= render 'shared/links' diff --git a/app/views/group/index.js.erb b/app/views/group/index.js.erb index 9ce16da7..fd1ddb36 100644 --- a/app/views/group/index.js.erb +++ b/app/views/group/index.js.erb @@ -1,8 +1,8 @@ $('#timeline').append('<% @timeline.each do |answer| %><%= j render 'shared/answerbox', a: answer %><% end %>'); -<% if @timeline.next_page %> - $('#pagination').html('<%= j will_paginate @timeline, renderer: BootstrapPagination::Rails, page_links: false %>'); +<% if @more_data_available %> + $('#pagination').html('<%= j render 'shared/cursored_pagination_dummy', more_data_available: @more_data_available, last_id: @timeline_last_id %>'); <% else %> $('#pagination, #load-more-btn').remove(); -<% end %> \ No newline at end of file +<% end %> diff --git a/app/views/inbox/show.html.haml b/app/views/inbox/show.html.haml index 45c62ba0..cb9b1f1e 100644 --- a/app/views/inbox/show.html.haml +++ b/app/views/inbox/show.html.haml @@ -12,10 +12,10 @@ - if @inbox.empty? = t 'views.inbox.empty' - #pagination= will_paginate @inbox, renderer: BootstrapPagination::Rails, page_links: false + = render 'shared/cursored_pagination_dummy', more_data_available: @more_data_available, last_id: @inbox_last_id - - if @inbox.next_page - %button#load-more-btn.btn.btn-default{type: :button, data: { current_page: @inbox.current_page }} + - if @more_data_available + %button#load-more-btn.btn.btn-default{type: :button, data: { last_id: @inbox_last_id }} = t 'views.actions.load' .col-md-9.col-xs-12.col-sm-8.visible-xs diff --git a/app/views/inbox/show.js.erb b/app/views/inbox/show.js.erb index 32154b61..1d7ece9a 100644 --- a/app/views/inbox/show.js.erb +++ b/app/views/inbox/show.js.erb @@ -1,9 +1,9 @@ $('#entries').append('<% @inbox.each do |i| %><%= j render 'inbox/entry', i: i %><% end %>'); -<% if @inbox.next_page %> -$('#pagination').html('<%= j will_paginate @inbox, renderer: BootstrapPagination::Rails, page_links: false %>'); +<% if @more_data_available %> +$('#pagination').html('<%= j render 'shared/cursored_pagination_dummy', more_data_available: @more_data_available, last_id: @inbox_last_id %>'); <% else %> $('#pagination, #load-more-btn').remove(); <% end %> -<% Inbox.where(id: @inbox.pluck(:id)).update_all(new: false) %> \ No newline at end of file +<% Inbox.where(id: @inbox.pluck(:id)).update_all(new: false) %> diff --git a/app/views/notifications/index.html.haml b/app/views/notifications/index.html.haml index 68d32a0a..2993cffd 100644 --- a/app/views/notifications/index.html.haml +++ b/app/views/notifications/index.html.haml @@ -18,9 +18,9 @@ - @notifications.each do |notification| = render 'notifications/notification', notification: notification - #pagination= will_paginate @notifications, renderer: BootstrapPagination::Rails, page_links: false + = render 'shared/cursored_pagination_dummy', more_data_available: @more_data_available, last_id: @notifications_last_id, permitted_params: %i[type] - - if @notifications.next_page - %button#load-more-btn.btn.btn-default{type: :button, data: { current_page: @notifications.current_page }} + - if @more_data_available + %button#load-more-btn.btn.btn-default{type: :button, data: { last_id: @notifications_last_id }} Load more - Notification.for(current_user).update_all(new: false) diff --git a/app/views/notifications/index.js.erb b/app/views/notifications/index.js.erb index f3f5bd18..ba7adebb 100644 --- a/app/views/notifications/index.js.erb +++ b/app/views/notifications/index.js.erb @@ -1,8 +1,8 @@ $('#notifications').append('<% @notifications.each do |notification| %><%= j render 'notifications/notification', notification: notification %><% end %>'); -<% if @notifications.next_page %> - $('#pagination').html('<%= j will_paginate @notifications, renderer: BootstrapPagination::Rails, page_links: false %>'); +<% if @more_data_available %> + $('#pagination').html('<%= j render 'shared/cursored_pagination_dummy', more_data_available: @more_data_available, last_id: @notifications_last_id, permitted_params: %i[type] %>'); <% else %> $('#pagination, #load-more-btn').remove(); -<% end %> \ No newline at end of file +<% end %> diff --git a/app/views/public/index.html.haml b/app/views/public/index.html.haml index 6b8187f7..09336b59 100644 --- a/app/views/public/index.html.haml +++ b/app/views/public/index.html.haml @@ -10,9 +10,9 @@ - @timeline.each do |answer| = render 'shared/answerbox', a: answer - #pagination= will_paginate @timeline, renderer: BootstrapPagination::Rails, page_links: false + = render 'shared/cursored_pagination_dummy', more_data_available: @more_data_available, last_id: @timeline_last_id - - if @timeline.next_page - %button#load-more-btn.btn.btn-default{type: :button, data: { current_page: @timeline.current_page }} + - if @more_data_available + %button#load-more-btn.btn.btn-default{type: :button, data: { last_id: @timeline_last_id }} Load more .visible-xs= render 'shared/links' diff --git a/app/views/public/index.js.erb b/app/views/public/index.js.erb index 9ce16da7..fd1ddb36 100644 --- a/app/views/public/index.js.erb +++ b/app/views/public/index.js.erb @@ -1,8 +1,8 @@ $('#timeline').append('<% @timeline.each do |answer| %><%= j render 'shared/answerbox', a: answer %><% end %>'); -<% if @timeline.next_page %> - $('#pagination').html('<%= j will_paginate @timeline, renderer: BootstrapPagination::Rails, page_links: false %>'); +<% if @more_data_available %> + $('#pagination').html('<%= j render 'shared/cursored_pagination_dummy', more_data_available: @more_data_available, last_id: @timeline_last_id %>'); <% else %> $('#pagination, #load-more-btn').remove(); -<% end %> \ No newline at end of file +<% end %> diff --git a/app/views/question/show.html.haml b/app/views/question/show.html.haml index dbca8da0..5d407ab4 100644 --- a/app/views/question/show.html.haml +++ b/app/views/question/show.html.haml @@ -8,10 +8,10 @@ - @answers.each do |a| = render 'shared/answerbox', a: a, show_question: false - #pagination= will_paginate @answers, renderer: BootstrapPagination::Rails, page_links: false + = render 'shared/cursored_pagination_dummy', more_data_available: @more_data_available, last_id: @answers_last_id - - if @answers.next_page - %button#load-more-btn.btn.btn-default{type: :button, data: { current_page: @answers.current_page }} + - if @more_data_available + %button#load-more-btn.btn.btn-default{type: :button, data: { last_id: @answers_last_id }} Load more - if user_signed_in? and !current_user.answered? @question and current_user != @question.user and @question.user.privacy_allow_stranger_answers diff --git a/app/views/question/show.js.erb b/app/views/question/show.js.erb index bcaf826e..10610656 100644 --- a/app/views/question/show.js.erb +++ b/app/views/question/show.js.erb @@ -1,8 +1,8 @@ $('#answers').append('<% @answers.each do |answer| %><%= j render 'shared/answerbox', a: answer, show_question: false %><% end %>'); -<% if @answers.next_page %> - $('#pagination').html('<%= j will_paginate @answers, renderer: BootstrapPagination::Rails, page_links: false %>'); +<% if @more_data_available %> + $('#pagination').html('<%= j render 'shared/cursored_pagination_dummy', more_data_available: @more_data_available, last_id: @answers_last_id %>'); <% else %> $('#pagination, #load-more-btn').remove(); -<% end %> \ No newline at end of file +<% end %> diff --git a/app/views/shared/_cursored_pagination_dummy.haml b/app/views/shared/_cursored_pagination_dummy.haml new file mode 100644 index 00000000..84436d34 --- /dev/null +++ b/app/views/shared/_cursored_pagination_dummy.haml @@ -0,0 +1,11 @@ +-# this renders a pagination html to keep compatibility with the current pagination js +-# it _should_ be replaced with something else entirely later on. +- permitted_params ||= [] +#pagination + %ul.pagination + %li.next{class: more_data_available ? nil : "disabled"} + - if more_data_available + %a{rel: :next, href: url_for(params.permit(*permitted_params).merge(last_id: last_id))} + Next page + - else + Next page diff --git a/app/views/static/index.html.haml b/app/views/static/index.html.haml index 2abb97e6..8318e9b7 100644 --- a/app/views/static/index.html.haml +++ b/app/views/static/index.html.haml @@ -9,9 +9,9 @@ - @timeline.each do |answer| = render 'shared/answerbox', a: answer - #pagination= will_paginate @timeline, renderer: BootstrapPagination::Rails, page_links: false + = render 'shared/cursored_pagination_dummy', more_data_available: @more_data_available, last_id: @timeline_last_id - - if @timeline.next_page - %button#load-more-btn.btn.btn-default{type: :button, data: { current_page: @timeline.current_page }} + - if @more_data_available + %button#load-more-btn.btn.btn-default{type: :button, data: { last_id: @timeline_last_id }} Load more - .visible-xs= render 'shared/links' \ No newline at end of file + .visible-xs= render 'shared/links' diff --git a/app/views/static/index.js.erb b/app/views/static/index.js.erb index 9ce16da7..fd1ddb36 100644 --- a/app/views/static/index.js.erb +++ b/app/views/static/index.js.erb @@ -1,8 +1,8 @@ $('#timeline').append('<% @timeline.each do |answer| %><%= j render 'shared/answerbox', a: answer %><% end %>'); -<% if @timeline.next_page %> - $('#pagination').html('<%= j will_paginate @timeline, renderer: BootstrapPagination::Rails, page_links: false %>'); +<% if @more_data_available %> + $('#pagination').html('<%= j render 'shared/cursored_pagination_dummy', more_data_available: @more_data_available, last_id: @timeline_last_id %>'); <% else %> $('#pagination, #load-more-btn').remove(); -<% end %> \ No newline at end of file +<% end %> diff --git a/app/views/user/questions.html.haml b/app/views/user/questions.html.haml index 628118ae..efd269ec 100644 --- a/app/views/user/questions.html.haml +++ b/app/views/user/questions.html.haml @@ -13,9 +13,9 @@ - @questions.each do |q| = render 'shared/question', q: q, type: nil - #pagination= will_paginate @questions, renderer: BootstrapPagination::Rails, page_links: false + = render 'shared/cursored_pagination_dummy', more_data_available: @more_data_available, last_id: @questions_last_id - - if @questions.next_page - %button#load-more-btn.btn.btn-default{type: :button, data: { current_page: @questions.current_page }} + - if @more_data_available + %button#load-more-btn.btn.btn-default{type: :button, data: { last_id: @questions_last_id }} = t 'views.actions.load' .visible-xs= render 'shared/links' diff --git a/app/views/user/questions.js.erb b/app/views/user/questions.js.erb index a27fddb8..5b9e0ce7 100644 --- a/app/views/user/questions.js.erb +++ b/app/views/user/questions.js.erb @@ -1,8 +1,8 @@ $('#questions').append('<% @questions.each do |q| %><%= j render 'shared/question', q: q, type: nil %><% end %>'); -<% if @questions.next_page %> -$('#pagination').html('<%= j will_paginate @questions, renderer: BootstrapPagination::Rails, page_links: false %>'); +<% if @more_data_available %> +$('#pagination').html('<%= j render 'shared/cursored_pagination_dummy', more_data_available: @more_data_available, last_id: @questions_last_id %>'); <% else %> $('#pagination, #load-more-btn').remove(); <% end %> diff --git a/app/views/user/show.html.haml b/app/views/user/show.html.haml index 8e2d714a..c9ac350f 100644 --- a/app/views/user/show.html.haml +++ b/app/views/user/show.html.haml @@ -14,10 +14,10 @@ - @answers.each do |a| = render 'shared/answerbox', a: a - #pagination= will_paginate @answers, renderer: BootstrapPagination::Rails, page_links: false + = render 'shared/cursored_pagination_dummy', more_data_available: @more_data_available, last_id: @answers_last_id - - if @answers.next_page - %button#load-more-btn.btn.btn-default{type: :button, data: { current_page: @answers.current_page }} + - if @more_data_available + %button#load-more-btn.btn.btn-default{type: :button, data: { last_id: @answers_last_id }} = t 'views.actions.load' .visible-xs= render 'shared/links' - if user_signed_in? diff --git a/app/views/user/show.js.erb b/app/views/user/show.js.erb index 572ae837..443b9d7b 100644 --- a/app/views/user/show.js.erb +++ b/app/views/user/show.js.erb @@ -1,8 +1,8 @@ $('#answers').append('<% @answers.each do |a| %><%= j render 'shared/answerbox', a: a %><% end %>'); -<% if @answers.next_page %> - $('#pagination').html('<%= j will_paginate @answers, renderer: BootstrapPagination::Rails, page_links: false %>'); +<% if @more_data_available %> + $('#pagination').html('<%= j render 'shared/cursored_pagination_dummy', more_data_available: @more_data_available, last_id: @answers_last_id %>'); <% else %> $('#pagination, #load-more-btn').remove(); -<% end %> \ No newline at end of file +<% end %> diff --git a/app/views/user/show_follow.html.haml b/app/views/user/show_follow.html.haml index df39ac26..f5b020bb 100644 --- a/app/views/user/show_follow.html.haml +++ b/app/views/user/show_follow.html.haml @@ -14,10 +14,10 @@ .col-md-4.col-sm-6.col-xs-12 = render 'shared/userbox', user: user - #pagination= will_paginate @users, renderer: BootstrapPagination::Rails, page_links: false + = render 'shared/cursored_pagination_dummy', more_data_available: @more_data_available, last_id: @users_last_id - - if @users.next_page - %button#load-more-btn.btn.btn-default{type: :button, data: { current_page: @users.current_page }} + - if @more_data_available + %button#load-more-btn.btn.btn-default{type: :button, data: { last_id: @users_last_id }} = t 'views.actions.load' .visible-xs= render 'shared/links' - if user_signed_in? diff --git a/app/views/user/show_follow.js.erb b/app/views/user/show_follow.js.erb index bd3ecf51..6857406a 100644 --- a/app/views/user/show_follow.js.erb +++ b/app/views/user/show_follow.js.erb @@ -1,8 +1,8 @@ $('#users').append('<% @users.each do |user| %>
<%= j render 'shared/userbox', user: user %>
<% end %>'); -<% if @users.next_page %> - $('#pagination').html('<%= j will_paginate @users, renderer: BootstrapPagination::Rails, page_links: false %>'); +<% if @more_data_available %> + $('#pagination').html('<%= j render 'shared/cursored_pagination_dummy', more_data_available: @more_data_available, last_id: @users_last_id %>'); <% else %> $('#pagination, #load-more-btn').remove(); <% end %> diff --git a/config/initializers/20_will_paginate.rb b/config/initializers/20_will_paginate.rb deleted file mode 100644 index 6f8de892..00000000 --- a/config/initializers/20_will_paginate.rb +++ /dev/null @@ -1 +0,0 @@ -WillPaginate.per_page = APP_CONFIG['items_per_page'] \ No newline at end of file diff --git a/config/initializers/rails_admin.rb b/config/initializers/rails_admin.rb index e89e849a..1a6125be 100644 --- a/config/initializers/rails_admin.rb +++ b/config/initializers/rails_admin.rb @@ -1,12 +1,6 @@ # frozen_string_literal: true # workaround to get pagination right -if defined? WillPaginate - Kaminari.configure do |config| - config.page_method_name = :per_page_kaminari - end -end - RailsAdmin.config do |config| config.main_app_name = ['justask', 'Kontrollzentrum'] diff --git a/spec/factories/answer.rb b/spec/factories/answer.rb new file mode 100644 index 00000000..f0ce6069 --- /dev/null +++ b/spec/factories/answer.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :answer do + transient do + question_content { Faker::Lorem.sentence } + end + + content { Faker::Lorem.sentence } + question { FactoryBot.build(:question, content: question_content) } + end +end diff --git a/spec/factories/question.rb b/spec/factories/question.rb new file mode 100644 index 00000000..d5474c8b --- /dev/null +++ b/spec/factories/question.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :question do + user { nil } + content { Faker::Lorem.sentence } + author_is_anonymous { true } + end +end diff --git a/spec/factories/users.rb b/spec/factories/user.rb similarity index 86% rename from spec/factories/users.rb rename to spec/factories/user.rb index 488ff726..ddf58cb9 100644 --- a/spec/factories/users.rb +++ b/spec/factories/user.rb @@ -2,9 +2,9 @@ FactoryBot.define do factory :user do - sequence(:screen_name) { |i| "#{Faker::Internet.username(specifier: 0..12, separators: %w(_))}#{i}" } + sequence(:screen_name) { |i| "#{Faker::Internet.username(specifier: 0..12, separators: %w[_])}#{i}" } sequence(:email) { |i| "#{i}#{Faker::Internet.email}" } - password { "P4s5w0rD" } + password { 'P4s5w0rD' } confirmed_at { Time.now.utc } display_name { Faker::Name.name } diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index af300d69..c928fc5b 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1,28 +1,158 @@ +# frozen_string_literal: true + require 'rails_helper' -RSpec.describe User, :type => :model do - before :each do - @user = User.new( +RSpec.describe User, type: :model do + let!(:me) { FactoryBot.create :user } + + describe 'basic assigns' do + before :each do + @user = User.new( screen_name: 'FunnyMeme2004', password: 'y_u_no_secure_password?', email: 'nice.meme@nsa.gov' - ) + ) + end + + subject { @user } + + it { should respond_to(:email) } + + it '#email returns a string' do + expect(@user.email).to match 'nice.meme@nsa.gov' + end + + it '#motivation_header has a default value' do + expect(@user.motivation_header).to match '' + end + + it 'does not save an invalid screen name' do + @user.screen_name = '$Funny-Meme-%&2004' + expect { @user.save! }.to raise_error(ActiveRecord::RecordInvalid) + end end - subject { @user } + # -- User::TimelineMethods -- - it { should respond_to(:email) } - - it '#email returns a string' do - expect(@user.email).to match 'nice.meme@nsa.gov' + shared_examples_for 'result is blank' do + it 'result is blank' do + expect(subject).to be_blank + end end - it '#motivation_header has a default value' do - expect(@user.motivation_header).to match '' + describe '#timeline' do + subject { me.timeline } + + context 'user answered nothing and is not following anyone' do + include_examples 'result is blank' + end + + context 'user answered something and is not following anyone' do + let(:answer) { FactoryBot.create(:answer, user: me) } + + let(:expected) { [answer] } + + it 'includes the answer' do + expect(subject).to eq(expected) + end + end + + context 'user answered something and follows users with answers' do + let(:user1) { FactoryBot.create(:user) } + let(:user2) { FactoryBot.create(:user) } + let(:answer1) { FactoryBot.create(:answer, user: user1, created_at: 12.hours.ago) } + let(:answer2) { FactoryBot.create(:answer, user: me, created_at: 1.day.ago) } + let(:answer3) { FactoryBot.create(:answer, user: user2, created_at: 10.minutes.ago) } + let(:answer4) { FactoryBot.create(:answer, user: user1, created_at: Time.now.utc) } + + let!(:expected) do + [answer4, answer3, answer1, answer2] + end + + before(:each) do + me.follow(user1) + me.follow(user2) + end + + it 'includes all answers' do + expect(subject).to include(answer1) + expect(subject).to include(answer2) + expect(subject).to include(answer3) + expect(subject).to include(answer4) + end + + it 'result is ordered by created_at in reverse order' do + expect(subject).to eq(expected) + end + end end - it 'does not save an invalid screen name' do - @user.screen_name = '$Funny-Meme-%&2004' - expect{@user.save!}.to raise_error(ActiveRecord::RecordInvalid) + describe '#cursored_timeline' do + let(:last_id) { nil } + + subject { me.cursored_timeline(last_id: last_id, size: 3) } + + context 'user answered nothing and is not following anyone' do + include_examples 'result is blank' + end + + context 'user answered something and is not following anyone' do + let(:answer) { FactoryBot.create(:answer, user: me) } + + let(:expected) { [answer] } + + it 'includes the answer' do + expect(subject).to eq(expected) + end + end + + context 'user answered something and follows users with answers' do + let(:user1) { FactoryBot.create(:user) } + let(:user2) { FactoryBot.create(:user) } + let!(:answer1) { FactoryBot.create(:answer, user: me, created_at: 1.day.ago) } + let!(:answer2) { FactoryBot.create(:answer, user: user1, created_at: 12.hours.ago) } + let!(:answer3) { FactoryBot.create(:answer, user: user2, created_at: 10.minutes.ago) } + let!(:answer4) { FactoryBot.create(:answer, user: user1, created_at: Time.now.utc) } + + before(:each) do + me.follow(user1) + me.follow(user2) + end + + context 'last_id is nil' do + let(:last_id) { nil } + let(:expected) do + [answer4, answer3, answer2] + end + + it 'includes three answers' do + expect(subject).not_to include(answer1) + expect(subject).to include(answer2) + expect(subject).to include(answer3) + expect(subject).to include(answer4) + end + + it 'result is ordered by created_at in reverse order' do + expect(subject).to eq(expected) + end + end + + context 'last_id is answer2.id' do + let(:last_id) { answer2.id } + + it 'includes answer1' do + expect(subject).to include(answer1) + expect(subject).not_to include(answer2) + expect(subject).not_to include(answer3) + expect(subject).not_to include(answer4) + end + end + + context 'last_id is answer1.id' do + let(:last_id) { answer1.id } + + include_examples 'result is blank' + end + end end end