Calculation logic to determine number of months in period is incorrect

Description

There is a bug in determining the number of months in a processing period/schedule. Currently this code is in the reference data service in the ProcessingScheduleController.getTotalDifference(). Some issues:

  • The "Period" domain object should not have any "getTotalDifference" calculations. We don't understand why any method is called "getTotalDifference()" because there is not a difference calculation to be done. Instead it should have a method to return a rounded whole number of months (eg 2 months or 3 months or N months).

  • This code currently calculates the difference according to all periods in the system, not just a single period. This code currently calculates according to months and days, but it should only do months. Every period lives within its one schedule. The calculation we should be doing is how many months are in a period--the duration of the period. This should use rounding logic to determine how many months a period is. Many calculations that depend on this are done based on number of doses per month. So we expect the domain logic should have a way to determine how many months (in round integers) any period is long.

  • The logic should probably be in the ProcessingPeriod/ProcessingSchedule domain layer, rather than in the controller.

  • The minimum duration returned by the method is 1 month (even for the shortest possible period)

  • For the remaining cases, the period duration is rounded to the closest number of whole months, which means a month and a half and more is rounded up to 2 months and less than a month and a half is rounded down to 1 month.

I have set the test that tests this method to Ignore. Once this bug is fixed, the integration test will need to be fixed (as well as unit tests that will need to be added).

Acceptance Criteria

  • Before starting implementation, coordinate with Chongsun about the design for this. Get his review/input before coding.

  • The new logic is in the domain layer; the old logic in the controller is removed.

  • Integration tests are re-enabled and passing/working.

Environment

None

relates to

QAlity Plus - Test Management

Checklists

Activity

Lucyna LaskaNovember 9, 2016 at 2:07 PM

I tested this ticket and all works.

Test steps:
1. Create period with startDate=2017-01-01 and endDate=2017-01-02.
2. Check the duration of the period above (server return 1 month).
3. Create period with startDate=2017-01-03 and endDate=2017-01-17.
4. Check the duration of the period above (server return 1 month).
5. Create period with startDate=2017-01-18 and endDate=2017-02-02.
6. Check the duration of the period above (server return 1 month).
7. Create period with startDate=2017-02-03 and endDate=2017-03-19.
8. Check the duration of the period above (server return 2 months).
9. Create some extreme period with startDate=2018-05-19 and endDate=2055-05-18.
10. Check the duration of the period above (server return 444 months).

For more details, see test case: https://openlmis.atlassian.net/wiki/x/mYCfBQ

Chongsun AhnNovember 7, 2016 at 4:20 PM

This is correct.

Sebastian BrudzińskiNovember 7, 2016 at 12:40 PM

Did you mean that the method should always return at least one month? I'm assuming yes, since when used in calculations you cannot divide by 0. I'm updating ticket acceptance criteria with this, but please edit if my reasoning is incorrect.

Chongsun AhnNovember 4, 2016 at 6:13 PM

I believe there is a one month minimum.

Jakub HopenNovember 4, 2016 at 9:52 AM

in unlikely event of period lasting less than 15 days, should it be treated as one month or 0 months?

Done
Pinned fields
Click on the next to a field label to start pinning.

Details

Assignee

Reporter

Story Points

Time tracking

3d 30m logged

Sprint

Fix versions

Affects versions

Priority

Time Assistant

Created October 4, 2016 at 8:18 PM
Updated November 9, 2016 at 2:10 PM
Resolved November 9, 2016 at 2:09 PM