fix weekday of birthdays

[Laura-O]

This should fix #230.

This fix is not perfect as it ignores the possibility that a competition is taking place in two different years. However, it is unlikely that a competition spans over two years, so in general it should be working as expected. Feel free to ignore this PR if it doesn't meet your quality standards. :-)

[fw42]

Thanks for making a PR. The idea makes sense to me (and I think for now it's acceptable that this is wrong if the competition falls on the last day of the year and similar edge cases).

I think we can make it a bit cleaner by moving this logic from the view into the model.

The Competitor class could have an instance method that does this, something like:

def birthday_in_competition_year
  birthday.change(:year => competition.days.first.date.year)
end

and then the view could just call that new method.

Could you also add a unit test for this? (in test/models/competitor_test.rb)

[fw42]

Actually, I think we can make it even easier, by reusing the existing logic. Take a look at this:

def birthday_on_competition?
  days.any?{ |day| birthday_on?(day.date) }
end

which checks if the competitor has a birthday on one of the competition days (and returns true if so and false otherwise). We can reuse the same logic by changing any? to detect (which would make it return the competition day that is the competitor's birthday, which means this will also automatically work for your edge-case):

def birthday_on_competition
  days.detect{ |day| birthday_on?(day.date) }
end

def birthday_on_competition?
  !!birthday_on_competition
end

or something like that.

Does that make sense?

[Laura-O]

Changed the files now and everything is working as expected.

I am a bit unsure about the unit test - is this enough or do we need to test more?

[fw42]

One comment about the test. The rest looks good.

Thanks Laura! 👍

[Laura-O]

Awesome! Added back the old test and I think I understood why that makes sense! 😄