Code review comment for lp:~matsubara/lp-devops-dashboard/929538-incident-reports-record

Hi there

I have perhaps a dumb question - if we are intending to show the days since the last incident report, is the comparison used in the code the wrong way around?

89 + if current and ((today() - current) > overall_delta):
90 + overall_delta = today() - current

The above code will end up finding the number of days since the oldest incident report I think? Where as we want the number of days since the latest incident report, so it should be:

if current and ((today() - current) < overall_delta):

The attribute used to hold the delta value, "most_days_with_no_incidents" seems incorrectly named now that we are changing the semantics of the calculation. It should now really be called "days_since_last_incident".

If what I say above is correct, then the tests are incorrect, eg:

183 + def test_get_last_reports_since(self):
184 + incidentreports._today = datetime(2011, 11, 9)
185 + self.ir.get_last_reports_since()
186 + self.assertEqual(self.ir.total_incidents, 7)
187 + self.assertEqual(self.ir.most_days_with_no_incidents, 89)
188 + self.assertEqual(self.ir.last_report, '2011-11-07-LP-some-error')

The latest incident report in the sample data is 2011-11-07 and "today" is 2011-11-09, so I would expect the days since last incident report to be 2.