search

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

wrong businessDiff calculation when the date is not a businessday #96

Closed lcaouen closed 3 years ago

lcaouen commented 3 years ago

When using businessDiff with bank holidays, if the date is not a businessDay (is a bank holiday or weed en day), there is one extra day added wrongly. For instance, the difference between 24/07/2021 and 20/07/2021 is wrong. It should be 2 instead of 3. Same problem if the date is a bank holiday (in my case I added 22/07/2021 as a bank holiday), the difference between 22/07/2021 and 20/07/2021 should be 1 instead of 2.

Here are some other examples 2021-07-20 - 2021-07-20 => realDiff : 0 / businessDiff: 0 / isBusinessDay: true 2021-07-20 - 2021-07-21 => realDiff : 1 / businessDiff: 1 / isBusinessDay: true 2021-07-20 - 2021-07-22 => realDiff : 1 / businessDiff: 2 / isBusinessDay: false 2021-07-20 - 2021-07-23 => realDiff : 2 / businessDiff: 2 / isBusinessDay: true 2021-07-20 - 2021-07-24 => realDiff : 2 / businessDiff: 3 / isBusinessDay: false 2021-07-20 - 2021-07-25 => realDiff : 2 / businessDiff: 3 / isBusinessDay: false 2021-07-20 - 2021-07-26 => realDiff : 3 / businessDiff: 3 / isBusinessDay: true

To be correct, I have to decrease 1 day if it is not a businessday : const workingdays = date.isBusinessDay() ? date.businessDiff(latest) : date.businessDiff(latest) - 1;

Here is a codesandbox to reproduce it : https://codesandbox.io/s/node-js-forked-v9buj

Do you think you can correct it ?

mcdado commented 3 years ago

I just added your tests, and the results are correct.

Probably what happened is that the fix is already in master but not in the released version. @kalmecak if you could push a new release out it would be nice.

@lcaouen as an interim alternative, in your package.json file you could reference the Github url like indicated in npm's docs:

{
  "dependencies": {
    "moment-business-days": "kalmecak/moment-business-days#master",
  }
}