lennym/moment-business-time

unexpected behavior when adding large number of hours

Closed this issue · 12 comments

mentheosis commented

adding a large number of hours has unexpected results, for example:

estimate = moment().addWorkingTime(108, 'hours')

returns a date much less than 108 business hours away. I guess the intended usage is to only use hours for <= 24 and use days above that, however when I first saw the result I thought the library was bugged and almost did not trust it. It would be better to throw an error if the input > 24 or something rather than just output bad result

lennym commented

Thanks for raising. I'll take a look.

Guria commented

ping. maybe some help needed?

Guria commented

I have wrong output already on 19 hours

lennym commented

Hi @Guria - there is an open PR that half fixes this problem at #4. If you'd like to finish it off so it covers both addition and subtraction then I'll be happy to merge.

lennym commented

@Guria Could you provide your test case?

Guria commented

Would be glad to drill down it, but also busy atm. I'll try to see it deeper if it will not solved till the end of month.

lennym commented

@Guria That URL doesn't seem to work for me - it has a broken redirect. Could you possibly either post a github gist, or just put the code inline in a comment if it's small?

Guria commented

Sorry have no much time right now. Hope tonic will fix it soon.

Guria commented

updated a link.
here it is inlined:

var chai = require('chai');
global.should = chai.should();

var moment = require("moment-business-time")
var date = moment('2016-03-21 15:30')
var format = 'YYYYMMDDHHmm'

date.clone().addWorkingTime(8,  'hour').format(format).should.equal('201603221530') // ok
date.clone().addWorkingTime(16, 'hour').format(format).should.equal('201603231530') // ok
date.clone().addWorkingTime(17, 'hour').format(format).should.equal('201603231630') // ok
date.clone().addWorkingTime(18, 'hour').format(format).should.equal('201603240930') // expected '201603220930' to equal '201603240930'

console.log('passed')
lennym commented

Thanks. I'll take a look.

lennym commented

Resolved at v0.4.1

Thanks for your test cases @Guria

  • 🎉1