From 8f8a4de69f8427c474cfd605d50686f2924e9ea6 Mon Sep 17 00:00:00 2001
From: Rhys Arkins <rhys@keylocation.sg>
Date: Wed, 2 Aug 2017 16:14:09 +0200
Subject: [PATCH] fix: Schedule should always be an array (#580)

Massaging of string to array is done in migration function.
---
 docs/configuration.md                            |  2 +-
 docs/faq.md                                      |  6 ++++--
 lib/config/definitions.js                        |  2 +-
 lib/config/migration.js                          |  3 +++
 lib/config/validation.js                         |  5 +----
 lib/workers/branch/schedule.js                   |  9 +++------
 test/config/__snapshots__/index.spec.js.snap     |  4 +++-
 test/config/__snapshots__/migration.spec.js.snap |  3 +++
 test/config/index.spec.js                        |  4 ++--
 test/config/migration.spec.js                    |  1 +
 test/config/validation.spec.js                   |  2 +-
 test/workers/branch/schedule.spec.js             | 12 ++++++------
 12 files changed, 29 insertions(+), 24 deletions(-)

diff --git a/docs/configuration.md b/docs/configuration.md
index ee0dc18522..3fff9b9474 100644
--- a/docs/configuration.md
+++ b/docs/configuration.md
@@ -539,7 +539,7 @@ Obviously, you can't set repository or package file location with this method.
   "commitMessage": "{{semanticPrefix}}Update lock file",
   "prTitle": "{{semanticPrefix}}Lock file maintenance",
   "prBody": "This {{#if isGitHub}}Pull{{else}}Merge{{/if}} Request updates `package.json` lock files to use the latest dependency versions.\n\n{{#if schedule}}\n**Note**: This PR was created on a configured schedule (\"{{schedule}}\"{{#if timezone}} in timezone `{{timezone}}`{{/if}}) and will not receive updates outside those times.\n{{/if}}\n\n{{#if hasErrors}}\n\n---\n\n### Errors\n\nRenovate encountered some errors when processing your repository, so you are being notified here even if they do not directly apply to this PR.\n\n{{#each errors as |error|}}\n-   `{{error.depName}}`: {{error.message}}\n{{/each}}\n{{/if}}\n\n{{#if hasWarnings}}\n\n---\n\n### Warnings\n\nPlease make sure the following warnings are safe to ignore:\n\n{{#each warnings as |warning|}}\n-   `{{warning.depName}}`: {{warning.message}}\n{{/each}}\n{{/if}}\n\n---\n\nThis {{#if isGitHub}}PR{{else}}MR{{/if}} has been generated by [Renovate Bot](https://renovateapp.com).",
-  "schedule": "before 5am on monday"
+  "schedule": ["before 5am on monday"]
 }</pre></td>
   <td>`RENOVATE_LOCK_FILE_MAINTENANCE`</td>
   <td><td>
diff --git a/docs/faq.md b/docs/faq.md
index b9f46e07c4..80f2d3f862 100644
--- a/docs/faq.md
+++ b/docs/faq.md
@@ -74,11 +74,13 @@ To restrict `aws-sdk` to only weekly updates, you could add this package rule:
   "packages": [
     {
       "packageName": "aws-sdk",
-      "schedule": "after 9pm on sunday"
+      "schedule": ["after 9pm on sunday"]
     }
   ]
 ```
 
+Note that schedule must be in the form of an array, even if only one schedule is present. Multiple entries in the array means "or".
+
 ### Selectively enable or disable renovate for specific `package.json` files
 
 You could:
@@ -104,7 +106,7 @@ Set configuration option `pinVersions` to `false`.
 
 ### Keep `yarn.lock` sub-dependencies up-to-date, even when `package.json` hasn't changed
 
-This is enabled by default, but its schedule is set to 'before 5am on monday'. If you want it more frequently, then update the `schedule` field inside the `lockFileMaintenance` object.
+This is enabled by default, but its schedule is set to `['before 5am on monday']`. If you want it more frequently, then update the `schedule` field inside the `lockFileMaintenance` object.
 
 ### Wait until tests have passed before creating the PR
 
diff --git a/lib/config/definitions.js b/lib/config/definitions.js
index 4db8770bad..25bd51866e 100644
--- a/lib/config/definitions.js
+++ b/lib/config/definitions.js
@@ -393,7 +393,7 @@ const options = [
       commitMessage: template('commitMessage', 'lock-file-maintenance'),
       prTitle: template('prTitle', 'lock-file-maintenance'),
       prBody: template('prBody', 'lock-file-maintenance'),
-      schedule: 'before 5am on monday',
+      schedule: ['before 5am on monday'],
     },
     cli: false,
     mergeable: true,
diff --git a/lib/config/migration.js b/lib/config/migration.js
index eb2376f8dc..473a0fe75e 100644
--- a/lib/config/migration.js
+++ b/lib/config/migration.js
@@ -24,6 +24,9 @@ function migrateConfig(config) {
     if (removedOptions.includes(key)) {
       isMigrated = true;
       delete migratedConfig[key];
+    } else if (key === 'schedule' && typeof val === 'string') {
+      isMigrated = true;
+      migratedConfig.schedule = [val];
     } else if (key === 'depTypes' && Array.isArray(val)) {
       val.forEach(depType => {
         if (isObject(depType)) {
diff --git a/lib/config/validation.js b/lib/config/validation.js
index 41de8f0edb..50620cf24e 100644
--- a/lib/config/validation.js
+++ b/lib/config/validation.js
@@ -32,7 +32,7 @@ function validateConfig(config) {
   }
 
   for (const key of Object.keys(config)) {
-    let val = config[key];
+    const val = config[key];
     if (
       !isIgnored(key) && // We need to ignore some reserved keys
       !isAFunction(val) // Ignore all functions
@@ -52,9 +52,6 @@ function validateConfig(config) {
             });
           }
         } else if (type === 'list') {
-          if (key === 'schedule' && typeof val === 'string') {
-            val = [val];
-          }
           if (!Array.isArray(val)) {
             errors.push({
               depName: 'Configuration Error',
diff --git a/lib/workers/branch/schedule.js b/lib/workers/branch/schedule.js
index 2d31641592..e5d2478227 100644
--- a/lib/workers/branch/schedule.js
+++ b/lib/workers/branch/schedule.js
@@ -47,10 +47,7 @@ function hasValidSchedule(schedule, logger) {
 
 function isScheduledNow(config) {
   config.logger.debug({ schedule: config.schedule }, `Checking schedule`);
-  // Massage into array
-  const configSchedule =
-    typeof config.schedule === 'string' ? [config.schedule] : config.schedule;
-  if (!module.exports.hasValidSchedule(configSchedule, config.logger)) {
+  if (!module.exports.hasValidSchedule(config.schedule, config.logger)) {
     // Return true if the schedule is invalid
     return true;
   }
@@ -70,9 +67,9 @@ function isScheduledNow(config) {
     now.hours() * 3600 + now.minutes() * 60 + now.seconds();
   config.logger.debug(`currentSeconds=${currentSeconds}`);
   // Support a single string but massage to array for processing
-  config.logger.debug(`Checking ${configSchedule.length} schedule(s)`);
+  config.logger.debug(`Checking ${config.schedule.length} schedule(s)`);
   // We run if any schedule matches
-  const isWithinSchedule = configSchedule.some(scheduleText => {
+  const isWithinSchedule = config.schedule.some(scheduleText => {
     config.logger.debug(`Checking schedule "${scheduleText}"`);
     const parsedSchedule = later.parse.text(fixShortHours(scheduleText));
     // Later library returns array of schedules
diff --git a/test/config/__snapshots__/index.spec.js.snap b/test/config/__snapshots__/index.spec.js.snap
index b0d69c50a6..db210bdd93 100644
--- a/test/config/__snapshots__/index.spec.js.snap
+++ b/test/config/__snapshots__/index.spec.js.snap
@@ -43,7 +43,9 @@ Please make sure the following warnings are safe to ignore:
 This {{#if isGitHub}}PR{{else}}MR{{/if}} has been generated by [Renovate Bot](https://renovateapp.com).",
   "prTitle": "{{semanticPrefix}}Lock file maintenance",
   "recreateClosed": true,
-  "schedule": "on monday",
+  "schedule": Array [
+    "on monday",
+  ],
 }
 `;
 
diff --git a/test/config/__snapshots__/migration.spec.js.snap b/test/config/__snapshots__/migration.spec.js.snap
index ef00800e7f..e7eebe8194 100644
--- a/test/config/__snapshots__/migration.spec.js.snap
+++ b/test/config/__snapshots__/migration.spec.js.snap
@@ -6,6 +6,9 @@ Object {
   "optionalDependencies": Object {
     "respectLatest": false,
   },
+  "schedule": Array [
+    "after 5pm",
+  ],
 }
 `;
 
diff --git a/test/config/index.spec.js b/test/config/index.spec.js
index 5a23ceb0ea..2f0bb4ccbe 100644
--- a/test/config/index.spec.js
+++ b/test/config/index.spec.js
@@ -152,14 +152,14 @@ describe('config/index', () => {
         foo: 'bar',
         pinVersions: false,
         lockFileMaintenance: {
-          schedule: 'on monday',
+          schedule: ['on monday'],
         },
       };
       const configParser = require('../../lib/config/index.js');
       const config = configParser.mergeChildConfig(parentConfig, childConfig);
       expect(config.foo).toEqual('bar');
       expect(config.pinVersions).toBe(false);
-      expect(config.lockFileMaintenance.schedule).toEqual('on monday');
+      expect(config.lockFileMaintenance.schedule).toEqual(['on monday']);
       expect(config.lockFileMaintenance).toMatchSnapshot();
     });
     it('merges depTypes', () => {
diff --git a/test/config/migration.spec.js b/test/config/migration.spec.js
index 4e9db9c9a2..5e3f8034f4 100644
--- a/test/config/migration.spec.js
+++ b/test/config/migration.spec.js
@@ -6,6 +6,7 @@ describe('config/migration', () => {
       const config = {
         enabled: true,
         maintainYarnLock: true,
+        schedule: 'after 5pm',
         depTypes: [
           'dependencies',
           {
diff --git a/test/config/validation.spec.js b/test/config/validation.spec.js
index 2e99501e08..a5ef8d43db 100644
--- a/test/config/validation.spec.js
+++ b/test/config/validation.spec.js
@@ -5,7 +5,7 @@ describe('config/validation', () => {
     it('returns nested errors', () => {
       const config = {
         foo: 1,
-        schedule: 'after 5pm',
+        schedule: ['after 5pm'],
         prBody: 'some-body',
         lockFileMaintenance: {
           bar: 2,
diff --git a/test/workers/branch/schedule.spec.js b/test/workers/branch/schedule.spec.js
index 742a72cb22..98789f23c3 100644
--- a/test/workers/branch/schedule.spec.js
+++ b/test/workers/branch/schedule.spec.js
@@ -87,22 +87,22 @@ describe('workers/branch/schedule', () => {
       expect(res).toBe(true);
     });
     it('supports before hours true', () => {
-      config.schedule = 'before 4:00pm';
+      config.schedule = ['before 4:00pm'];
       const res = schedule.isScheduledNow(config);
       expect(res).toBe(true);
     });
     it('supports before hours false', () => {
-      config.schedule = 'before 4:00am';
+      config.schedule = ['before 4:00am'];
       const res = schedule.isScheduledNow(config);
       expect(res).toBe(false);
     });
     it('supports outside hours', () => {
-      config.schedule = 'after 4:00pm';
+      config.schedule = ['after 4:00pm'];
       const res = schedule.isScheduledNow(config);
       expect(res).toBe(false);
     });
     it('supports timezone', () => {
-      config.schedule = 'after 4:00pm';
+      config.schedule = ['after 4:00pm'];
       config.timezone = 'Asia/Singapore';
       const res = schedule.isScheduledNow(config);
       expect(res).toBe(true);
@@ -113,12 +113,12 @@ describe('workers/branch/schedule', () => {
       expect(res).toBe(true);
     });
     it('supports day match', () => {
-      config.schedule = 'on friday and saturday';
+      config.schedule = ['on friday and saturday'];
       const res = schedule.isScheduledNow(config);
       expect(res).toBe(true);
     });
     it('supports day mismatch', () => {
-      config.schedule = 'on monday and tuesday';
+      config.schedule = ['on monday and tuesday'];
       const res = schedule.isScheduledNow(config);
       expect(res).toBe(false);
     });
-- 
GitLab