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 calculation of businessDiff ? #89

Closed epaillous closed 3 years ago

epaillous commented 3 years ago

Hi ! First of all thank you for this awesome library :) I was happy to discover I would not have to reimplement everything about business days for my project :)

I am trying to show the difference in working days between two dates: startDate and endDate. Let's say startDate is 2021-02-15 (monday), and endDate is 2021-02-20 (saturday)

Actual behaviour is :
endDate.businessDiff(startDate) => 5

Expected behaviour is : endDate.businessDiff(startDate) => 4

Since saturday is not a business day, the difference in working days between monday and saturday should be the same as the difference in working days between monday and friday, ie 4 days.

Also, if startDate is 2021-02-15 (monday) and end Date is 2021-02-22 (monday), the difference is correct, endDate.businessDiff(startDate) => 5

I was expected that businessDiff would give the same result when endDate is friday / saturday / sunday.

Is it a bug or is it the expected behaviour ?

Thank you!

mcdado commented 3 years ago

Hello, I'm not the maintainer but I contributed quite a bit to this project.

It's an interesting question, because answering would depend on whom you ask!

In my company's use case, we don't use businessDiff, we mainly use businessAdd to calculate "when it this order expected to be ready?". In that use case, if I say add 5 days and the fifth day is a Saturday, I expect to receive the next Monday (or the next working day according to configuration) as a result.

But in the case of businessDiff, it's a different question: how many business days between these two days (including the start and end date)? In that case I believe it should be 4, like you said.

I'll setup some tests and figure out what exactly is the current behavior and what should be the right one.

epaillous commented 3 years ago

To give some context about my usecase : I am doing a form to allow product managers to know:

So, there are 3 inputs on my form :

When I change the duration, the end date should be computed from the new duration When I change the end date, the duration should be computed from the new endDate When I change the start date, the duration should be computed from the new startDate

Also, if startDate and endDate are the same, I need :

So I am using both businessDiff and businessAdd at the same time

mcdado commented 3 years ago

Good evening @epaillous

You were right: when the latter date is on a non-business day, businessDiff() would count up one more day.

Regarding your use case, I think you simply need to add 1 to the result of businessDiff() because it doesn't count the first day, and it never did, but does count the end date.

@kalmecak I think you can release this change. It is breaking an expectation, but a buggy one. If you plan to bump a major version, please let me know because in that case I'd drop the relative parameter of businessDiff() and make it the default behavior.

mcdado commented 3 years ago

In the meantime you can test it out by using a github url as a dependency: https://docs.npmjs.com/cli/v7/configuring-npm/package-json#github-urls