1078243 - [Costcontrol] When the Starting day of the reset period is 31th, the app does not calculated correctly the end of the reset period.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Cost Control, defect)

Description

[Marina Rodríguez [:mai]]

If the user selects on the Data Report as "Starting on" a day that does not exists all the months. The reset day is not calculated correctly the months that the selected day does not exist.

STR.
1. On settings App (Date & time section), change the date to February 6.
2. Open costcontrol App, open settings screen.
3. Change on the "Starting on" select, the reset day to 31.
4. Press "Done" button, on settings screen.

Current behaviour:
  When the graphic is reloaded, the upper limit of the graphic is "2nd March"
Expected behaviour:
 The upper limit must be "1st March"

Comment 1

[Marina Rodríguez [:mai]]

Attachment: patch v1.0

Hi Salva, 
would you mind reviewing the patch?
Regards.

Comment 2

[Salvador de la Puente González [:salva]]

Since I agree with you on the expected behavior in comment 0, this is a UX problem as we lack from specification about selecting dates not present in all months: IMHO, we should not allow to select 29, 30 or 31 as starting date but at "the end of the month" and I'm not really sure what company is establishing the beginning of the billing period in day not present in all months.

Comment 3

[Salvador de la Puente González [:salva]]

Comment on attachment 8506855 [details] [review]
patch v1.0

Well though I don't even agree with the solution adopted. Notice this refer to "Starting date" so this means I want to count since that day. This means if I set the date on 15, I want to count intervals from [15, 15), i.e, from current month's 15th to next month's 14th, both included. The next month 15th is the first day of the next period.

So, understanding dates 29, 30 or 31 as "the end of the month", I want to count from current month's end to next month's day before end in such a way the latest day of the current month is included in the current period but not the last date of the following month as it is part of the next period (just the same as in the former example).

This way, when selecting any of 29, 30 or 31, I want to see the period starting from 29, 30 and 31 respectively and ending one day before the last day of the next month, **not** the first.

Comment 5

[Rachelle Yang [:ryang][[e-mail]]]

Hi Marina,

Per comment3, Would you please share your thoughts and kindly provide us oyur revised patch ?

Thank you very much!!

Comment 6

[Juwei Huang[:juwei]]

Agree most of the points with Salvador.

Normally, if the start date is set as the 1st of every month, then the bar chart shows the first day to the last day of the month (Ex. Feb 1 to Feb 28). If the start date is set as 15th of every month, then the bar chart shows the 15th day on this month to the 14th day on next month (Ex. Feb15 to Mar 14)
Follow this logic, once the start date is set as 31st of every month, which is the last day, then the chart should show the last day on this month and the day before last day on next month (Ex. Feb 28 to Mar 30).
It means the start day must always be the last day of the month.

So the answer to this issue should be Jan 31 to Feb 27. Then the next billing cycle can start from Feb 28, which is the last day of the month.

Comment 7

[Marina Rodríguez [:mai]]

Hi Rachelle,
IMO, this is a very strange use case, and I don't know a company that billing on those days of the month. Anyway, the two possibilities that I see when it does not exist the reset day, they are the following:
1. mark as reset Day the first day of the following month at the resetDay
2. mark as resetDay the last day of the month.

The scenario that Juwei and Salva recomend is the second, I'm going to show some scenarios to clarify the recomended posibility:

Reset Report starting on: 31
1. Base case (No problem with date limits)
    Today is Jan 15
    Resetdate = Jan 31 at 0:00
    The chart should show "Dec 31 to Jan 30"

#############################################
2. Not exits the end period day 

    Today is Feb 15
    Resetdate = Feb 28 at 0:00
    The chart should show "Jan 31 to Feb 27"
_____________________________________________

    Today is Apr 13    
    Resetdate = Apr 30 at 0:00
    The chart should show "Mar 31 to Apr 29"    

#############################################
3. Not exits the init period day 
    Today is March 15
    Resetdate = Feb 31 at 0:00
    The chart should show "Feb 28 to March 30"
_____________________________________________

    Today is May 3    
    Resetdate = May 31 at 0:00
    The chart should show "Apr 30 to May 30"

Regards

Comment 8

[Marina Rodríguez [:mai]]

Comment on attachment 8506855 [details] [review]
patch v1.0

Hi Salva,
I've updated the pr with the solution proposed. Would you mind taking a look?
Regards

Comment 9

[Salvador de la Puente González [:salva]]

Comment on attachment 8506855 [details] [review]
patch v1.0

The logic looks ok, the tests are passing but the bug is tricky enough to push for a refactor and improve the readability of the code. Let's address the comments on GitHub and ask for my review again.

Comment 10

[Marina Rodríguez [:mai]]

Comment on attachment 8506855 [details] [review]
patch v1.0

Updated the pr with your comments

Comment 11

[Salvador de la Puente González [:salva]]

Comment on attachment 8506855 [details] [review]
patch v1.0

There is some kind of problem with the chart. See what I'm referring in the following attachment.

Comment 12

[Salvador de la Puente González [:salva]]

Attachment: chart-density.png

STR:

 1. Make reset-gaia with your patch
 2. Pass the FTE
 3. Observe the chart

Expected:
As today is Nov, 12th, the period should start at Nov, 1st.

Actual:
The period starts at Oct, 1st

Try to catch this problem with the tests you're providing and fix it.

Thank you!

Comment 13

[Marina Rodríguez [:mai]]

Comment on attachment 8506855 [details] [review]
patch v1.0

Update the pr, sorry I've forgotten to check this base case. Added a test to cover this scenario.
Regards

Comment 14

[Salvador de la Puente González [:salva]]

Comment on attachment 8506855 [details] [review]
patch v1.0

Nice! Thank you Marina.

Comment 15

[Marina Rodríguez [:mai]]

Master: 8db0c8bbe2099d6dafec94b480207f805f4e6a56

Comment 16

[vsireesha246]

Attachment: Bug_1078243_V2.0.patch

Hi Mai,

I made the V2.0 patch for this issue(duplicate Bug-1074052).
Bug-1074052 previous your patch(Attachment 8499414 [details]) having problem of as mentioned by Salva in https://bugzilla.mozilla.org/show_bug.cgi?id=1078243#c12
So, i need to retake your latest patch in my local branch.

So would you mind to review and let me know your feedback.

Thanks..
Sireesha

Comment 17

[Marina Rodríguez [:mai]]

Comment on attachment 8524297 [details] [diff] [review]
Bug_1078243_V2.0.patch

Review of attachment 8524297 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM,
Thanks Sireesha.