From e443e6b848bb1c92ce19d5aa0f4ca95f35305fdb Mon Sep 17 00:00:00 2001
From: Rhys Arkins <rhys@keylocation.sg>
Date: Wed, 23 Aug 2017 12:38:47 +0200
Subject: [PATCH] fix: only migrate 'and' schedule if it includes before and
 after (#724)

---
 docs/faq.md                                      |  4 ++--
 lib/config/migration.js                          |  6 +++++-
 test/config/__snapshots__/migration.spec.js.snap |  8 +++++++-
 test/config/migration.spec.js                    | 15 ++++++++++++++-
 test/workers/branch/schedule.spec.js             |  5 +++++
 5 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/docs/faq.md b/docs/faq.md
index 93bf9ad9e0..de5b181ab2 100644
--- a/docs/faq.md
+++ b/docs/faq.md
@@ -61,8 +61,8 @@ Example scheduling:
 ```
 every weekend
 before 5:00am
-after 10pm and before 5:00am
-after 10pm and before 5am every weekday
+[after 10pm, before 5:00am]
+[after 10pm every weekday, before 5am every weekday]
 on friday and saturday
 ```
 
diff --git a/lib/config/migration.js b/lib/config/migration.js
index cb395816ac..73399a8f25 100644
--- a/lib/config/migration.js
+++ b/lib/config/migration.js
@@ -80,7 +80,11 @@ function migrateConfig(config, parentConfig) {
       let schedules = typeof val === 'string' ? [val] : val;
       // split 'and'
       for (let i = 0; i < schedules.length; i += 1) {
-        if (schedules[i].indexOf(' and ') !== -1) {
+        if (
+          schedules[i].includes(' and ') &&
+          schedules[i].includes('before ') &&
+          schedules[i].includes('after ')
+        ) {
           isMigrated = true;
           const split = schedules[i].split(' and ');
           schedules[i] = split[0];
diff --git a/test/config/__snapshots__/migration.spec.js.snap b/test/config/__snapshots__/migration.spec.js.snap
index 6c540b2342..3f8ee96f27 100644
--- a/test/config/__snapshots__/migration.spec.js.snap
+++ b/test/config/__snapshots__/migration.spec.js.snap
@@ -1,5 +1,11 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`config/migration migrateConfig(config, parentConfig) does not migrate multi days 1`] = `
+Object {
+  "schedule": "after 5:00pm on wednesday and thursday",
+}
+`;
+
 exports[`config/migration migrateConfig(config, parentConfig) it migrates config 1`] = `
 Object {
   "autodiscover": true,
@@ -101,7 +107,7 @@ Object {
 }
 `;
 
-exports[`config/migration migrateConfig(config, parentConfig) migrates schedules with and 1`] = `
+exports[`config/migration migrateConfig(config, parentConfig) migrates before and after schedules 1`] = `
 Object {
   "schedule": Array [
     "after 10pm",
diff --git a/test/config/migration.spec.js b/test/config/migration.spec.js
index 289f4760a4..cd6c99472f 100644
--- a/test/config/migration.spec.js
+++ b/test/config/migration.spec.js
@@ -57,7 +57,7 @@ describe('config/migration', () => {
       expect(migratedConfig.automerge).toEqual(false);
       expect(migratedConfig).toMatchSnapshot();
     });
-    it('migrates schedules with and', () => {
+    it('migrates before and after schedules', () => {
       const config = {
         schedule: 'after 10pm and before 7am',
       };
@@ -72,6 +72,19 @@ describe('config/migration', () => {
       expect(migratedConfig.schedule[0]).toEqual('after 10pm');
       expect(migratedConfig.schedule[1]).toEqual('before 7am');
     });
+    it('does not migrate multi days', () => {
+      const config = {
+        schedule: 'after 5:00pm on wednesday and thursday',
+      };
+      const parentConfig = { ...defaultConfig };
+      const { isMigrated, migratedConfig } = configMigration.migrateConfig(
+        config,
+        parentConfig
+      );
+      expect(migratedConfig).toMatchSnapshot();
+      expect(isMigrated).toBe(false);
+      expect(migratedConfig.schedule).toEqual(config.schedule);
+    });
     it('it migrates packages', () => {
       const config = {
         packages: [
diff --git a/test/workers/branch/schedule.spec.js b/test/workers/branch/schedule.spec.js
index 8b09c38b1a..6e5e8659ec 100644
--- a/test/workers/branch/schedule.spec.js
+++ b/test/workers/branch/schedule.spec.js
@@ -38,6 +38,11 @@ describe('workers/branch/schedule', () => {
         true
       );
     });
+    it('returns true for multi day schedules', () => {
+      expect(
+        schedule.hasValidSchedule(['after 5:00pm on wednesday and thursday'])[0]
+      ).toBe(true);
+    });
     it('returns true if schedule has a start time', () => {
       expect(schedule.hasValidSchedule(['after 8:00pm'])[0]).toBe(true);
     });
-- 
GitLab