devfort/behabitual — pull request #42 by annashipman (2013-06-11 22:57:49+00:00)
Title: We shouldn't try and collect data from the future

This pull request addresses issue #40. I hadn't quite understood the problem when I raised the issue so to restate it here:

Be Habitual requests information for the current time period. This is not a major problem if the time period is a day, but if it's a week it means you get an email at 9.30 on Monday morning asking you how you have done, and the link takes you to a page which wants the upcoming week's data (i.e. future data).

This is nonsensical, but it's also problematic in that you are not permitted to enter only *some* data on that page. So if I want to enter last week's data only, I can't, I also need to enter data for this week, which hasn't happened yet. So I have to wait until the end of this week to enter last week's data, or set this week's data forever to 0.

1. I think the longer term solution for this is actually slightly more complex logic on the record progress page, such that you cannot leave the page without entering previous time periods' data but data on the current week does not have to be entered/can be altered. That would also address #39 and #36.

2. In the current absence of that more complex logic, we can default to either:
(a) having to enter the current period's data
(b) not being able to enter the current period's data
Currently, it is (a). This leads to the non-sensical situation I described above. Therefore, the changes in this PR changes the logic so it defaults instead to (b).

3. The logic change is here: dec0b825
It looks like the +1 is solely to ensure we include the current time period.

4. I have also had to change tests other than the one I changed to reflect the exact behaviour I was changing. Therefore I'd really appreciate a close eye on this from others - especially in case there are other areas that use this functionality that are not unit tested.

Paricularly @nickstenning and @ashb who worked on this file; and/or @georgebrock, @mnowster, @SteveMarshall, @jaylett, @jcoglan, or @norm.

Oh, and the other thing I haven't done is tested it locally as my local running Be Habitual falls over when I try and create a habit (on master as well as this branch). Tips on that welcome...

If this is merged whoever does that could also raise an issue for point 1?

Cheers! To Industry!