Merge pull request #105 from opentable/preview-refactoring

Preview refactoring
opencomponents · Sep 15, 2015 · e38fdbc · e38fdbc
2 parents 4a37a26 + d06da1b
commit e38fdbc
Show file tree

Hide file tree

Showing 15 changed files with 252 additions and 159 deletions.
diff --git a/cli/commands.js b/cli/commands.js
@@ -76,10 +76,6 @@ module.exports = {
       options: {
         componentHref: {
           help: 'The name of the component to preview'
-        },
-        port: {
-          help: 'The port where to start the server. Default 3000',
-          required: false
         }
       }
     },

diff --git a/cli/facade/preview.js b/cli/facade/preview.js
@@ -1,9 +1,9 @@
 'use strict';
 
-var colors = require('colors');
-var format = require('stringformat');
-var http = require('http');
+var colors = require('colors/safe');
 var opn = require('opn');
+var querystring = require('querystring');
+var _ = require('underscore');
 
 var strings = require('../../resources/index');
 var urlParser = require('../../registry/domain/url-parser');
@@ -14,24 +14,22 @@ module.exports = function(dependencies){
 
   return function(opts){
 
-    var port = opts.port || 3000;
-
     urlParser.parse(opts.componentHref, function(err, parsed){
-      if(err){ return logger.log(strings.errors.cli.COMPONENT_HREF_NOT_FOUND.red); }
-
-      http.createServer(function(req, res){
-        res.writeHead(200, {'Content-Type': 'text/html'});
-        res.end(format('<!DOCTYPE html><html><head><meta charset="utf-8" />' +
-                       '</head><body><oc-component href="{0}"></oc-component>' +
-                       '<script src="{1}"></script></body></html>', 
-                        opts.componentHref, parsed.clientHref));
-
-      }).listen(port, function(){
-        var previewUrl = 'http://localhost:' + port;
-        logger.log(format(strings.messages.cli.PREVIEW_STARTED_AT_URL, previewUrl).green);
-        opn(previewUrl);
-      });
-    });
+      if(err){ return logger.log(colors.red(strings.errors.cli.COMPONENT_HREF_NOT_FOUND)); }
+
+      var href = parsed.registryUrl + parsed.componentName + '/';
 
+      if(!!parsed.version){
+        href += parsed.version + '/';
+      }
+
+      href += '~preview/';
+
+      if(!!parsed.parameters && !_.isEmpty(parsed.parameters)){
+        href += '?' + querystring.stringify(parsed.parameters);
+      }
+
+      opn(href);
+    });
   };
 };
diff --git a/docs/cli.md b/docs/cli.md
@@ -147,7 +147,7 @@ Runs a test page consuming a component
 
 Usage:
 ```sh
-oc preview <componentHref> [port]
+oc preview <componentHref>
 ```
 
 
@@ -156,7 +156,6 @@ Parameters:
 |Name|Description|
 |----|-----------|
 |componentHref|The name of the component to preview|
-|port|The port where to start the server. Default 3000|
 
 ##publish
 Publish a component

