fix(config): don't overwrite url port if port is manually specified

closes #592 - if both url and port are passed via args, we don't want to overwrite the port of the url
TryGhost · May 28, 2018 · b094495 · b094495
1 parent 687c499
commit b094495
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 14 deletions.
diff --git a/lib/commands/config/index.js b/lib/commands/config/index.js
@@ -23,7 +23,7 @@ class ConfigCommand extends Command {
         this.instance = this.system.getInstance();
     }
 
-    handleAdvancedOptions(argv) {
+    handleAdvancedOptions(argv = {}) {
         const errors = require('../../errors');
         const Promise = require('bluebird');
         const isFunction = require('lodash/isFunction');
@@ -65,22 +65,31 @@ class ConfigCommand extends Command {
                 this.instance.config.set(configKey, value);
             });
         }).then(() => {
-            // Because the 'port' option can end up being different than the one supplied
-            // in the URL itself, we want to make sure the port in the URL
-            // (if one was there to begin with) is correct.
-            const parsedUrl = url.parse(this.instance.config.get('url'));
-            if (parsedUrl.port && parsedUrl.port !== this.instance.config.get('server.port', parsedUrl.port)) {
-                parsedUrl.port = this.instance.config.get('server.port');
-                // url.format won't take the new port unless 'parsedUrl.host' is undefined
-                delete parsedUrl.host;
-
-                this.instance.config.set('url', url.format(parsedUrl));
-            }
-
+            this.syncUrl(argv);
             this.instance.config.save();
         });
     }
 
+    syncUrl(argv) {
+        if (argv.url && argv.port) {
+            // If we have supplied both url and port options via args, we
+            // don't want to override anything so just return
+            return;
+        }
+
+        // Because the 'port' option can end up being different than the one supplied
+        // in the URL itself, we want to make sure the port in the URL
+        // (if one was there to begin with) is correct.
+        const parsedUrl = url.parse(this.instance.config.get('url'));
+        if (parsedUrl.port && parsedUrl.port !== this.instance.config.get('server.port', parsedUrl.port)) {
+            parsedUrl.port = this.instance.config.get('server.port');
+            // url.format won't take the new port unless 'parsedUrl.host' is undefined
+            delete parsedUrl.host;
+
+            this.instance.config.set('url', url.format(parsedUrl));
+        }
+    }
+
     getConfigPrompts(argv) {
         const validator = require('validator');
         const path = require('path');

diff --git a/test/unit/commands/config-spec.js b/test/unit/commands/config-spec.js
@@ -156,7 +156,7 @@ describe('Unit: Command > Config', function () {
             return config.handleAdvancedOptions({url: 'http://localhost:2368/', port: 1234}).then(() => {
                 expect(getInstanceStub.calledOnce).to.be.true;
                 expect(save.calledOnce).to.be.true;
-                expect(instanceConfig.get('url')).to.equal('http://localhost:1234/');
+                expect(instanceConfig.get('url')).to.equal('http://localhost:2368/');
                 expect(instanceConfig.get('server.port')).to.equal(1234);
             });
         });