Fix: Workshops without subjects have exit surveys #29718
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
In particular, workshops with a `NULL` subject will still be included in the `with_surveys` enrollment scope, and therefore will remind about pending exit surveys on the professional learning landing page. Fixes a regression introduced in #29511 when I added a condition to the `with_surveys` scope, not realizing it would implicitly exclude null values as well. Did some work to evaluate impact, and it looks like we've had no workshops with `NULL` subject end in the week or so since I introduced this regression, so overall impact is near-zero. Still, I'd like to get it fixed before I move on to disabling exit surveys for Facilitator workshops (which also have a `NULL` subject).
breville
reviewed
Jul 15, 2019
| @@ -120,7 +120,11 @@ def self.for_school_district(school_district) | |||
| # Except for FiT workshops - no exit surveys for them! | |||
| # This scope is used in ProfessionalLearningLandingController to direct the teacher | |||
| # to their latest pending survey. | |||
| scope :with_surveys, -> {for_ended_workshops.attended.where.not(pd_workshops: {subject: SUBJECT_FIT})} | |||
| scope :with_surveys, (lambda do | |||
There was a problem hiding this comment.
Just curious: is the switch to lambda purely to get multi-line code support?
There was a problem hiding this comment.
Yeah; Rubocop prefers curlies for one-lines, and do ... end for multiline, and it turns out lambda syntax is tied to one or the other.
breville
approved these changes
Jul 16, 2019
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.
In particular, workshops with a
NULLsubject will still be included in thewith_surveysenrollment scope, and therefore will remind about pending exit surveys on the professional learning landing page.Fixes a regression introduced in #29511 when I added a condition to the
with_surveysscope, not realizing it would implicitly exclude null values as well.Did some work to evaluate impact, and it looks like we've had no workshops with
NULLsubject end in the week or so since I introduced this regression, so overall impact is near-zero. Still, I'd like to get it fixed before I move on to disabling exit surveys for Facilitator workshops (which also have aNULLsubject).