diff --git a/registry/routes/component-preview.js b/registry/routes/component-preview.js
@@ -17,6 +17,8 @@ module.exports = function(repository){
       var isHtmlRequest = !!req.headers.accept && req.headers.accept.indexOf('text/html') >= 0;
 
       if(isHtmlRequest && !!res.conf.discovery){
+
+
 
         return res.render('component-preview', {
           component: component,

diff --git a/test/mocks/console.js b/test/mocks/console.js
diff --git a/test/unit/cli-facade-dev.js b/test/unit/cli-facade-dev.js
@@ -4,21 +4,19 @@ var colors = require('colors');
 var expect = require('chai').expect;
 var sinon = require('sinon');
 
-var consoleMock = require('../mocks/console');
-
 describe('cli : facade : dev', function(){
 
-  var DevFacade = require('../../cli/facade/dev'),
+  var logSpy = {},
+      DevFacade = require('../../cli/facade/dev'),
       Local = require('../../cli/domain/local'),
       local = new Local(),
       npm = require('npm'),
-      devFacade = new DevFacade({ local: local, logger: consoleMock }),
-      logs;
+      devFacade = new DevFacade({ local: local, logger: logSpy });
 
   var execute = function(dirName, port){
-    consoleMock.reset();
+    logSpy.logNoNewLine = sinon.spy();
+    logSpy.log = sinon.spy();
     devFacade({ dirName: dirName, port: port });
-    logs = consoleMock.get();
   };
 
   describe('when running a dev version of the registry', function(){
@@ -37,7 +35,7 @@ describe('cli : facade : dev', function(){
       });
 
       it('should show an error', function(){
-        expect(logs[1]).to.equal('path is not valid!'.red);
+        expect(logSpy.log.args[0][0]).to.equal('path is not valid!'.red);
       });
     });
 
@@ -55,7 +53,7 @@ describe('cli : facade : dev', function(){
       });
 
       it('should show an error', function(){
-        expect(logs[1]).to.equal('An error happened when initialising the dev runner: no components found in specified path'.red);
+        expect(logSpy.log.args[0][0]).to.equal('An error happened when initialising the dev runner: no components found in specified path'.red);
       });
     });
   });

diff --git a/test/unit/cli-facade-info.js b/test/unit/cli-facade-info.js
@@ -7,14 +7,13 @@ var _ = require('underscore');
 
 describe('cli : facade : info', function(){
 
-  var Registry = require('../../cli/domain/registry'),
+  var logSpy = {},
+      Registry = require('../../cli/domain/registry'),
       registry = new Registry(),
-      consoleMock = require('../mocks/console'),
       Local = require('../../cli/domain/local'),
       local = new Local(),
       InfoFacade = require('../../cli/facade/info'),
-      infoFacade = new InfoFacade({ registry: registry, local: local, logger: consoleMock }),
-      logs;
+      infoFacade = new InfoFacade({ registry: registry, local: local, logger: logSpy });
 
   var setup = function(stubs){
     _.forEach(stubs, function(stubInfo){
@@ -26,9 +25,8 @@ describe('cli : facade : info', function(){
   };
 
   var execute = function(){
-    consoleMock.reset();
+    logSpy.log = sinon.spy();
     infoFacade();
-    logs = consoleMock.get();    
   };
 
   describe('when showing project info', function(){
@@ -45,7 +43,7 @@ describe('cli : facade : info', function(){
       });
 
       it('should show an error', function(){
-        expect(logs[0]).to.be.equal('something bad happened'.red);
+        expect(logSpy.log.args[0][0]).to.be.equal('something bad happened'.red);
       });
     });
 
@@ -63,7 +61,7 @@ describe('cli : facade : info', function(){
         });
 
         it('should show an error', function(){
-          expect(logs[0]).to.be.equal('oc registries not found. Run "oc registry add <registry href>"'.red);
+          expect(logSpy.log.args[0][0]).to.be.equal('oc registries not found. Run "oc registry add <registry href>"'.red);
         });
       });
 
@@ -79,7 +77,7 @@ describe('cli : facade : info', function(){
         });
 
         it('should show an error', function(){
-          expect(logs[0]).to.be.equal('No components linked in the project'.red);
+          expect(logSpy.log.args[0][0]).to.be.equal('No components linked in the project'.red);
         });
       });
 
@@ -117,21 +115,21 @@ describe('cli : facade : info', function(){
         });
 
         it('should list the components', function(){
-          expect(logs[0]).to.be.equal('Components linked in project:'.yellow);
+          expect(logSpy.log.args[0][0]).to.be.equal('Components linked in project:'.yellow);
         }); 
 
         it('should show a message when a component is not found on the registry', function(){
-          expect(logs[1]).to.include('Not available'.red);
+          expect(logSpy.log.args[1][0]).to.include('Not available'.red);
         });
 
         it('should show the component\'s details when it is found on the registry', function(){
-          expect(logs[1]).to.include('hello');
-          expect(logs[1]).to.include('the best component ever');
-          expect(logs[1]).to.include('http://registry.com/hello/~1.0.0');
+          expect(logSpy.log.args[1][0]).to.include('hello');
+          expect(logSpy.log.args[1][0]).to.include('the best component ever');
+          expect(logSpy.log.args[1][0]).to.include('http://registry.com/hello/~1.0.0');
         });
 
         it('should show the component\'s resolved version', function(){
-          expect(logs[1]).to.include('~1.0.0 => 1.0.1');
+          expect(logSpy.log.args[1][0]).to.include('~1.0.0 => 1.0.1');
         });
       });
     });

diff --git a/test/unit/cli-facade-init.js b/test/unit/cli-facade-init.js
@@ -4,20 +4,17 @@ var colors = require('colors');
 var expect = require('chai').expect;
 var sinon = require('sinon');
 
