Remove squeel queries from codebase #2283
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
houndci-bot
reviewed
May 11, 2017
| end | ||
|
|
||
| def ended | ||
| where { (end_at == nil) | (end_at >= Time.zone.now) } | ||
| where.has { (end_at == nil) | (end_at >= Time.zone.now) } |
There was a problem hiding this comment.
Prefer the use of the nil? predicate.
| @@ -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) } | |||
There was a problem hiding this comment.
Prefer the use of the nil? predicate.
| 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) |
There was a problem hiding this comment.
Inconsistent indentation detected.
| @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) |
There was a problem hiding this comment.
Align page with @experience_points_records.active.includes(:actable, :updater).order(updated_at: :desc). on line 16.
weiqingtoh
force-pushed
the
weiqingtoh/baby-squeel
branch
from
238aacd
to
766f7c6
Compare
- Previously, renderComponent(response.data) was called before the document was ready.
weiqingtoh
force-pushed
the
weiqingtoh/baby-squeel
branch
from
766f7c6
to
86fdabd
Compare
houndci-bot
reviewed
May 11, 2017
| 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) |
There was a problem hiding this comment.
Use 2 (not 0) spaces for indenting an expression spanning multiple lines.
- 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.
weiqingtoh
force-pushed
the
weiqingtoh/baby-squeel
branch
from
86fdabd
to
ea1ac14
Compare
|
👍 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Starts #2271. Squeel is not being maintained now, hence removing all dependencies on it would be useful.
This PR removes all Squeel queries from Coursemology, and replaces them with vanilla ActiveRecord and baby_squeel (aimed at replacing Squeel, without monkey-patching ActiveRecord). For most parts, I've opted to stick to normal Activerecord to ensure easier upgrade paths in the future.
Note that however,
cancancan-squeel, which has a dependency on squeel, is still installed. This is because thecancancan-squeeladapter does fix certain issues with self-joins and calculated attributes which is still necessary for Coursemology to function. The next step is to work on an alternative for that adapter.