From 5196ac8d32e28884456bd4f0d648ee675584bd73 Mon Sep 17 00:00:00 2001
From: Juan Cruz Viotti <jv@jviotti.com>
Date: Wed, 9 Aug 2017 08:43:19 -0700
Subject: [PATCH] refactor: simplify release type handling within the app
 (#1667)

As another step towards moving to GitHub Releases, this commit makes the
application care much less about the actual release type of the current
version, instead checking if the application is stable or not, which is
more aligned to what GitHub provides us.

Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
---
 lib/gui/app.js                        | 10 +++++-----
 lib/gui/components/update-notifier.js |  2 +-
 lib/shared/release.js                 |  6 +++++-
 tests/shared/release.spec.js          |  4 ++--
 tests/shared/s3-packages.spec.js      | 15 +++++++++++++--
 5 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/lib/gui/app.js b/lib/gui/app.js
index 87da1568..ddb22481 100644
--- a/lib/gui/app.js
+++ b/lib/gui/app.js
@@ -101,14 +101,14 @@ app.run(() => {
       lastSleptUpdateNotifierVersion: settings.get('lastSleptUpdateNotifierVersion')
     })
 
-    const currentReleaseType = release.getReleaseType(currentVersion)
+    const isStableRelease = release.isStableRelease(currentVersion)
     const updatesEnabled = settings.get('updatesEnabled')
 
     if (!shouldCheckForUpdates || !updatesEnabled) {
       analytics.logEvent('Not checking for updates', {
         shouldCheckForUpdates,
         updatesEnabled,
-        releaseType: currentReleaseType
+        stable: isStableRelease
       })
 
       return Bluebird.resolve()
@@ -119,12 +119,12 @@ app.run(() => {
 
     analytics.logEvent('Checking for updates', {
       currentVersion,
-      releaseType: currentReleaseType,
+      stable: isStableRelease,
       updateSemverRange,
       includeUnstableChannel
     })
 
-    return s3Packages.getLatestVersion(currentReleaseType, {
+    return s3Packages.getLatestVersion(release.getReleaseType(currentVersion), {
       range: updateSemverRange,
       includeUnstableChannel
     }).then((latestVersion) => {
@@ -152,7 +152,7 @@ app.run(() => {
       })
 
       return updateNotifier.notify(latestVersion, {
-        allowSleepUpdateCheck: currentReleaseType === release.RELEASE_TYPE.PRODUCTION
+        allowSleepUpdateCheck: isStableRelease
       })
 
     // If the error is an update user error, then we don't want
diff --git a/lib/gui/components/update-notifier.js b/lib/gui/components/update-notifier.js
index 9938ff74..0e7315c8 100644
--- a/lib/gui/components/update-notifier.js
+++ b/lib/gui/components/update-notifier.js
@@ -68,7 +68,7 @@ exports.shouldCheckForUpdates = (options) => {
 
   if (_.some([
     !options.lastSleptUpdateNotifier,
-    release.getReleaseType(options.currentVersion) !== release.RELEASE_TYPE.PRODUCTION,
+    !release.isStableRelease(options.currentVersion),
     options.currentVersion !== options.lastSleptUpdateNotifierVersion
   ])) {
     return true
diff --git a/lib/shared/release.js b/lib/shared/release.js
index a8b0f81c..39bcf2c1 100644
--- a/lib/shared/release.js
+++ b/lib/shared/release.js
@@ -99,5 +99,9 @@ exports.getReleaseType = (version) => {
  * }
  */
 exports.isStableRelease = (version) => {
-  return _.isEmpty(_.get(semver.parse(version), [ 'prerelease' ]))
+  const parsedVersion = semver.parse(version)
+  return _.every([
+    _.isEmpty(_.get(parsedVersion, [ 'prerelease' ])),
+    _.isEmpty(_.get(parsedVersion, [ 'build' ]))
+  ])
 }
diff --git a/tests/shared/release.spec.js b/tests/shared/release.spec.js
index 7e2aac92..5c38fdbf 100644
--- a/tests/shared/release.spec.js
+++ b/tests/shared/release.spec.js
@@ -101,8 +101,8 @@ describe('Shared: Release', function () {
       m.chai.expect(release.isStableRelease('1.0.0-beta.1')).to.be.false
     })
 
-    it('should return true if given a snapshot stable version', function () {
-      m.chai.expect(release.isStableRelease('1.0.0+6374412')).to.be.true
+    it('should return false if given a snapshot stable version', function () {
+      m.chai.expect(release.isStableRelease('1.0.0+6374412')).to.be.false
     })
 
     it('should return false if given a snapshot release candidate version', function () {
diff --git a/tests/shared/s3-packages.spec.js b/tests/shared/s3-packages.spec.js
index 5bfc18ea..07b88b81 100644
--- a/tests/shared/s3-packages.spec.js
+++ b/tests/shared/s3-packages.spec.js
@@ -916,12 +916,23 @@ describe('Shared: s3Packages', function () {
         this.getRemoteVersionsStub.restore()
       })
 
-      it('should ignore production versions', function (done) {
-        s3Packages.getLatestVersion(release.RELEASE_TYPE.SNAPSHOT).then((latestVersion) => {
+      it('should ignore production versions if includeUnstableChannel is true', function (done) {
+        s3Packages.getLatestVersion(release.RELEASE_TYPE.SNAPSHOT, {
+          includeUnstableChannel: true
+        }).then((latestVersion) => {
           m.chai.expect(latestVersion).to.equal('1.0.0+abb6139')
           done()
         }).catch(done)
       })
+
+      it('should return undefined if includeUnstableChannel is false', function (done) {
+        s3Packages.getLatestVersion(release.RELEASE_TYPE.SNAPSHOT, {
+          includeUnstableChannel: false
+        }).then((latestVersion) => {
+          m.chai.expect(latestVersion).to.be.undefined
+          done()
+        }).catch(done)
+      })
     })
 
     describe('given a greater snapshot version in a production bucket', function () {