-var consoleMock = require('../mocks/console');
-
 describe('cli : facade : init', function(){
 
-  var InitFacade = require('../../cli/facade/init'),
+  var logSpy = {},
+      InitFacade = require('../../cli/facade/init'),
       Local = require('../../cli/domain/local'),
       local = new Local(),
-      initFacade = new InitFacade({ local: local, logger: consoleMock }),
-      logs;
+      initFacade = new InitFacade({ local: local, logger: logSpy });
 
   var execute = function(componentName, templateType){
-    consoleMock.reset();
+    logSpy.log = sinon.spy();
     initFacade({ componentName: componentName, templateType: templateType });
-    logs = consoleMock.get();
   };
 
   describe('when initialising a new component', function(){
@@ -29,7 +26,7 @@ describe('cli : facade : init', function(){
       });
 
       it('should show an error', function(){
-        expect(logs[0]).to.equal('An error happened when initialising the component: the name is not valid. Allowed characters are alphanumeric, _, -'.red);
+        expect(logSpy.log.args[0][0]).to.equal('An error happened when initialising the component: the name is not valid. Allowed characters are alphanumeric, _, -'.red);
       });
     });
 
@@ -40,17 +37,18 @@ describe('cli : facade : init', function(){
       });
 
       it('should show an error', function(){
-        expect(logs[0]).to.equal('An error happened when initialising the component: the name is not valid. Allowed characters are alphanumeric, _, -'.red);
+        expect(logSpy.log.args[0][0]).to.equal('An error happened when initialising the component: the name is not valid. Allowed characters are alphanumeric, _, -'.red);
       });
     });
 
     describe('when the template is of a non valid type', function(){
         beforeEach(function(){
-            execute('valid-component', 'invalid-type');
+          execute('valid-component', 'invalid-type');
         });
 
         it('should show an error', function(){
-            expect(logs[0]).to.equal('An error happened when initialising the component: the template is not valid. Allowed values are handlebars and jade'.red);
+          var expected = 'An error happened when initialising the component: the template is not valid. Allowed values are handlebars and jade';
+          expect(logSpy.log.args[0][0]).to.equal(expected.red);
         });
     });
 
@@ -66,7 +64,7 @@ describe('cli : facade : init', function(){
       });
 
       it('should show an error', function(){
-        expect(logs[0]).to.equal('An error happened when initialising the component: nope!'.red);
+        expect(logSpy.log.args[0][0]).to.equal('An error happened when initialising the component: nope!'.red);
       });
     });
 
@@ -82,7 +80,7 @@ describe('cli : facade : init', function(){
       });
 
       it('should show a message', function(){
-        expect(logs[0]).to.equal('Component "the-best-component" created'.green);
+        expect(logSpy.log.args[0][0]).to.equal('Component "the-best-component" created'.green);
       });
     });
   });

diff --git a/test/unit/cli-facade-link.js b/test/unit/cli-facade-link.js
@@ -4,20 +4,17 @@ var colors = require('colors');
 var expect = require('chai').expect;
 var sinon = require('sinon');
 
-var consoleMock = require('../mocks/console');
-
 describe('cli : facade : link', function(){
 
-  var LinkFacade = require('../../cli/facade/link'),
+  var logSpy = {},
+      LinkFacade = require('../../cli/facade/link'),
       Local = require('../../cli/domain/local'),
       local = new Local(),
-      linkFacade = new LinkFacade({ local: local, logger: consoleMock }),
-      logs;
+      linkFacade = new LinkFacade({ local: local, logger: logSpy });
 
   var execute = function(){
-    consoleMock.reset();
+    logSpy.log = sinon.spy();
     linkFacade({ componentName: 'hello' });
-    logs = consoleMock.get();
   };
 
   describe('when linking component', function(){
@@ -34,7 +31,7 @@ describe('cli : facade : link', function(){
       });
 
       it('should show the error', function(){
-        expect(logs[0]).to.equal('an error!'.red);
+        expect(logSpy.log.args[0][0]).to.equal('an error!'.red);
       });
     }); 
 
@@ -50,7 +47,7 @@ describe('cli : facade : link', function(){
       });
 
       it('should show a confirmation message', function(){
-        expect(logs[0]).to.equal('oc Component linked'.green);
+        expect(logSpy.log.args[0][0]).to.equal('oc Component linked'.green);
       });
     });
   });