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.

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

PL: support multi-select when viewing JotForm survey results #28441

Merged

Conversation

breville
Copy link
Member

@breville breville commented May 9, 2019

This adds support for multi-select answers when viewing workshop per-day JotForm survey results. After a long day of diving into this code, I'm surprised at how small the fix is, and am also not yet sure whether it's the right or complete fix.

But here's why the simple addition of a .flatten seems to have worked. Almost everything came down to this line in the controller:

summary = surveys_for_session[response_section].map {|survey| survey[q_key]}.group_by {|v| v}.transform_values(&:size)

For these examples, we'll take surveys_for_session[response_section] to be this, which represents two surveys, each with the same four questions answered:

[{"type_of_last_pd"=>
   ["I completed an online course/webinar.",
    "I participated in a professional learning community/lesson study/teacher study group.",
    "I received assistance or feedback from a formally designated coach/mentor.",
    "I took a formal course for college credit."],
  "reasons_for_no_PD"=>["Did not have school/admin support", "my other text"],
  "wouldYou220"=>"No, thanks.",
  "permission"=>"No, I do not give permission to quote me."},
 {"type_of_last_pd"=>
   ["I completed an online course/webinar.",
    "I participated in a professional learning community/lesson study/teacher study group.",
    "I received assistance or feedback from a formally designated coach/mentor."],
  "reasons_for_no_PD"=>["Did not have financial support", "second other text"],
  "wouldYou220"=>"No, thanks.",
  "permission"=>"No, I do not give permission to quote me."}]

For a single-select, the transformation worked like this:

r.map {|survey| survey["permission"]}
=> ["No, I do not give permission to quote me.", "No, I do not give permission to quote me."]

r.map {|survey| survey["permission"]}.group_by {|v| v}
=> {"No, I do not give permission to quote me."=>["No, I do not give permission to quote me.", "No, I do not give permission to quote me."]}

r.map {|survey| survey["permission"]}.group_by {|v| v}.transform_values(&:size)
=> {"No, I do not give permission to quote me."=>2}

For a multi-select, the same transformation worked like this:

r.map {|survey| survey["reasons_for_no_PD"]}
=> [["Did not have school/admin support", "my other text"], ["Did not have financial support", "second other text"]]

r.map {|survey| survey["reasons_for_no_PD"]}.group_by {|v| v}
=> {["Did not have school/admin support", "my other text"]=>[["Did not have school/admin support", "my other text"]], ["Did not have financial support", "second other text"]=>[["Did not have financial support", "second other text"]]}

r.map {|survey| survey["reasons_for_no_PD"]}.group_by {|v| v}.transform_values(&:size)
=> {["Did not have school/admin support", "my other text"]=>1, ["Did not have financial support", "second other text"]=>1}

which was not what we wanted, and didn't group answers properly on the client.

But with the addition of the .flatten, we see this:

r.map {|survey| survey["reasons_for_no_PD"]}.flatten
=> ["Did not have school/admin support", "my other text", "Did not have financial support", "second other text"]

r.map {|survey| survey["reasons_for_no_PD"]}.flatten.group_by {|v| v}
=> {"Did not have school/admin support"=>["Did not have school/admin support"], "my other text"=>["my other text"], "Did not have financial support"=>["Did not have financial support"], "second other text"=>["second other text"]}

r.map {|survey| survey["reasons_for_no_PD"]}.flatten.group_by {|v| v}.transform_values(&:size)
=> {"Did not have school/admin support"=>1, "my other text"=>1, "Did not have financial support"=>1, "second other text"=>1}

This looks closer to what we're after.

Taking a look at the view for this data at /pd/workshop_dashboard/daily_survey_results/ID, with only a small change made to render SingleChoiceResponses for multiSelect answers as well, things amazingly seem to work with no further changes:

Screenshot 2019-05-08 19 05 00

Except the eagle-eyed will notice that the percentages aren't right here, because the logic for singleSelect questions looked at the number of answers supplied by one respondent divided by the number of answers supplied by all respondents. In the case of multiSelect, we want to divide by the number of respondents to that question, rather than the number of answers. A second commit handles this:

Screenshot 2019-05-10 12 09 09

Copy link
Contributor

@islemaster islemaster left a comment

Choose a reason for hiding this comment

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

Wow! Thanks for the great explanation of this change, and excellent find! Is there any place we could add a test case for this, even just over the one line you changed? (If not, maybe we can sprout a method from that line and test it in isolation.)

Copy link

@clareconstantine clareconstantine left a comment

Choose a reason for hiding this comment

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

Niiiice! I can imagine how long this took to unravel based on my previous forays into this code jungle. I agree that some testing would be nice but I feel good about this change!

@agealy
Copy link

agealy commented May 9, 2019

I wonder if the percentage calculations should work differently for multi-select questions. I would normally expect these to sum to over 100% for multi-select and represent the % of users who chose that as an answer. So if there are 2 people responding to the "Within the last 3 years..." question, the "I completed an online course/webinar" should read as 100%. I don't understand the use case well enough to say for sure right now

@breville
Copy link
Member Author

I've updated this PR with a second commit that fixes the percentages for multi-select questions. The description has also been updated to describe this. The most complex logic has also been extracted into its own function and is now unit-tested.

@breville breville requested review from clareconstantine and islemaster and removed request for clareconstantine May 13, 2019 16:36
@breville breville merged commit 4dd53ad into staging May 13, 2019
@breville breville deleted the pl-support-multi-select-when-viewing-jotform-survey-results branch May 13, 2019 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants