Previous Business Day limited to only the last 7 days

[richarddavenport]

I setup a (most likely improbable situation, but you never know ¯_(ツ)_/¯) scenario. Today is the 10th, the 5th-9th are holidays, Monday is the only working day.

moment.updateLocale('us', {
   holidays: ['04-05-2019', '04-06-2019', '04-07-2019', '04-08-2019', '04-09-2019'],
   holidayFormat: 'MM-DD-YYYY',
   workingWeekdays: [1]
});

console.log(moment('04-10-2019', 'MM-DD-YYYY').prevBusinessDay().toString());
// "Thu Apr 04 2019 00:00:00 GMT-0500"

// expected:
// "Mon Apr 01 2019 00:00:00 GMT-0500"

jsbin: https://jsbin.com/vafaseriti/edit?js,console

Possible solution:

moment.updateLocale('us', {
   holidays: ['04-01-2019', '04-05-2019', '04-06-2019', '04-07-2019', '04-08-2019', '04-09-2019'],
   holidayFormat: 'MM-DD-YYYY',
   workingWeekdays: [1],
   prevBusinessDayLimit: 31,
});

moment.fn.prevBusinessDay = function () {
  var locale = this.localeData();

  var loop = 1;
  var defaultPrevBusinessDayLimit = 7;
  var limit = locale._prevBusinessDayLimit || defaultPrevBusinessDayLimit;
  while (loop < limit) {
    if (this.subtract(1, 'd').isBusinessDay()) {
      break;
    }
    loop++;
  }
  return this;
};

Open to PR's?

[mcdado]

Thanks for reporting! Wow I've never seen this! I'm trying to figure it out by looking at code that's untouched since the first commit b9b836a050905d38ae171e2839d6bfbaa9e3aeb6 and remember that I started contributing only some time ago.

[mcdado]

I've created https://github.com/kalmecak/moment-business-days/pull/68 but I'm not sure of implications. Basically I've added a hard limit of 1 year (366 days, considering leap years too) to avoid infinite loops in the imaginary case where all days that are working days (say Mondays) are also holidays. Is this an acceptable compromise?

[mcdado]

I've actually not read your suggested solution. Yes it could work with a parameter, in which case should have the same name for both nextBusinessDay and prevBusinessDay, something businessDayLimit, defaulting to either 7 or 31 or 366.

[richarddavenport]

We do have a use case for this on a project I'm working on. Here's what I have so far: https://github.com/richarddavenport/moment-business-days

[richarddavenport]

@mcdado I know that there is a WIP PR, but I went ahead and submitted my code changes for review. Let me know if you need any additional input/help. In #69 one of the tests are failing, but I believe it is due to my lack of understanding on how to reset the locale in mocha.

[mcdado]

@richarddavenport That's good! I actually like your approach better!

To fix the test probably you need to reset this module's custom properties in the updateLocale call:

var resetLocale = function (done) {
  moment.updateLocale('us', {
    holidays: [],
    holidayFormat: '',
    workingWeekdays: [1,2,3,4,5],
  });
  done();
};```

[richarddavenport]

@mcdado Does the version need to change for me to pull it in to my project?

[mcdado]

I think until @kalmecak publishes a new version you can put kalmecak/moment-business-days in your package.json file: https://docs.npmjs.com/files/package.json#github-urls

This should pull in the latest version, but i don't know for sure.

[mcdado]

Closed with #68.