From ea1ac14da4bb58408f466ecd775a89c399a77cc2 Mon Sep 17 00:00:00 2001 From: weiqingtoh Date: Thu, 4 May 2017 16:13:41 +0800 Subject: [PATCH] Replace squeel with baby_squeel as squeel is not maintained - This commit attempts to remove most of squeel APIs. However, note that squeel is still bundled as part of the cancancan-squeel, which fixes some of Coursemology's cancancan issues (self-joins, etc). - The changes are aimed at using pure ActiveRecord methods which enables easier upgrading in the future. In cases where AR is insufficient, baby_squeel is used. --- Gemfile | 4 +-- Gemfile.lock | 5 +-- .../experience_points_records_controller.rb | 4 +-- .../course/lesson_plan/items_controller.rb | 2 +- .../course/user_invitations_controller.rb | 6 +++- .../course/forum_participation_concern.rb | 4 +-- app/models/concerns/course/search_concern.rb | 6 ++-- .../course_user/achievements_concern.rb | 2 +- .../concerns/course_user/staff_concern.rb | 10 +++--- .../concerns/instance_user_search_concern.rb | 6 ++-- app/models/concerns/user_search_concern.rb | 6 ++-- app/models/course.rb | 3 +- app/models/course/assessment.rb | 13 ++++---- .../answer/programming_file_annotation.rb | 6 ++-- .../assessment/programming_evaluation.rb | 4 +-- app/models/course/assessment/submission.rb | 19 +++++------ .../course/assessment/submission_question.rb | 6 ++-- app/models/course/condition/achievement.rb | 2 +- app/models/course/condition/assessment.rb | 6 ++-- app/models/course/discussion/post.rb | 11 ++++--- .../course/experience_points/disbursement.rb | 6 ++-- .../experience_points/forum_disbursement.rb | 8 +++-- app/models/course/experience_points_record.rb | 2 +- app/models/course/forum.rb | 21 ++++++------ app/models/course/forum/search.rb | 2 +- app/models/course/forum/topic.rb | 29 ++++++++--------- app/models/course/group.rb | 32 +++++++++---------- app/models/course/lesson_plan/item.rb | 2 +- app/models/course/lesson_plan/todo.rb | 12 +++---- app/models/course/material.rb | 2 +- app/models/course/material/folder.rb | 10 +++--- app/models/course/video.rb | 5 ++- app/models/course_user.rb | 24 +++++++------- app/models/instance.rb | 12 +++---- app/models/instance_user.rb | 2 +- .../email_invitation_concern.rb | 7 ++-- .../course/material/preload_service.rb | 3 +- .../coursemology/polyglot/language.rb | 2 +- .../time_bounded_record/active_record/base.rb | 4 +-- .../lesson_plan_milestone_management_spec.rb | 6 +++- spec/features/course/lesson_plan_spec.rb | 24 ++++---------- spec/libraries/date_time_helpers.rb | 4 +-- 42 files changed, 169 insertions(+), 175 deletions(-) diff --git a/Gemfile b/Gemfile index aeb235b1fc4..77bc2aef1d5 100644 --- a/Gemfile +++ b/Gemfile @@ -33,8 +33,8 @@ gem 'activerecord-userstamp', '>= 3.0.2' gem 'after_commit_action' # Allow declaring the calculated attributes of a record gem 'calculated_attributes', '>= 0.1.3' -# Squeel as an SQL-like DSL -gem 'squeel' +# Baby Squeel as an SQL-like DSL +gem 'baby_squeel' # For multiple table inheritance # TODO: Figure out breaking changes in v2 as polymorphism is not working correctly. gem 'active_record-acts_as', github: 'Coursemology/active_record-acts_as' diff --git a/Gemfile.lock b/Gemfile.lock index 6fb7dc3b6e9..872d7e99997 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -100,6 +100,8 @@ GEM autoprefixer-rails (7.0.1) execjs awesome_print (1.7.0) + baby_squeel (0.3.1) + activerecord (>= 4.0.0) bcrypt (3.1.11) bootstrap-sass (3.3.7) autoprefixer-rails (>= 5.2.1) @@ -555,6 +557,7 @@ DEPENDENCIES acts_as_tenant after_commit_action autoprefixer-rails + baby_squeel bootstrap-sass bootstrap-sass-extras (>= 0.0.7) bootstrap-select-rails @@ -630,7 +633,6 @@ DEPENDENCIES simplecov slim-rails spring - squeel summernote-rails themes_on_rails (>= 0.3.1)! traceroute @@ -644,6 +646,5 @@ DEPENDENCIES workflow yard - BUNDLED WITH 1.14.6 diff --git a/app/controllers/course/experience_points_records_controller.rb b/app/controllers/course/experience_points_records_controller.rb index 67bf7b823a7..a63bfcc94db 100644 --- a/app/controllers/course/experience_points_records_controller.rb +++ b/app/controllers/course/experience_points_records_controller.rb @@ -13,8 +13,8 @@ def index # :nodoc: Course::CourseUserPreloadService.new(updater_ids, current_course) @experience_points_records = - @experience_points_records.active.includes { actable }. - includes(:updater).order(updated_at: :desc).page(page_param) + @experience_points_records.active. + includes(:actable, :updater).order(updated_at: :desc).page(page_param) end def update diff --git a/app/controllers/course/lesson_plan/items_controller.rb b/app/controllers/course/lesson_plan/items_controller.rb index 6ecd3047084..850f7d72e63 100644 --- a/app/controllers/course/lesson_plan/items_controller.rb +++ b/app/controllers/course/lesson_plan/items_controller.rb @@ -49,7 +49,7 @@ def render_json_response # @return [Hash{Integer => Array] def assessment_tabs_titles_hash @assessment_tabs_titles_hash ||= - current_course.assessment_categories.includes { tabs }.map(&:tabs).flatten. + current_course.assessment_categories.includes(:tabs).map(&:tabs).flatten. map do |tab| [tab.id, tab_title_array(tab)] end.to_h diff --git a/app/controllers/course/user_invitations_controller.rb b/app/controllers/course/user_invitations_controller.rb index 17daf27f357..458821be3cc 100644 --- a/app/controllers/course/user_invitations_controller.rb +++ b/app/controllers/course/user_invitations_controller.rb @@ -104,7 +104,11 @@ def load_invitations @invitations ||= begin ids = resend_invitation_params ids ||= current_course.invitations.unconfirmed.select(:id) - ids.blank? ? [] : current_course.invitations.unconfirmed.where { id >> ids } + if ids.blank? + [] + else + current_course.invitations.unconfirmed.where('course_user_invitations.id IN (?)', ids) + end end end diff --git a/app/models/concerns/course/forum_participation_concern.rb b/app/models/concerns/course/forum_participation_concern.rb index 2fb257261b1..180ca5f874b 100644 --- a/app/models/concerns/course/forum_participation_concern.rb +++ b/app/models/concerns/course/forum_participation_concern.rb @@ -4,11 +4,11 @@ module Course::ForumParticipationConcern module ClassMethods def forum_posts - joins { topic }.where { topic.actable_type == Course::Forum::Topic } + joins(:topic).where('course_discussion_topics.actable_type = ?', Course::Forum::Topic.name) end def from_course(course) - joins { topic }.where { topic.course_id == course.id } + joins(:topic).where('course_discussion_topics.course_id = ?', course.id) end end end diff --git a/app/models/concerns/course/search_concern.rb b/app/models/concerns/course/search_concern.rb index 86e110f7c9d..643129a1528 100644 --- a/app/models/concerns/course/search_concern.rb +++ b/app/models/concerns/course/search_concern.rb @@ -11,9 +11,9 @@ def search(keyword) # rubocop:disable Metrics/AbcSize return all if keyword.blank? condition = "%#{keyword}%" - joins { users.outer }. - where { (title =~ condition) | (description =~ condition) | (users.name =~ condition) }. - group { courses.id } + joining { users.outer }. + where.has { (title =~ condition) | (description =~ condition) | (users.name =~ condition) }. + group('courses.id') end end end diff --git a/app/models/concerns/course_user/achievements_concern.rb b/app/models/concerns/course_user/achievements_concern.rb index 4751a5898f7..8aac8a639ba 100644 --- a/app/models/concerns/course_user/achievements_concern.rb +++ b/app/models/concerns/course_user/achievements_concern.rb @@ -2,6 +2,6 @@ module CourseUser::AchievementsConcern # Order achievements based on when each course_user obtained the achievement. def ordered_by_date_obtained - order { course_user_achievements.obtained_at.desc } + order('course_user_achievements.obtained_at DESC') end end diff --git a/app/models/concerns/course_user/staff_concern.rb b/app/models/concerns/course_user/staff_concern.rb index 45011c569f2..067bbbcdb20 100644 --- a/app/models/concerns/course_user/staff_concern.rb +++ b/app/models/concerns/course_user/staff_concern.rb @@ -28,11 +28,11 @@ def self.order_by_average_marking_time(staff) def published_submissions # rubocop:disable Metrics/AbcSize @published_submissions ||= Course::Assessment::Submission. - joins { experience_points_record.course_user }. - where { experience_points_record.course_user.role == CourseUser.roles[:student] }. - where { experience_points_record.course_user.phantom == false }. - where { experience_points_record.course_user.course_id == my { course_id } }. - where { publisher_id == my { user_id } } + joins(experience_points_record: :course_user). + where('course_users.role = ?', CourseUser.roles[:student]). + where('course_users.phantom = ?', false). + where('course_assessment_submissions.publisher_id = ?', user_id). + where('course_users.course_id = ?', course_id) end # Returns the average marking time of the staff. diff --git a/app/models/concerns/instance_user_search_concern.rb b/app/models/concerns/instance_user_search_concern.rb index 21e053cff6c..e9d00a5725c 100644 --- a/app/models/concerns/instance_user_search_concern.rb +++ b/app/models/concerns/instance_user_search_concern.rb @@ -12,9 +12,9 @@ def search(keyword) # rubocop:disable Metrics/AbcSize return all if keyword.blank? condition = "%#{keyword}%" - joins { user.emails.outer }. - where { (user.name =~ condition) | (user.emails.email =~ condition) }. - group { instance_users.id } + joining { user.emails.outer }. + where.has { (sql('users.name') =~ condition) | (sql('user_emails.email') =~ condition) }. + group('instance_users.id') end end end diff --git a/app/models/concerns/user_search_concern.rb b/app/models/concerns/user_search_concern.rb index 7477edecf5f..a23cd631de5 100644 --- a/app/models/concerns/user_search_concern.rb +++ b/app/models/concerns/user_search_concern.rb @@ -12,9 +12,9 @@ def search(keyword) # rubocop:disable Metrics/AbcSize return all if keyword.blank? condition = "%#{keyword}%" - joins { emails.outer }. - where { (name =~ condition) | (emails.email =~ condition) }. - group { users.id } + joining { emails.outer }. + where.has { (name =~ condition) | (emails.email =~ condition) }. + group('users.id') end end end diff --git a/app/models/course.rb b/app/models/course.rb index 26f1f1e95b2..406353d2085 100644 --- a/app/models/course.rb +++ b/app/models/course.rb @@ -59,8 +59,7 @@ class Course < ActiveRecord::Base # @!method containing_user # Selects all the courses with user as one of its members scope :containing_user, (lambda do |user| - joins { course_users }. - where { course_users.user_id == user.id } + joins(:course_users).where('course_users.user_id = ?', user.id) end) # @!method with_owners diff --git a/app/models/course/assessment.rb b/app/models/course/assessment.rb index 744b2a64888..32ec4889c30 100644 --- a/app/models/course/assessment.rb +++ b/app/models/course/assessment.rb @@ -51,17 +51,18 @@ class Course::Assessment < ActiveRecord::Base # @return [Fixnum] calculated :maximum_grade, (lambda do Course::Assessment::Question.unscope(:order). - select { coalesce(sum(course_assessment_questions.maximum_grade), 0) }. - where { course_assessment_questions.assessment_id == course_assessments.id } + select('coalesce(sum(course_assessment_questions.maximum_grade), 0)'). + where('course_assessment_questions.assessment_id = course_assessments.id') end) # @!method self.ordered_by_date_and_title # Orders the assessments by the starting date and title. scope :ordered_by_date_and_title, (lambda do - select('course_assessments.*'). - select { [lesson_plan_item.start_at, lesson_plan_item.title] }. - joins { lesson_plan_item }. - merge(Course::LessonPlan::Item.ordered_by_date_and_title) + select(<<~SQL). + course_assessments.*, course_lesson_plan_items.start_at, course_lesson_plan_items.title + SQL + joins(:lesson_plan_item). + merge(Course::LessonPlan::Item.ordered_by_date_and_title) end) # @!method with_submissions_by(creator) diff --git a/app/models/course/assessment/answer/programming_file_annotation.rb b/app/models/course/assessment/answer/programming_file_annotation.rb index 910b850d2a5..6348827ed50 100644 --- a/app/models/course/assessment/answer/programming_file_annotation.rb +++ b/app/models/course/assessment/answer/programming_file_annotation.rb @@ -10,9 +10,9 @@ class Course::Assessment::Answer::ProgrammingFileAnnotation < ActiveRecord::Base # Specific implementation of Course::Discussion::Topic#from_user, this is not supposed to be # called directly. scope :from_user, (lambda do |user_id| - joins { file.answer.answer.submission }. - where { file.answer.answer.submission.creator_id >> user_id }. - joins { discussion_topic }.select { discussion_topic.id } + joining { file.answer.answer.submission }. + where.has { file.answer.answer.submission.creator_id.in(user_id) }. + joining { discussion_topic }.selecting { discussion_topic.id } end) def notify(post) diff --git a/app/models/course/assessment/programming_evaluation.rb b/app/models/course/assessment/programming_evaluation.rb index 233b58215bf..1071a91b22e 100644 --- a/app/models/course/assessment/programming_evaluation.rb +++ b/app/models/course/assessment/programming_evaluation.rb @@ -55,7 +55,7 @@ class Course::Assessment::ProgrammingEvaluation < ActiveRecord::Base # Gets the programming evaluation jobs which are in the submitted state, and pending # allocation to an evaluator. scope :pending, (lambda do - where do + where.has do (status == 'submitted') | ( (status == 'assigned') & (assigned_at < Time.zone.now - TIMEOUT)) end @@ -64,7 +64,7 @@ class Course::Assessment::ProgrammingEvaluation < ActiveRecord::Base # @!method self.finished # Gets the programming evaluation jobs which have finished, which are those which are # completed or errored. - scope :finished, -> { where { (status == 'completed') | (status == 'errored') } } + scope :finished, -> { where(status: ['completed', 'errored']) } # Checks if the given task is still pending assignment to an evaluator. # diff --git a/app/models/course/assessment/submission.rb b/app/models/course/assessment/submission.rb index 7738b2840d1..0717118d453 100644 --- a/app/models/course/assessment/submission.rb +++ b/app/models/course/assessment/submission.rb @@ -72,9 +72,9 @@ class Course::Assessment::Submission < ActiveRecord::Base # Gets the time the submission was graded. # @return [Time] calculated :graded_at, (lambda do - Course::Assessment::Answer.unscope(:order).where do - course_assessment_answers.submission_id == course_assessment_submissions.id - end.select { max(course_assessment_answers.graded_at) } + Course::Assessment::Answer.unscope(:order). + where('course_assessment_answers.submission_id = course_assessment_submissions.id'). + select('max(course_assessment_answers.graded_at)') end) # @!method self.by_user(user) @@ -84,21 +84,22 @@ class Course::Assessment::Submission < ActiveRecord::Base # @!method self.by_users(user) # @param [Fixnum|Array] user_ids The user ids to filter submissions by - scope :by_users, ->(user_ids) { where { creator_id >> user_ids } } + scope :by_users, ->(user_ids) { where(creator_id: user_ids) } # @!method self.from_category(category) # Finds all the submissions in the given category. # @param [Course::Assessment::Category] category The category to filter submissions by scope :from_category, (lambda do |category| - where { assessment_id >> category.assessments.select(:id) } + where(assessment_id: category.assessments.select(:id)) end) scope :from_course, (lambda do |course| - joins { assessment.tab.category }.where { assessment.tab.category.course == course } + joins(assessment: { tab: :category }). + where('course_assessment_categories.course_id = ?', course.id) end) scope :from_group, (lambda do |group_id| - joins(experience_points_record: [course_user: :groups]). + joins(experience_points_record: { course_user: :groups }). where('course_groups.id IN (?)', group_id) end) @@ -117,8 +118,8 @@ class Course::Assessment::Submission < ActiveRecord::Base scope :pending_for_grading, (lambda do where(workflow_state: [:submitted, :graded]). - joins { assessment }. - where { assessment.autograded == false } + joins(:assessment). + where('course_assessments.autograded = ?', false) end) # Filter submissions by category_id, assessment_id, group_id and/or user_id (creator) diff --git a/app/models/course/assessment/submission_question.rb b/app/models/course/assessment/submission_question.rb index e1177b34e73..47a975ff01f 100644 --- a/app/models/course/assessment/submission_question.rb +++ b/app/models/course/assessment/submission_question.rb @@ -14,9 +14,9 @@ class Course::Assessment::SubmissionQuestion < ActiveRecord::Base # Specific implementation of Course::Discussion::Topic#from_user, this is not supposed to be # called directly. scope :from_user, (lambda do |user_id| - joins { submission }. - where { submission.creator_id >> user_id }. - joins { discussion_topic }.select { discussion_topic.id } + joining { submission }. + where.has { submission.creator_id.in(user_id) }. + joining { discussion_topic }.selecting { discussion_topic.id } end) def notify(post) diff --git a/app/models/course/condition/achievement.rb b/app/models/course/condition/achievement.rb index 9ea70fd64ef..f3a6ac4faa8 100644 --- a/app/models/course/condition/achievement.rb +++ b/app/models/course/condition/achievement.rb @@ -62,7 +62,7 @@ def initialize_duplicate(duplicator, other) def required_achievements_for(conditional) # Course::Condition::Achievement. # joins { condition.conditional(Course::Achievement) }. - # where { condition.conditional.id == achievement.id }. + # where.has { condition.conditional.id == achievement.id }. # map(&:achievement) # Workaround, pending the squeel bugfix (activerecord-hackery/squeel#390) that will allow diff --git a/app/models/course/condition/assessment.rb b/app/models/course/condition/assessment.rb index bf4374392d8..2984db75b57 100644 --- a/app/models/course/condition/assessment.rb +++ b/app/models/course/condition/assessment.rb @@ -75,9 +75,9 @@ def submitted_submissions_by_user(user) def published_submissions_with_minimum_grade(user, minimum_grade) assessment.submissions.by_user(user).with_published_state. - joins { answers }. - group { course_assessment_submissions.id }. - having { coalesce(sum(answers.grade), 0) >= minimum_grade } + joins(:answers). + group('course_assessment_submissions.id'). + when_having { coalesce(sum(answers.grade), 0) >= minimum_grade } end def validate_assessment_condition diff --git a/app/models/course/discussion/post.rb b/app/models/course/discussion/post.rb index 193a340c663..84d462c3177 100644 --- a/app/models/course/discussion/post.rb +++ b/app/models/course/discussion/post.rb @@ -27,7 +27,8 @@ class Course::Discussion::Post < ActiveRecord::Base scope :with_user_votes, (lambda do |user| post_ids = pluck('course_discussion_posts.id') votes = Course::Discussion::Post::Vote. - where { post_id.in(post_ids) & (creator_id == user.id) } + where('course_discussion_post_votes.post_id IN (?)', post_ids). + where('course_discussion_post_votes.creator_id = ?', user.id) all.tap do |result| preloader = ActiveRecord::Associations::Preloader::ManualPreloader.new @@ -39,16 +40,16 @@ class Course::Discussion::Post < ActiveRecord::Base # The number of upvotes for the given post. calculated :upvotes, (lambda do Vote.upvotes. - select { count(id) }. - where { post_id == course_discussion_posts.id } + select('count(id)'). + where('post_id = course_discussion_posts.id') end) # @!attribute [r] downvotes # The number of downvotes for the given post. calculated :downvotes, (lambda do Vote.downvotes. - select { count(id) }. - where { post_id == course_discussion_posts.id } + select('count(id)'). + where('post_id = course_discussion_posts.id') end) # Calculates the total number of votes given to this post. diff --git a/app/models/course/experience_points/disbursement.rb b/app/models/course/experience_points/disbursement.rb index 34d96a240d7..24bd18fd0b6 100644 --- a/app/models/course/experience_points/disbursement.rb +++ b/app/models/course/experience_points/disbursement.rb @@ -87,9 +87,7 @@ def filtered_students # @param [Integer] group_id The id of the group # @return [Array] The students in the group def students_from_group(group_id) - course.course_users. - joins { group_users }. - merge(Course::GroupUser.normal). - where { group_users.group_id == group_id } + course.course_users.joins(:group_users).where('course_group_users.group_id = ?', group_id). + merge(Course::GroupUser.normal) end end diff --git a/app/models/course/experience_points/forum_disbursement.rb b/app/models/course/experience_points/forum_disbursement.rb index dd427d8c9d3..2fcd53add86 100644 --- a/app/models/course/experience_points/forum_disbursement.rb +++ b/app/models/course/experience_points/forum_disbursement.rb @@ -144,10 +144,12 @@ def ranked_statistic_groups # @return [Array] def discussion_posts return [] if end_time_preceeds_start_time? - @discussion_posts ||= + @discussion_posts ||= begin + user_ids = forum_participants.map(&:user_id) Course::Discussion::Post.forum_posts.from_course(course).calculated(:upvotes, :downvotes). - where(created_at: start_time..end_time). - where { creator_id >> my { forum_participants.map(&:user_id) } } + where(created_at: start_time..end_time). + where(creator_id: user_ids) + end end # Check if end time preceeds start time and sets an error if necessary. diff --git a/app/models/course/experience_points_record.rb b/app/models/course/experience_points_record.rb index da51d516ef1..88c817799af 100644 --- a/app/models/course/experience_points_record.rb +++ b/app/models/course/experience_points_record.rb @@ -11,7 +11,7 @@ class Course::ExperiencePointsRecord < ActiveRecord::Base # TODO: Add an optional: true when moving to Rails 5. belongs_to :awarder, class_name: User.name, inverse_of: nil - scope :active, -> { where { points_awarded != nil } } # rubocop:disable Style/NonNilCheck + scope :active, -> { where.not(points_awarded: nil) } # Checks if the current record is active, i.e. it has been granted by a course staff. # diff --git a/app/models/course/forum.rb b/app/models/course/forum.rb index a325f75e289..1a9e51b13aa 100644 --- a/app/models/course/forum.rb +++ b/app/models/course/forum.rb @@ -10,30 +10,31 @@ class Course::Forum < ActiveRecord::Base # @!attribute [r] topic_count # The number of topics in this forum. calculated :topic_count, (lambda do - Course::Forum::Topic.where { course_forum_topics.forum_id == course_forums.id }. - select { count('*') } + Course::Forum::Topic.where('course_forum_topics.forum_id = course_forums.id'). + select("count('*')") end) # @!attribute [r] topic_post_count # The number of posts in this forum. calculated :topic_post_count, (lambda do - Course::Forum::Topic.joins { discussion_topic.outer.posts.outer }. - where { course_forum_topics.forum_id == course_forums.id }. - select { count('*') } + Course::Forum::Topic. + joining { discussion_topic.outer.posts.outer }. + where('course_forum_topics.forum_id = course_forums.id'). + select("count('*')") end) # @!attribute [r] topic_view_count # The number of views in this forum. calculated :topic_view_count, (lambda do - Course::Forum::Topic.joins { views }. - where { course_forum_topics.forum_id == course_forums.id }. - select { count('*') } + Course::Forum::Topic.joins(:views). + where('course_forum_topics.forum_id = course_forums.id'). + select("count('*')") end) calculated :topic_unread_count, (lambda do |user| - Course::Forum::Topic.where { course_forum_topics.forum_id == course_forums.id }. + Course::Forum::Topic.where('course_forum_topics.forum_id = course_forums.id'). unread_by(user). - select { count('*') } + select("count('*')") end) # @!method self.with_forum_statistics diff --git a/app/models/course/forum/search.rb b/app/models/course/forum/search.rb index 5752112fe6b..a22bcada962 100644 --- a/app/models/course/forum/search.rb +++ b/app/models/course/forum/search.rb @@ -32,7 +32,7 @@ def posts @posts ||= Course::Discussion::Post.forum_posts.from_course(@course). - includes { topic.actable.forum }. + includes(topic: { actable: :forum }). calculated(:upvotes, :downvotes). where(created_at: start_time..end_time). where(creator_id: @user) diff --git a/app/models/course/forum/topic.rb b/app/models/course/forum/topic.rb index f722c3515cd..d80582bda2a 100644 --- a/app/models/course/forum/topic.rb +++ b/app/models/course/forum/topic.rb @@ -20,29 +20,25 @@ class Course::Forum::Topic < ActiveRecord::Base # @!attribute [r] vote_count # The number of votes in this topic. calculated :vote_count, (lambda do - Course::Discussion::Post::Vote.joins { post.topic }. - where do - (course_forum_topics.id == course_discussion_topics.actable_id) & - (course_discussion_topics.actable_type == Course::Forum::Topic.name) - end.select { count('*') } + Course::Discussion::Post::Vote.joins(post: :topic). + where('course_forum_topics.id = course_discussion_topics.actable_id'). + where('course_discussion_topics.actable_type = ?', Course::Forum::Topic.name). + select("count('*')") end) # @!attribute [r] post_count # The number of posts in this topic. calculated :post_count, (lambda do - Course::Discussion::Topic.joins { posts.inner }. - where do - (actable_id == course_forum_topics.id) & - (actable_type == Course::Forum::Topic.name) - end.select { count('*') } + Course::Discussion::Topic.joins(:posts). + where('actable_id = course_forum_topics.id'). + where(actable_type: Course::Forum::Topic.name). + select("count('*')") end) # @!attribute [r] view_count # The number of views in this topic. calculated :view_count, (lambda do - Course::Forum::Topic::View.where do - topic_id == course_forum_topics.id - end.select { count('*') } + Course::Forum::Topic::View.where('topic_id = course_forum_topics.id').select("count('*')") end) # @!method self.order_by_date @@ -57,8 +53,9 @@ class Course::Forum::Topic < ActiveRecord::Base scope :with_latest_post, (lambda do topic_ids = distinct(false).pluck('course_discussion_topics.id') ids = Course::Discussion::Post.unscope(:order). - select { max(id) }.group { course_discussion_posts.topic_id }. - where { topic_id.in(topic_ids) } + select('max(id)'). + group('course_discussion_posts.topic_id'). + where(topic_id: topic_ids) last_posts = Course::Discussion::Post.with_creator.where(id: ids) all.tap do |result| @@ -73,7 +70,7 @@ class Course::Forum::Topic < ActiveRecord::Base -> { all.calculated(:post_count, :view_count) } # Get all the topics from specified course. - scope :from_course, ->(course) { joins { forum }.where { forum.course_id == course } } + scope :from_course, ->(course) { joins(:forum).where('course_forums.course_id = ?', course.id) } # Create view record for a user # diff --git a/app/models/course/group.rb b/app/models/course/group.rb index 214fdeb118d..065e794fd7c 100644 --- a/app/models/course/group.rb +++ b/app/models/course/group.rb @@ -18,14 +18,14 @@ class Course::Group < ActiveRecord::Base # @!attribute [r] average_experience_points # Returns the average experience points of group users in this group who are students. calculated :average_experience_points, (lambda do - Course::GroupUser.where { group_id == course_groups.id }. - joins { course_user.experience_points_records.outer }. - where { course_user.role == CourseUser.roles[:student] }. + Course::GroupUser.where('course_group_users.group_id = course_groups.id'). + joining { course_user.experience_points_records.outer }. + where('course_users.role = ?', CourseUser.roles[:student]). # CAST is used to force a float division (integer division by default). # greatest(#, 1) is used to avoid division by 0. - select do - cast(coalesce(sum(course_experience_points_records.points_awarded), 0.0).as(float)) / - greatest(count(distinct(course_group_users.course_user_id)), 1.0) + selecting do + cast(sql('coalesce(sum(course_experience_points_records.points_awarded), 0.0) as float')) / + greatest(sql('count(distinct(course_group_users.course_user_id)), 1.0')) end end) @@ -33,14 +33,14 @@ class Course::Group < ActiveRecord::Base # Returns the average number of achievements obtained by group users in this group who are # students. calculated :average_achievement_count, (lambda do - Course::GroupUser.where { group_id == course_groups.id }. - joins { course_user.course_user_achievements.outer }. - where { course_user.role == CourseUser.roles[:student] }. + Course::GroupUser.where('course_group_users.group_id = course_groups.id'). + joining { course_user.course_user_achievements.outer }. + where('course_users.role = ?', CourseUser.roles[:student]). # CAST is used to force a float division (integer division by default). # greatest(#, 1) is used to avoid division by 0. - select do - cast(count(course_user_achievements.id).as(float)) / - greatest(count(distinct(course_group_users.course_user_id)), 1.0) + selecting do + cast(sql('count(course_user_achievements.id) as float')) / + greatest(sql('count(distinct(course_group_users.course_user_id)), 1.0')) end end) @@ -48,10 +48,10 @@ class Course::Group < ActiveRecord::Base # Returns the time of the last obtained achievement by group users in this group who are # students. calculated :last_obtained_achievement, (lambda do - Course::GroupUser.where { group_id == course_groups.id }. - joins { course_user.course_user_achievements }. - where { course_user.role == CourseUser.roles[:student] }. - select { course_user_achievements.obtained_at }.limit(1).order('obtained_at DESC') + Course::GroupUser.where('course_group_users.group_id = course_groups.id'). + joins(course_user: :course_user_achievements). + where('course_users.role = ?', CourseUser.roles[:student]). + select('course_user_achievements.obtained_at').limit(1).order('obtained_at DESC') end) scope :ordered_by_experience_points, (lambda do diff --git a/app/models/course/lesson_plan/item.rb b/app/models/course/lesson_plan/item.rb index 23a4aba5209..ef3cd6d056d 100644 --- a/app/models/course/lesson_plan/item.rb +++ b/app/models/course/lesson_plan/item.rb @@ -14,7 +14,7 @@ class Course::LessonPlan::Item < ActiveRecord::Base # @!method self.ordered_by_date # Orders the lesson plan items by the starting date. scope :ordered_by_date, (lambda do - order { start_at } + order(:start_at) end) scope :ordered_by_date_and_title, (lambda do diff --git a/app/models/course/lesson_plan/todo.rb b/app/models/course/lesson_plan/todo.rb index afcbe47d2da..343918eddb1 100644 --- a/app/models/course/lesson_plan/todo.rb +++ b/app/models/course/lesson_plan/todo.rb @@ -13,19 +13,19 @@ class Course::LessonPlan::Todo < ActiveRecord::Base belongs_to :user, inverse_of: :todos belongs_to :item, class_name: Course::LessonPlan::Item.name, inverse_of: :todos - default_scope { joins { item }.order { item.start_at.asc } } + default_scope { joins(:item).order('course_lesson_plan_items.start_at ASC') } # Started is not used as it is defined in Extensions::TimeBoundedRecord::ActiveRecord::Base - scope :opened, -> { joins { item }.where { item.start_at <= Time.zone.now } } - scope :published, -> { joins { item }.where { item.published == true } } + scope :opened, -> { joins(:item).where.has { item.start_at <= Time.zone.now } } + scope :published, -> { joins(:item).where('course_lesson_plan_items.published = ?', true) } scope :not_ignored, -> { where(ignore: false) } scope :not_completed, -> { where.not(workflow_state: :completed) } scope :from_course, (lambda do |course| - joins { item }.where { item.course_id == course.id } + joins(:item).where('course_lesson_plan_items.course_id = ?', course.id) end) scope :pending_for, (lambda do |course_user| opened.published.not_ignored.from_course(course_user.course).not_completed. - where { user_id == course_user.user_id } + where('course_lesson_plan_todos.user_id = ?', course_user.user_id) end) class << self @@ -56,7 +56,7 @@ def create_for!(items, course_users) def delete_for(item, course_users = nil) return unless item user_ids = course_users ? course_users.select(:user_id) : item.todos.select(:user_id) - item.todos.where { user_id.in(user_ids) }.destroy_all + item.todos.where.has { user_id.in(user_ids) }.destroy_all end private diff --git a/app/models/course/material.rb b/app/models/course/material.rb index 6dbf02f17e1..8055e92089f 100644 --- a/app/models/course/material.rb +++ b/app/models/course/material.rb @@ -40,7 +40,7 @@ def initialize_duplicate(duplicator, other) def validate_name_is_unique_among_folders return if folder.nil? - conflicts = folder.children.where { name =~ my { name } } + conflicts = folder.children.where.has { |parent| name =~ parent.name } errors.add(:name, :taken) unless conflicts.empty? end diff --git a/app/models/course/material/folder.rb b/app/models/course/material/folder.rb index ab54993fec4..9011aa20940 100644 --- a/app/models/course/material/folder.rb +++ b/app/models/course/material/folder.rb @@ -20,16 +20,16 @@ class Course::Material::Folder < ActiveRecord::Base # @!attribute [r] material_count # Returns the number of files in current folder. calculated :material_count, (lambda do - Course::Material.select { count('*') }. - where { course_materials.folder_id == course_material_folders.id } + Course::Material.select("count('*')"). + where('course_materials.folder_id = course_material_folders.id') end) # @!attribute [r] children_count # Returns the number of subfolders in current folder. calculated :children_count, (lambda do - select { count('*') }. + select("count('*')"). from('course_material_folders children'). - where { children.parent_id == course_material_folders.id } + where('children.parent_id = course_material_folders.id') end) scope :with_content_statistics, ->() { all.calculated(:material_count, :children_count) } @@ -104,7 +104,7 @@ def set_defaults def validate_name_is_unique_among_materials return if parent.nil? - conflicts = parent.materials.where { name =~ my { name } } + conflicts = parent.materials.where.has { |parent| name =~ parent.name } errors.add(:name, :taken) unless conflicts.empty? end diff --git a/app/models/course/video.rb b/app/models/course/video.rb index 67af2a0862d..dd111b286fb 100644 --- a/app/models/course/video.rb +++ b/app/models/course/video.rb @@ -12,9 +12,8 @@ class Course::Video < ActiveRecord::Base # @!method self.ordered_by_date_and_title # Orders the videos by the starting date and title. scope :ordered_by_date_and_title, (lambda do - select('course_videos.*'). - select('course_lesson_plan_items.start_at, course_lesson_plan_items.title'). - joins { lesson_plan_item }. + select('course_videos.*, course_lesson_plan_items.start_at, course_lesson_plan_items.title'). + joins(:lesson_plan_item). merge(Course::LessonPlan::Item.ordered_by_date_and_title) end) diff --git a/app/models/course_user.rb b/app/models/course_user.rb index 1ede134c516..373157bd034 100644 --- a/app/models/course_user.rb +++ b/app/models/course_user.rb @@ -39,32 +39,30 @@ class CourseUser < ActiveRecord::Base # Sums the total experience points for the course user. # Default value is 0 when CourseUser does not have Course::ExperiencePointsRecord calculated :experience_points, (lambda do - Course::ExperiencePointsRecord.select do - coalesce(sum(course_experience_points_records.points_awarded), 0) - end.where { course_experience_points_records.course_user_id == course_users.id } + Course::ExperiencePointsRecord.selecting { coalesce(sum(points_awarded), 0) }. + where('course_experience_points_records.course_user_id = course_users.id') end) # @!attribute [r] last_experience_points_record # Returns the time of the last awarded experience points record. calculated :last_experience_points_record, (lambda do Course::ExperiencePointsRecord.select(:awarded_at).limit(1).order(awarded_at: :desc). - where do - (course_experience_points_records.course_user_id == course_users.id) & (awarded_at != nil) - end + where('course_experience_points_records.course_user_id = course_users.id'). + where('course_experience_points_records.awarded_at IS NOT NULL') end) # @!attribute [r] achievement_count # Returns the total number of achievements obtained by CourseUser in this course calculated :achievement_count, (lambda do - Course::UserAchievement.select { count('*') }. - where { course_user_achievements.course_user_id == course_users.id } + Course::UserAchievement.select("count('*')"). + where('course_user_achievements.course_user_id = course_users.id') end) # @!attribute [r] last_obtained_achievement # Returns the time of the last obtained achievement calculated :last_obtained_achievement, (lambda do Course::UserAchievement.select(:obtained_at).limit(1).order(obtained_at: :desc). - where { course_user_achievements.course_user_id == course_users.id } + where('course_user_achievements.course_user_id = course_users.id') end) # Gets the staff associated with the course. @@ -135,8 +133,8 @@ def real_student? # @return[Array] def my_students my_groups = group_users.manager.select(:group_id) - CourseUser.joins { group_users.group }.merge(Course::GroupUser.normal). - where { group_users.group.id >> my_groups } + CourseUser.joining { group_users.group }.merge(Course::GroupUser.normal). + where.has { group_users.group.id.in(my_groups) } end # Returns the managers of the groups I belong to in the course. @@ -144,8 +142,8 @@ def my_students # @return[Array] def my_managers my_groups = group_users.select(:group_id) - CourseUser.joins { group_users.group }.merge(Course::GroupUser.manager). - where { group_users.group.id >> my_groups } + CourseUser.joining { group_users.group }.merge(Course::GroupUser.manager). + where.has { group_users.group.id.in(my_groups) } end private diff --git a/app/models/instance.rb b/app/models/instance.rb index 0676a68f22e..4a1515ff9a0 100644 --- a/app/models/instance.rb +++ b/app/models/instance.rb @@ -20,7 +20,7 @@ def default # as www) are not handled automatically. # @return [Instance] def find_tenant_by_host(host) - where { lower(self.host) == lower(host) }.take + where.has { self.host.lower == host.downcase }.take end # Finds the given tenant by host, falling back to the default is none is found. @@ -29,8 +29,8 @@ def find_tenant_by_host(host) # as www) are not handled automatically. # @return [Instance] def find_tenant_by_host_or_default(host) - tenants = where do - (lower(self.host) == lower(host)) | (id == DEFAULT_INSTANCE_ID) + tenants = where.has do + (self.host.lower == host.downcase) | (id == DEFAULT_INSTANCE_ID) end.to_a tenants.find { |tenant| !tenant.default? } || tenants.first @@ -68,15 +68,13 @@ def find_tenant_by_host_or_default(host) # @!attribute [r] course_count # The number of courses in the instance. calculated :course_count, (lambda do - Course.unscoped.where { courses.instance_id == instances.id }. - select { count('*') } + Course.unscoped.where('courses.instance_id = instances.id').select("count('*')") end) # @!attribute [r] user_count # The number of users in the instance. calculated :user_count, (lambda do - InstanceUser.unscoped.where { instance_users.instance_id == instances.id }. - select { count('*') } + InstanceUser.unscoped.where('instance_users.instance_id = instances.id').select("count('*')") end) def self.use_relative_model_naming? diff --git a/app/models/instance_user.rb b/app/models/instance_user.rb index ee2265ba893..3a71f47c412 100644 --- a/app/models/instance_user.rb +++ b/app/models/instance_user.rb @@ -9,6 +9,6 @@ class InstanceUser < ActiveRecord::Base scope :ordered_by_username, -> { joins(:user).merge(User.order(name: :asc)) } def self.search_and_ordered_by_username(keyword) - keyword.blank? ? ordered_by_username : search(keyword).group { user.name }.ordered_by_username + keyword.blank? ? ordered_by_username : search(keyword).group('users.name').ordered_by_username end end diff --git a/app/services/concerns/course/user_invitation_service/email_invitation_concern.rb b/app/services/concerns/course/user_invitation_service/email_invitation_concern.rb index ec51dbcee6b..549ee958fa4 100644 --- a/app/services/concerns/course/user_invitation_service/email_invitation_concern.rb +++ b/app/services/concerns/course/user_invitation_service/email_invitation_concern.rb @@ -125,8 +125,9 @@ def augment_user_objects(users) # @param [Array] email_addresses An array of email addresses to query. # @return [Hash{String=>User}] The mapping from email address to users. def find_existing_users(email_addresses) - found_users = @current_instance.users.includes(:emails).references(emails: :email). - where { emails.email.in(email_addresses) } + # TODO: Move this search query into the +User+ model. + found_users = @current_instance.users.includes(:emails).joins(:emails). + where('user_emails.email IN (?)', email_addresses) found_users.each.flat_map do |user| user.emails.map { |user_email| [user_email.email, user] } @@ -181,7 +182,7 @@ def resend_invitation_emails(invitations) Course::Mailer.user_invitation_email(@current_course, invitation).deliver_later end ids = invitations.select(&:id) - Course::UserInvitation.where { id >> ids }.update_all(sent_at: Time.zone.now) + Course::UserInvitation.where(id: ids).update_all(sent_at: Time.zone.now) true end diff --git a/app/services/course/material/preload_service.rb b/app/services/course/material/preload_service.rb index f443a8d690d..df66340f5d6 100644 --- a/app/services/course/material/preload_service.rb +++ b/app/services/course/material/preload_service.rb @@ -22,6 +22,7 @@ def folders_for_assessment_hash def assessments_folders @assessments_folders ||= - @course.material_folders.where { owner_type == Course::Assessment.name }.includes(:materials) + @course.material_folders.includes(:materials). + where('course_material_folders.owner_type = ?', Course::Assessment.name) end end diff --git a/lib/extensions/polyglot_with_database/coursemology/polyglot/language.rb b/lib/extensions/polyglot_with_database/coursemology/polyglot/language.rb index d37904aa591..4b58a106c15 100644 --- a/lib/extensions/polyglot_with_database/coursemology/polyglot/language.rb +++ b/lib/extensions/polyglot_with_database/coursemology/polyglot/language.rb @@ -26,7 +26,7 @@ module Extensions::PolyglotWithDatabase::Coursemology::Polyglot::Language if !languages || languages.empty? all else - where { name.in(languages) } + where(name: languages) end end) end diff --git a/lib/extensions/time_bounded_record/active_record/base.rb b/lib/extensions/time_bounded_record/active_record/base.rb index 534eb31d4fd..5ef1867fe68 100644 --- a/lib/extensions/time_bounded_record/active_record/base.rb +++ b/lib/extensions/time_bounded_record/active_record/base.rb @@ -8,11 +8,11 @@ def currently_active private def started - where { (start_at == nil) | (start_at <= Time.zone.now) } + where.has { (start_at == nil) | (start_at <= Time.zone.now) } end def ended - where { (end_at == nil) | (end_at >= Time.zone.now) } + where.has { (end_at == nil) | (end_at >= Time.zone.now) } end end diff --git a/spec/features/course/lesson_plan_milestone_management_spec.rb b/spec/features/course/lesson_plan_milestone_management_spec.rb index 26624de7977..202a09a5694 100644 --- a/spec/features/course/lesson_plan_milestone_management_spec.rb +++ b/spec/features/course/lesson_plan_milestone_management_spec.rb @@ -6,7 +6,11 @@ with_tenant(:instance) do let!(:course) { create(:course) } - let(:milestones) { create_list(:course_lesson_plan_milestone, 2, course: course) } + let(:milestones) do + [2.days.ago, 2.days.from_now].map do |start_at| + create(:course_lesson_plan_milestone, course: course, start_at: start_at) + end + end before do login_as(user, scope: :user) diff --git a/spec/features/course/lesson_plan_spec.rb b/spec/features/course/lesson_plan_spec.rb index 417a711ebe6..6a2f249e57c 100644 --- a/spec/features/course/lesson_plan_spec.rb +++ b/spec/features/course/lesson_plan_spec.rb @@ -2,7 +2,6 @@ require 'rails_helper' RSpec.feature 'Course: Lesson Plan' do - subject { page } let!(:instance) { Instance.default } with_tenant(:instance) do @@ -32,10 +31,6 @@ end end - let!(:assessments) do - create_list(:course_assessment_assessment, 1, course: course) - end - before do login_as(user, scope: :user) end @@ -45,13 +40,11 @@ scenario 'I can view all lesson plan items and milestones', js: true do visit course_lesson_plan_path(course) - milestones.each do |m| - expect(subject).to have_text(m.title) - end + expect(page).to have_link(nil, href: new_course_lesson_plan_event_path(course)) + expect(page).to have_link(nil, href: new_course_lesson_plan_milestone_path(course)) - events.each do |item| - expect(subject).to have_text(item.title) - end + milestones.each { |milestone| expect(page).to have_text(milestone.title) } + events.each { |event| expect(page).to have_text(event.title) } end end @@ -69,13 +62,8 @@ expect(page).not_to have_link(nil, href: new_course_lesson_plan_event_path(course)) expect(page).not_to have_link(nil, href: new_course_lesson_plan_milestone_path(course)) - milestones.each do |m| - expect(subject).to have_text(m.title) - end - - events.each do |item| - expect(subject).to have_text(item.title) - end + milestones.each { |milestone| expect(page).to have_text(milestone.title) } + events.each { |event| expect(page).to have_text(event.title) } end end end diff --git a/spec/libraries/date_time_helpers.rb b/spec/libraries/date_time_helpers.rb index 0fbd726680d..76443213384 100644 --- a/spec/libraries/date_time_helpers.rb +++ b/spec/libraries/date_time_helpers.rb @@ -4,13 +4,13 @@ RSpec.describe Extensions::DateTimeHelpers do describe '.min' do it 'is a valid time in database' do - expect { User.where { created_at > Time.min } }.not_to raise_error + expect { User.where.has { created_at > Time.min } }.not_to raise_error end end describe '.max' do it 'is a valid time in database' do - expect { User.where { created_at < Time.max } }.not_to raise_error + expect { User.where.has { created_at < Time.max } }.not_to raise_error end end end