Workshop Daily Survey Controller Cleanup #36382
Conversation
|
Initial comment -- |
| end | ||
|
|
||
| def get_workshop_by_enrollment(enrollment_code:, should_have_attended:) | ||
| workshop = Workshop.by_enrollment_code(enrollment_code, current_user) |
There was a problem hiding this comment.
I don't think this is much different than what you have verbosity-wise, but it seems unusual to me as a class method on the workshop model (also -- can you reference Workshops without the Pd:: prefix?). Could just do something like:
workshop = Pd::Enrollment.find_by(enrollment_code: enrollment_code, user: current_user)&.workshop
There was a problem hiding this comment.
Or, maybe just changing method name to find_by_enrollment_code would work as well.
There was a problem hiding this comment.
Yeah I was trying to move things out of the controller, but I can switch it back to what was there before (which was similar to what you had written there). The move out of the controller didn't end up being that much less code, and I only call it once.
Most of the surveys served by
workshop_daily_survey_controllernow use Foorm instead of JotForm (the exception is CSF Deep Dive, which is in the process of being moved over). This change cuts out all the JotForm-specific code and tests for CSD and CSP workshop types. It also refactors the common code for finding a workshop and displaying a survey inworkshop_daily_survey_controllerto be in a common method, as the logic was duplicated in multiple methods before. I also added unit tests for CSF Intro that were previously missing.There is a small change to the logic for finding the workshop for the given survey link to make everything consistent. Previously for post surveys we would find the workshop by looking first for the nearest workshop the teacher attended, and if they attended no workshops, the nearest workshop they were enrolled in. This meant if a teacher was enrolled but not attended in a recent workshop, but had previously attended a workshop they would either see a survey for that old workshop or a thank you message if they had already filled it out. If it was their first workshop they would see the post survey for the recent workshop they did not attend. Since we generally want to only show post-surveys for workshops teachers had enrolled in, and so teachers see the same behavior no matter how many workshops they have attended, we filter for attendance for all post surveys now. Due to this change I updated a couple unit tests to sign the teacher in before trying to access the post-survey.
Previously all CSD and CSP workshops shared the same survey urls. With the new Academic Year Workshop links the urls will no longer support AYW, which the Ed Programs team has signed off on.
Future work
Once we move CSF Deep Dive to Foorm we can remove all references to JotForm from this controller. We can also cut out more tests. We will need to add CSF Deep Dive tests for Foorm to the test file as well.
Testing story
After updating the unit tests, I ensured they all passed. I also manually tested with different workshop types to ensure accessing surveys still works as expected.
Reviewer Checklist: