Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix pre-survey bug #35179

Merged
merged 2 commits into from
Jun 5, 2020
Merged

Fix pre-survey bug #35179

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b41af8c
fix pre survey bug
molly-moen Jun 5, 2020
b0803b4
Merge remote-tracking branch 'origin/staging' into molly/fix-pre-surv…
molly-moen Jun 5, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 20 additions & 1 deletion dashboard/app/controllers/pd/workshop_daily_survey_controller.rb
View file
Open in desktop
Expand Up @@ -132,7 +132,9 @@ def new_post_foorm
def new_general_foorm(survey_names, day:)
# We may be redirecting from our legacy route which uses enrollment_code
enrollment_code = params[:enrollmentCode] || params[:enrollment_code]
workshop = get_workshop_for_new_general(enrollment_code, current_user)
workshop = day == 0 ?
get_workshop_for_pre_survey(enrollment_code, current_user) :
get_workshop_for_new_general(enrollment_code, current_user)
unless validate_enrolled(workshop)
return
end
Expand Down Expand Up @@ -509,6 +511,9 @@ def key_params
end
end

# Either get workshop for the given enrollment_code or look up
# the nearest workshop the user attended or was enrolled in, in that priority
# order.
def get_workshop_for_new_general(enrollment_code, current_user)
if enrollment_code
Pd::Enrollment.find_by!(code: enrollment_code).workshop
Expand All @@ -520,6 +525,20 @@ def get_workshop_for_new_general(enrollment_code, current_user)
end
end

# Pre surveys should be filled out by a teacher without attendance so
# look up nearest workshop that the current_user is enrolled in if
# enrollment_code is not given.
def get_workshop_for_pre_survey(enrollment_code, current_user)
if enrollment_code
Pd::Enrollment.find_by!(code: enrollment_code).workshop
else
Workshop.
where(course: [COURSE_CSD, COURSE_CSP]).
where.not(subject: SUBJECT_FIT).
nearest_enrolled_in_by(current_user)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the nearest_attended_or_enrolled_in_by method poorly named? I would have thought that method does what you'd like (probably what you thought as well when you used it :).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's more stringent than named really. First it looks for the nearest workshop with attendance, then if it can't find one it looks for the nearest enrolled one. So a bit of a confusing name, yes

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think such a change is probably out of scope for this bug fix, but maybe nearest_attended_else_enrolled_in_by? :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be clear I'm talking about the old method, not your new one :) I guess I still don't understand, was the problem that the survey failed for teachers who have attended some other workshop in the past?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah the issue was if a teacher had attended another workshop previously it was trying to load a survey which didn't exist (ex. we don't have a foorm survey for academic year workshops). This was only an issue for the pre-survey because for everything else the teacher should have been marked attended for this workshop.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 got it thanks!

end
end

def get_session_for_workshop_and_day(workshop, day)
day > 0 ? workshop.sessions[day - 1] : nil
end
Expand Down
8 changes: 8 additions & 0 deletions dashboard/app/models/pd/workshop.rb
View file
Open in desktop
Expand Up @@ -296,6 +296,14 @@ def self.nearest_attended_or_enrolled_in_by(teacher)
current_scope.with_nearest_attendance_by(teacher) || current_scope.enrolled_in_by(teacher).nearest
end

# Find the workshop with the closest session to today
# enrolled in by the given teacher.
# @param [User] teacher
# @return [Pd::Workshop, nil]
def self.nearest_enrolled_in_by(teacher)
current_scope.enrolled_in_by(teacher).nearest
end

def course_name
COURSE_NAME_OVERRIDES[course] || course
end
Expand Down