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

Fix NoMethodError #26996

Merged
merged 1 commit into from Feb 11, 2019
Merged

Fix NoMethodError #26996

merged 1 commit into from Feb 11, 2019

Conversation

clareconstantine
Copy link

We were calling .reduce(:+) on an array of nil values, which caused the user to see a 500 error and endless spinner.

Fixed code rendering data that was previously erroring out:
screen shot 2019-02-11 at 12 43 09 pm

@clareconstantine clareconstantine requested review from islemaster and hacodeorg and removed request for islemaster and hacodeorg February 11, 2019 20:50
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.

👍 Nice job tracking this down.

I'll suggest an alternative approach, but up to you which you like better: .reject(:nil?).reduce(:+) might be more expressive, but wouldn't handle it if, for example, we started passing false in this array.

Seems like we should get this urgent fix out, but following up with a small regression test would be good.

@clareconstantine
Copy link
Author

Sounds good - it's taking me a little while to pare down the test data to only the relevant pieces.

@clareconstantine clareconstantine merged commit de9d2f7 into staging Feb 11, 2019
@@ -489,7 +489,7 @@ def generate_facilitator_averages(summary)
QUESTIONS_FOR_FACILITATOR_AVERAGES.each do |category, questions|
facilitator_averages[facilitator][category.to_s.downcase.to_sym] = {}
[:this_workshop, :all_my_workshops].each do |column|
average = (facilitator_averages[facilitator].slice(*(questions.map {|question| question[:primary_id]})).values.map {|x| x[column]}.reduce(:+) || 0) / questions.size.to_f
average = (facilitator_averages[facilitator].slice(*(questions.map {|question| question[:primary_id]})).values.map {|x| x[column]}.inject(0) {|sum, n| sum + (n || 0)} || 0) / questions.size.to_f
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the rename from reduce -> inject? My understanding is they are aliased to each other.

Would be great to clean up this (already very long line) into a more readable set of statements!

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

3 participants