AYW Survey Links #36307
AYW Survey Links #36307
Conversation
| protected | ||
|
|
||
| def get_attended_workshop_by_enrollment_or_course(enrollment_code, course, subject) | ||
| attended_workshop = nil | ||
| def get_workshop_by_enrollment_or_course(enrollment_code, course, subject, should_have_attended=true) |
There was a problem hiding this comment.
This is pretty logically tricky and I don't se any unit tests, might be good to add as a follow up?
There was a problem hiding this comment.
good call, I can add it now since it's used multiple places
There was a problem hiding this comment.
Writing it out to wrap my head around it...
If an enrollment code is provided, we search for a workshop by that enrollment code. If that enrollment code is invalid, we render invalid enrollment code.
If the current user attended the workshop we found, or we don't care whether the user attended the workshop (I don't understand this part), we set this workshop as the workshop to return.
If no enrollment code is provided, we get all workshops that this user has enrolled in. If they haven't enrolled in any workshops, we render not enrolled.
If they have enrolled in workshops AND we care that they attended the workshop, we set the workshop to return to the most recent attended workshop; otherwise, we return the most recent workshop they're enrolled in.
If we have no workshops at this point, then we couldn't find a workshop with attendance for this user.
There was a problem hiding this comment.
I'll add a comment explaining this :) The should_have_attended addition covers that for pre surveys we don't expect the user to be marked attended yet and it is not a prerequisite for them filling out the survey. For post surveys we do want the user to have been marked attended.
Scenarios this covers:
- user provided an enrollment code-we find the enrollment with that code. If they need to have attended, we verify the user attended the workshop.
- user did not provide an enrollment code: we find the most workshops the user has enrolled in. If we don't care about attendance, we find the most recent workshop. If we do care about attendance, we find the most recent workshop that they were marked as attended.
If at the end the workshop_to_return was blank we know it's because there was no attendance, because we already checked that there were possible workshops they were enrolled in.
There was a problem hiding this comment.
turns out its hard to add a test for a helper method in a controller in isolation--I'll update the comment here and make sure the unit tests for this method exercise the functionality
There was a problem hiding this comment.
I think I've found that in the past, too, and have just hoisted functions into the model where they are much easier to test. (And therefore probably belong there :)
There was a problem hiding this comment.
👍 I will refactor this as a follow-up (there's a lot in here that I think could be refactored to the model but it's out of scope for this pr)
dashboard/app/controllers/api/v1/pd/workshop_survey_foorm_submissions_controller.rb
Outdated
Show resolved
Hide resolved
| @@ -41,7 +41,9 @@ def self.parse_forms(forms) | |||
| parsed_forms = {general: {}, facilitator: {}} | |||
| forms.each do |form| | |||
| parsed_form = {general: {}, facilitator: {}} | |||
| form_questions = JSON.parse(form.questions, symbolize_names: true) | |||
| form_questions = JSON.parse(form.questions) | |||
| form_questions = ::Foorm::Form.fill_in_library_items(form_questions) | |||
There was a problem hiding this comment.
this was to allow us to see results from forms with library items--we weren't filling in library items in the parser before so any library questions would not have been able to be viewed (which is many questions in the new surveys).
| SUBJECT_CSF_201 => 'surveys/pd/csf_deep_dive_post' | ||
| SUBJECT_CSF_201 => 'surveys/pd/csf_deep_dive_post', | ||
| # TODO: update with rest of AYW subjects | ||
| SUBJECT_WORKSHOP_4 => 'surveys/pd/ayw_workshop_post_survey' |
There was a problem hiding this comment.
I like this workaround to allow testing :)
| elsif survey_key.starts_with? "Post Workshop - Module" | ||
| # last character will be the module number, use that as an index | ||
| return 1000 + survey_key[survey_key.length - 1].to_i | ||
| else |
There was a problem hiding this comment.
What is the else supposed to catch here? Won't this be the same value as post-workshop for module 1?
There was a problem hiding this comment.
this would be the case of in_person agenda, which I should just combine with "Post Workshop", since both would not exist at the same time. Module 1 and in person would never exist for the same workshop to your question specifically.
| @@ -24,12 +24,35 @@ def get_survey_key(ws_submission) | |||
| def get_index_for_survey_key(survey_key) | |||
There was a problem hiding this comment.
I guess this whole method seems a little suspect for the long term, how are you thinking about it? Works in the short term and we figure something else out later?
There was a problem hiding this comment.
yeah I'm open to suggestions on making it better. Basically we need it to sort the surveys by (1) Pre surveys (sorted by module if applicable) (2) daily surveys (sorted by day) (3) Post surveys (sorted by module if applicable). (This sorts the tabs for the results view).
There was a problem hiding this comment.
Got it, seems like it'll work in the short term.
There was a problem hiding this comment.
cleaned this method up and added unit tests for this file
Add support in Foorm for academic year workshop survey links. This PR sets up the links below (see spec linked below for more information). It also enables viewing of these links on the workshop_daily_survey_results page.
Pre surveys:
/pd/AYWX/pre
/pd/AYWX_Y/pre
/pd/AYWX/pre/module/y
/od/AYWX/pre/module/y_z
Daily Surveys (only used for in person 2-day workshop):
/pd/AYWX_Y/day/z
Post Surveys:
/pd/AYWX/post/in_person
/pd/AYWX_Y/post/in_person
/pd/AYWX/post/module/y
/pd/AYWX/post/module/y_z
/pd/kickoff/post
The links also make use of the new
workshop_agendavariable, which will bein_person,modulex,modulex_y(ex module1 or module1_2), ornull.Currently only AYW4 links will work so that we don't expose surveys currently being edited that have a risk of being used in the next few days. See future work for work required after this PR.
Future work
workshop_survey_foorm_constants)Links
Testing story
Tested the new urls locally--that they show the expected surveys and store in the database as expected. Also verified the new results view worked. Checked that 5-day summer surveys were not impacted by any changes made here.
Reviewer Checklist: