Cannot list sessions for a given reviewer #191
Comments
|
@hugocorbucci I really don't know rails so well like you and others, but if the AR querying API is limiting a little bit we could introduce a view on the database and map it to a new model as a temporary workaround until finding a permanent solution. It is really not pretty, but it may be a fast way to solve the problem. |
|
#192 makes the count of the number of sessions to review wrong (it combines the groups). |
|
As I said before, I'm don't know ruby nor rails, but I don't see why something like this would be so terrible (as an attachable and easy to remove workaround): CREATE VIEW session_for_review AS
SELECT
sessions.id AS sessions_id,
COUNT(*) AS count_all,
sessions.final_reviews_count,
sessions.conference_id,
sessions.state,
sessions.author_id,
sessions.second_author_id,
sessions.track_id,
sessions.audience_level_id,
sessions.created_at,
reviews.reviewer_id,
reviews.type,
COUNT(reviews.id) AS matching_reviews
FROM sessions
LEFT OUTER JOIN reviews
ON sessions.id = reviews.session_id
GROUP BY sessions.id
HAVING matching_reviews = final_reviews_count;class SessionForReview < ActiveRecord::Base
# Mapping information and other things...
scope :for_conference, ->(conference) { where(conference_id: conference.id)}
scope :not_author, ->(u) {
where('author_id <> ? AND (second_author_id IS NULL OR second_author_id <> ?)', u.to_i, u.to_i)
}
scope :with_incomplete_final_reviews, -> { where('final_reviews_count < ?', 3) }
scope :submitted_before, ->(date) { where('created_at <= ?', date) }
scope :not_reviewed_by, ->(user, review_type) {
where('(reviewer_id IS NULL OR reviewer_id <> ?) AND type = ?', user.id, review_type)
}
scope :for_preferences, ->(*preferences) {
return none if preferences.empty?
clause = preferences.map { |p| "(track_id = ? AND audience_level_id <= ?)" }.join(" OR ")
args = preferences.map {|p| [p.track_id, p.audience_level_id]}.flatten
where(clause, *args)
}
def self.for_reviewer(user, conference)
sessions = for_review_in(conference).
not_author(user.id).
for_preferences(*user.preferences(conference)).
not_reviewed_by(user, conference.in_early_review_phase? ? 'EarlyReview' : 'FinalReview')
if conference.in_final_review_phase?
sessions.with_incomplete_final_reviews
else
sessions
end
end
def self.for_review_in(conference)
sessions = for_conference(conference).without_state(:cancelled) # I don't if this works =P
if conference.in_early_review_phase?
sessions.submitted_before(conference.presubmissions_deadline + 3.hours)
else
sessions
end
end
endI'm cloning the repo and will try something like this. |
|
Sorry guys, I won't be able to try that approach. I can't provision the bundled VM =/ Anyway... good luck there 😄 |
This one handles both the count and the actual usage so that pagination works. Added tests that break for mysql.
|
@lcobucci Mainly this is pretty hard to implement because of the coupling ActiveRecord imposes between your database and your code. To use your view, I'd have to identify anything that uses sessions in those contexts. |
|
@lcobucci Having said that, I have no idea why your VM is not provisioning correctly. I'd have to find that out. |
|
Perfect @hugocorbucci! I'm glad that the issue is solved. I'll open an issue for the VM with more information. |
After fixing #190, I discovered that listing the sessions that a given reviewer can review is no longer working.
This has not been caught by automated tests because sqlite3 doesn't apply the rules regarding group by and having clauses of SQL like MySQL does.
MySQL (https://dev.mysql.com/doc/refman/5.0/en/group-by-handling.html) doesn't allow a non-aggregated column (such as session.final_reviews_count) to be references in either a group_by or a having clause unless aliased somewhere else.
As a result, as soon as the reviewer_sessions page starts loading, the db query is performed and errors out with the following error:
The SQL fix is simple:
(Note the extra AS for reviews count at the select level)
Unfortunately, ActiveRecord builds the count call in https://github.com/rails/rails/blob/b88220732dffe336a0814bb91a4f5acc6e91e508/activerecord/lib/active_record/relation/calculations.rb#L300. When it does so, the only AS fields that are gathered are the ones of the group_by clause.
So we either have to find a way to change that query to remove the group_by and having clauses when counting (unlikely) or add the extra aliasing without changing the results.
This is likely an issue introduced by #114 as Rails 4 did a bunch of optimizing with the Arel queries which likely included simplifying the count queries.
The text was updated successfully, but these errors were encountered: