eduolalo / moment-business-days

This is a momentJS plugin that allows you to use only business days (Monday to Friday)
MIT License
240 stars 67 forks source link

businessDiff not working as expected #95

Closed mattm closed 3 years ago

mattm commented 3 years ago

In the following code, April 5th (a Monday) is the start date and we're measuring business days to several dates later that week:

moment("2021-04-05").businessDiff(moment("2021-04-09")); // Friday
moment("2021-04-05").businessDiff(moment("2021-04-10")); // Saturday
moment("2021-04-05").businessDiff(moment("2021-04-11")); // Sunday

All of these return 4, but I would expect them to return 5 because Monday through Friday are business days.

Thanks for this plugin, it's fantastic, just having trouble wrapping my head around this behavior.

mcdado commented 3 years ago

Imagine the difference between today (an imaginary working Monday) and tomorrow (Tuesday). What should be the difference? 1 or 2?

In this case businessDiff is the difference between dates, and in that sense it doesn't include the starting date. Also, since this plugin discards hours, it only considers days as a whole. If you wanted to consider the hour or simply always count the first day, I would suggest to add 1 to the result of businessDiff.

Dr1Ku commented 3 years ago

Also see the relevant test here, what tripped me up was a businessDiff between an imaginary non-working Saturday/Sunday and an also-imaginary working Friday was 4 and not 5, as David mentioned.

Also be sure to check your moment's locale (although you can rely on sensible defaults).

tscislo commented 1 year ago

@mcdado @mattm @Dr1Ku with version 1.2.0 I get different results

import moment from "moment-business-days";

console.log(moment("2021-04-05").businessDiff(moment("2021-04-09"))); // Friday // 4 console.log(moment("2021-04-05").businessDiff(moment("2021-04-10"))); // Saturday // 5 console.log(moment("2021-04-05").businessDiff(moment("2021-04-11"))); // Sunday // 5 In my opinion it should be always 4, what is wrong?

Also I made sure my locale is correct moment.updateLocale('us', { workingWeekdays: [1, 2, 3, 4, 5] });

console.log(moment('2021-04-10', 'YYYY-MM-DD').isBusinessDay()); // false console.log(moment('2021-04-11', 'YYYY-MM-DD').isBusinessDay()); // false