Skip to content

Commit

Permalink
Replace squeel with baby_squeel as squeel is not maintained
Browse files Browse the repository at this point in the history
- 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.
  • Loading branch information
weiqingtoh committed May 11, 2017
1 parent a1e1591 commit ea1ac14
Show file tree
Hide file tree
Showing 42 changed files with 169 additions and 175 deletions.
4 changes: 2 additions & 2 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
5 changes: 3 additions & 2 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -630,7 +633,6 @@ DEPENDENCIES
simplecov
slim-rails
spring
squeel
summernote-rails
themes_on_rails (>= 0.3.1)!
traceroute
Expand All @@ -644,6 +646,5 @@ DEPENDENCIES
workflow
yard


BUNDLED WITH
1.14.6
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/course/lesson_plan/items_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def render_json_response
# @return [Hash{Integer => Array<String>]
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
Expand Down
6 changes: 5 additions & 1 deletion app/controllers/course/user_invitations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions app/models/concerns/course/forum_participation_concern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
6 changes: 3 additions & 3 deletions app/models/concerns/course/search_concern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion app/models/concerns/course_user/achievements_concern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
10 changes: 5 additions & 5 deletions app/models/concerns/course_user/staff_concern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
6 changes: 3 additions & 3 deletions app/models/concerns/instance_user_search_concern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
6 changes: 3 additions & 3 deletions app/models/concerns/user_search_concern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
3 changes: 1 addition & 2 deletions app/models/course.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 7 additions & 6 deletions app/models/course/assessment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions app/models/course/assessment/programming_evaluation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
#
Expand Down
19 changes: 10 additions & 9 deletions app/models/course/assessment/submission.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -84,21 +84,22 @@ class Course::Assessment::Submission < ActiveRecord::Base

# @!method self.by_users(user)
# @param [Fixnum|Array<Fixnum>] 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)

Expand All @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions app/models/course/assessment/submission_question.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion app/models/course/condition/achievement.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions app/models/course/condition/assessment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 6 additions & 5 deletions app/models/course/discussion/post.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down
6 changes: 2 additions & 4 deletions app/models/course/experience_points/disbursement.rb
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,7 @@ def filtered_students
# @param [Integer] group_id The id of the group
# @return [Array<CourseUser>] 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
8 changes: 5 additions & 3 deletions app/models/course/experience_points/forum_disbursement.rb
Original file line number Diff line number Diff line change
Expand Up @@ -144,10 +144,12 @@ def ranked_statistic_groups
# @return [Array<Course::Discussion::Post>]
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.
Expand Down
2 changes: 1 addition & 1 deletion app/models/course/experience_points_record.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.
#
Expand Down
Loading

0 comments on commit ea1ac14

Please sign in to comment.