nmrf / sadata

Samoa's Human Rights National Mechanism for Reporting and Follow-up (NMRF) - Client-side application
Other
0 stars 0 forks source link

Not all due dates shown when storing schedule information on indicator #39

Closed tmfrnz closed 5 years ago

tmfrnz commented 5 years ago

@ashbowe reported (via email)

There seems to be something funny happening with the report due dates when I'm adding indicators. I thought I noticed the dates not appearing in the implementation plan view initially but haven't been able to replicate it yet. However, with some indicators it is not reflecting the full range of reports due. E.g. indicator 237 - I have set the first report as due May 2018 (so already overdue) and then repeating annually. This is the same as for indicators 235 and 236 yet the first report is not reflected when viewing the implementation plan, see below:

implplan

it also happens for dates in the future too. E.g. Indicator 246. Might it be because there is no user assigned to the indicator? I don't think that always appears to be the case though

tmfrnz commented 5 years ago

Investigating:

tmfrnz commented 5 years ago

Having had a look at the code, it turns out that

only future due dates are generated for 'repeat indicators' (see L34)

but

past due dates are generated for 'one-off indicators' (see L40)

However when updating an indicator, more specifically its reporting schedule,

only future due dates are deleted and re-generated on update. (see L47)

This is what I suspect must have happened to indicators 235 and 236 where at first only a single date was defined and stored and subsequently they were updated to include multiple dates, generating all future dates but without deleting the original date that is already in the past.

Indicator 237 must have been created with a range of dates from the start.

tmfrnz commented 5 years ago

I understand that this behaviour is unexpected and hard for the user to grasp. Therefore we should either

  1. allow past due dates also for 'repeat indicators' (as we do for one-offs), or
  2. validate that due dates have to be in the future and provide the user with a corresponding error message.

My recommendation would be to do (1) that would give more flexibility but also responsibility to the user.

What do you think @ashbowe?

tmfrnz commented 5 years ago

For a temporary workaround you can create a past due date by

  1. disable the repeat option for the indicator and save (to generate the past due date)
  2. then re-enable the repeat option and save for all future due dates
ashbowe commented 5 years ago

I understand that this behaviour is unexpected and hard for the user to grasp. Therefore we should either

  1. allow past due dates also for 'repeat indicators' (as we do for one-offs), or
  2. validate that due dates have to be in the future and provide the user with a corresponding error message.

My recommendation would be to do (1) that would give more flexibility but also responsibility to the user.

What do you think @ashbowe?

Hi Timo - yes option 1 definitely

ashbowe commented 5 years ago

For a temporary workaround you can create a past due date by

  1. disable the repeat option for the indicator and save (to generate the past due date)
  2. then re-enable the repeat option and save for all future due dates

Sounds good

tmfrnz commented 5 years ago

Fixed BUT new due dates are not shown right away... https://github.com/nmrf/sadata/issues/40