From af851ed6375c4d51d40ba9806f29821853b92e58 Mon Sep 17 00:00:00 2001 From: German Attanasio Ruiz Date: Wed, 18 May 2016 17:12:29 -0400 Subject: [PATCH] code review + eslint --- .cfignore | 3 +- .eslintignore | 2 + .eslintrc | 16 ++++++ .gitignore | 4 +- .travis.yml | 4 +- CONTRIBUTING.md | 1 + README.md | 8 +-- app.js | 2 +- config/error-handler.js | 8 +-- config/express.js | 17 +++--- config/security.js | 15 +++-- manifest.yml | 2 +- package.json | 19 +++--- public/js/components/App.js | 54 +++++++++--------- public/js/components/helpers.js | 20 +++---- public/js/components/tab-panels.js | 4 +- public/js/demo.js | 92 +++++++++++++++--------------- server.js | 8 ++- test/test.express.js | 3 - 19 files changed, 152 insertions(+), 130 deletions(-) create mode 100644 .eslintignore create mode 100644 .eslintrc diff --git a/.cfignore b/.cfignore index 2099835f..3091757a 100644 --- a/.cfignore +++ b/.cfignore @@ -1,3 +1,2 @@ node_modules -run.sh -VCAP_SERVICES.json +coverage \ No newline at end of file diff --git a/.eslintignore b/.eslintignore new file mode 100644 index 00000000..d1e4162b --- /dev/null +++ b/.eslintignore @@ -0,0 +1,2 @@ +coverage +public/js/vendors/prism.js \ No newline at end of file diff --git a/.eslintrc b/.eslintrc new file mode 100644 index 00000000..fc5a13ca --- /dev/null +++ b/.eslintrc @@ -0,0 +1,16 @@ +{ + "rules": { + "no-console": 0, + "func-names": 0, + "vars-on-top": 0, + "consistent-return": 0 + }, + "globals": { + describe: true, + it: true + }, + "env": { + "node": true + }, + "extends": "eslint-config-airbnb-es5", +} \ No newline at end of file diff --git a/.gitignore b/.gitignore index 08f7c23e..f3bbd849 100755 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,4 @@ node_modules .DS_Store -run.sh -VCAP_SERVICES.json +.env +coverage diff --git a/.travis.yml b/.travis.yml index bec59a05..7760d6ff 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,8 +1,8 @@ language: node_js sudo: true -node_js: -- stable +node_js: stable script: +- npm run lint - npm test env: global: diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3b59e440..695f5bbb 100755 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -12,5 +12,6 @@ If you want to contribute to the repository, here's a quick guide: 1. Fork the repo. 1. develop your code changes: `npm install -d` +1. test your code changes: `npm run lint && npm test` 1. Commit your changes 1. Push to your fork and submit a pull request diff --git a/README.md b/README.md index b0b526fe..43858cc0 100755 --- a/README.md +++ b/README.md @@ -22,7 +22,7 @@ applications: - services: - tone-analyzer-service name: - command: node server.js + command: npm start path: . memory: 256M ``` @@ -46,7 +46,7 @@ applications: 5. Create the Tone Analyzer Service in Bluemix ```sh - $ cf create-service tone_analyzer beta tone-analyzer-service + $ cf create-service tone_analyzer standard tone-analyzer-service ``` 6. Push it live! @@ -79,7 +79,7 @@ See the full [Getting Started][getting_started] documentation for more details, }, "label": "tone_analyzer", "name": "tone-analyzer-service", - "plan": "beta" + "plan": "standard" }] } } @@ -91,7 +91,7 @@ See the full [Getting Started][getting_started] documentation for more details, 3. Go to the project folder in a terminal and run: `npm install` 4. Start the application -5. `node server.js` +5. `npm start` 6. Go to `http://localhost:3000` ## Troubleshooting diff --git a/app.js b/app.js index 5e0aecd1..62d657b8 100755 --- a/app.js +++ b/app.js @@ -25,7 +25,7 @@ require('./config/express')(app); // Create the service wrapper var toneAnalyzer = watson.tone_analyzer({ - url: 'https://gateway.watsonplatform.net/tone-analyzer-beta/api/', + url: 'https://gateway.watsonplatform.net/tone-analyzer/api/', username: '', password: '', version_date: '2016-05-19', diff --git a/config/error-handler.js b/config/error-handler.js index 9f66a99b..c3a73d87 100755 --- a/config/error-handler.js +++ b/config/error-handler.js @@ -16,8 +16,7 @@ 'use strict'; -module.exports = function (app) { - +module.exports = function(app) { // catch 404 and forward to error handler app.use(function(req, res, next) { var err = new Error('Not Found'); @@ -33,8 +32,7 @@ module.exports = function (app) { error: err.error || err.message }; console.log('error:', error); - + next ? '' : ''; res.status(error.code).json(error); }); - -}; \ No newline at end of file +}; diff --git a/config/express.js b/config/express.js index da1545ff..b89a8228 100755 --- a/config/express.js +++ b/config/express.js @@ -17,23 +17,26 @@ 'use strict'; // Module dependencies -var express = require('express'), - bodyParser = require('body-parser'); +var express = require('express'); +var bodyParser = require('body-parser'); +var path = require('path'); -module.exports = function (app) { +module.exports = function(app) { app.enable('trust proxy'); // Configure Express app.set('view engine', 'ejs'); require('ejs').delimiter = '$'; - app.use(bodyParser.urlencoded({ extended: true })); + app.use(bodyParser.urlencoded({ + extended: true + })); app.use(bodyParser.json()); // Setup static public directory - app.use(express.static(__dirname + '/../public')); + app.use(express.static(path.join(__dirname, '..', '/public'))); // Only loaded when VCAP_APPLICATION is `true` - if (process.env.VCAP_APPLICATION) + if (process.env.VCAP_APPLICATION) { require('./security')(app); - + } }; diff --git a/config/security.js b/config/security.js index 860327a4..95bf4392 100644 --- a/config/security.js +++ b/config/security.js @@ -17,12 +17,12 @@ 'use strict'; // security.js -var rateLimit = require('express-rate-limit'), - helmet = require('helmet'), - csrf = require('csurf'), - cookieParser = require('cookie-parser'); +var rateLimit = require('express-rate-limit'); +var helmet = require('helmet'); +var csrf = require('csurf'); +var cookieParser = require('cookie-parser'); -module.exports = function (app) { +module.exports = function(app) { app.enable('trust proxy'); // 1. helmet with defaults @@ -34,9 +34,9 @@ module.exports = function (app) { delayMs: 0, max: 6, message: JSON.stringify({ - error:'Too many requests, please try again in 30 seconds.', + error: 'Too many requests, please try again in 30 seconds.', code: 429 - }), + }) })); // 3. setup cookies @@ -46,7 +46,6 @@ module.exports = function (app) { // 4. csrf var csrfProtection = csrf({ cookie: true }); app.get('/', csrfProtection, function(req, res, next) { - console.log(req.csrfToken()); req._csrfToken = req.csrfToken(); next(); }); diff --git a/manifest.yml b/manifest.yml index b3e762f5..d5b61832 100644 --- a/manifest.yml +++ b/manifest.yml @@ -2,7 +2,7 @@ declared-services: tone-analyzer-service: label: tone_analyzer - plan: beta + plan: standard applications: - services: - tone-analyzer-service diff --git a/package.json b/package.json index 4c392670..f21ddee1 100644 --- a/package.json +++ b/package.json @@ -27,29 +27,34 @@ }, "scripts": { "start": "node server.js", - "test": "npm run jshint && (npm run codecov || true)", - "jshint": "jshint --exclude ./node_modules/", - "codecov": "istanbul cover ./node_modules/mocha/bin/_mocha && codecov" + "test": "istanbul cover ./node_modules/mocha/bin/_mocha", + "lint": "eslint .", + "autofix": "eslint --fix .", + "codecov": "npm run test && (codecov || true)" }, "dependencies": { "body-parser": "^1.13.2", "cf-deployment-tracker-client": "0.0.8", "cookie-parser": "^1.4.1", "csurf": "^1.8.3", + "dotenv": "^2.0.0", "ejs": "^2.3.4", "errorhandler": "^1.4.1", "express": "^4.12.2", "express-rate-limit": "^2.1.0", "express-secure-only": "^0.2.1", "helmet": "^1.3.0", - "watson-developer-cloud": "https://github.com/watson-developer-cloud/node-sdk.git" + "watson-developer-cloud": "^1.8.0" }, "devDependencies": { + "babel-eslint": "^6.0.4", "codecov": "^1.0.1", + "eslint": "^2.8.0", + "eslint-config-airbnb-es5": "^1.0.9", + "eslint-plugin-react": "^5.1.1", "istanbul": "^0.4.2", - "jshint": "^2.9.1", "mocha": "^2.4.5", - "nock": "7.7.0", - "supertest": "^1.2.0" + "supertest": "^1.2.0", + "nock": "7.7.0" } } diff --git a/public/js/components/App.js b/public/js/components/App.js index f5aac0ba..db170e13 100644 --- a/public/js/components/App.js +++ b/public/js/components/App.js @@ -27,18 +27,18 @@ */ function App(documentTones, sentences, thresholds, selectedSample) { var _selectedFilter = 'Anger', - _selectedTone = 'Emotion Tone', - _selectedSample = selectedSample || 'customer-call', - _lowToHigh = false, - _currentHoveredOriginalSentence = document.querySelector('body'), - _rankedSentences = sentences, - _originalSentences, - _documentTones = documentTones, - _thresholds = thresholds, - _isHoveringOriginalText = false, - _socialToneHoverTexts, - _toneHash, - TONE_CATEGORIES_RESET = [{ + _selectedTone = 'Emotion Tone', + _selectedSample = selectedSample || 'customer-call', + _lowToHigh = false, + _currentHoveredOriginalSentence = document.querySelector('body'), + _rankedSentences = sentences, + _originalSentences, + _documentTones = documentTones, + _thresholds = thresholds, + _isHoveringOriginalText = false, + _socialToneHoverTexts, + _toneHash, + TONE_CATEGORIES_RESET = [{ tones: [{ score: 0, tone_id: 'Anger', @@ -129,10 +129,10 @@ function App(documentTones, sentences, thresholds, selectedSample) { category_id: 'social_tone', category_name: 'Social Tone' }], SCORE_DECIMAL_PLACE = 2, - PERCENTAGE_DECIMAL_PLACE = 1, - SOCIAL_TONE_MIN_RANGE = -1, - SOCIAL_TONE_MAX_RANGE = 1, - output = {}; + PERCENTAGE_DECIMAL_PLACE = 1, + SOCIAL_TONE_MIN_RANGE = -1, + SOCIAL_TONE_MAX_RANGE = 1, + output = {}; /** * Make sure sentences have proper tone data values. @@ -165,9 +165,9 @@ function App(documentTones, sentences, thresholds, selectedSample) { */ function _toneLevel(toneKey, score, classNameType) { var output, - toneValue = _toneHash[toneKey], - newScore = score, - baseThreshold = 0; + toneValue = _toneHash[toneKey], + newScore = score, + baseThreshold = 0; if (newScore <= baseThreshold) output = ''; @@ -292,7 +292,7 @@ function App(documentTones, sentences, thresholds, selectedSample) { return b.tone_categories[_searchIndex(_selectedTone)].tones[_searchIndex(_selectedFilter)].score - a.tone_categories[_searchIndex(_selectedTone)].tones[_searchIndex(_selectedFilter)].score; }, - map = function(item) { + map = function(item) { var score = item.tone_categories[_searchIndex(_selectedTone)].tones[_searchIndex(_selectedFilter)].score.toFixed(SCORE_DECIMAL_PLACE); return { text: item.text, @@ -300,7 +300,7 @@ function App(documentTones, sentences, thresholds, selectedSample) { className: 'sentence-rank--score_' + normalize(_selectedFilter) }; }, - filter = (_selectedTone === 'Social Tone') ? + filter = (_selectedTone === 'Social Tone') ? function(item) { return item.score >= -1; } : @@ -348,17 +348,17 @@ function App(documentTones, sentences, thresholds, selectedSample) { */ output.selectFilterBySample = function() { var getHighestTone = function(toneCategory) { - var highestTone = _documentTones.tone_categories[_searchIndex(toneCategory)].tones[0].tone_name, - highestScore = 0; - _documentTones.tone_categories[_searchIndex(toneCategory)].tones.forEach(function(item) { + var highestTone = _documentTones.tone_categories[_searchIndex(toneCategory)].tones[0].tone_name, + highestScore = 0; + _documentTones.tone_categories[_searchIndex(toneCategory)].tones.forEach(function(item) { if (highestScore < item.score) { highestScore = item.score; highestTone = item.tone_name; } }); - return highestTone; - }, - sample = { + return highestTone; + }, + sample = { 'customer-call': getHighestTone('Emotion Tone'), 'email': getHighestTone('Social Tone'), 'corporate-announcement': getHighestTone('Language Tone'), diff --git a/public/js/components/helpers.js b/public/js/components/helpers.js index 3e52130b..dfdf98be 100644 --- a/public/js/components/helpers.js +++ b/public/js/components/helpers.js @@ -66,18 +66,18 @@ function scrollTo($element) { * @return {Array} arr */ function move(arr, old_index, new_index) { - while (old_index < 0) { - old_index += arr.length; + while (old_index < 0) { + old_index += arr.length; } - while (new_index < 0) { - new_index += arr.length; + while (new_index < 0) { + new_index += arr.length; } - if (new_index >= arr.length) { - var k = new_index - arr.length; - while ((k--) + 1) { - arr.push(undefined); + if (new_index >= arr.length) { + var k = new_index - arr.length; + while ((k--) + 1) { + arr.push(undefined); } } - arr.splice(new_index, 0, arr.splice(old_index, 1)[0]); - return arr; + arr.splice(new_index, 0, arr.splice(old_index, 1)[0]); + return arr; } diff --git a/public/js/components/tab-panels.js b/public/js/components/tab-panels.js index bbeabcf9..7df65419 100644 --- a/public/js/components/tab-panels.js +++ b/public/js/components/tab-panels.js @@ -2,7 +2,7 @@ Tabbed Panels js */ (function() { - $('.tab-panels--tab').click(function(e){ + $('.tab-panels--tab').click(function(e) { e.preventDefault(); var self = $(this); var inputGroup = self.closest('.tab-panels'); @@ -13,4 +13,4 @@ Tabbed Panels js idName = self.attr('href'); $(idName).addClass('active'); }); -})(); \ No newline at end of file +})(); diff --git a/public/js/demo.js b/public/js/demo.js index b881e130..b10864c2 100755 --- a/public/js/demo.js +++ b/public/js/demo.js @@ -53,40 +53,40 @@ function ready() { */ function allReady(thresholds, sampleText) { var $input = $('.input'), - $output = $('.output'), - $loading = $('.loading'), - $error = $('.error'), - $errorMessage = $('.error--message'), - $inputRadio = $('.input--radio'), - $textarea = $('.input--textarea'), - $submitButton = $('.input--submit-button'), - $emotionGraph = $('.summary-emotion-graph'), - $writingGraph = $('.summary-writing-graph'), - $socialGraph = $('.summary-social-graph'), - $summaryJsonButton = $('.js-toggle-summary-json'), - $summaryJson = $('.js-summary-json'), - $summaryJsonView = $('.js-toggle-summary-json_show'), - $summaryJsonHide = $('.js-toggle-summary-json_hide'), - $summaryJsonCode = $('.js-summary-json .json--code'), - $emotionFilters = $('.filters--emotion'), - $writingFilters = $('.filters--writing'), - $socialFilters = $('.filters--social'), - $originalText = $('.original-text'), - $originalTexts = $('.original-text--texts'), - $originalTextTooltipContainer = $('.original-text--tooltip-container'), - $legend = $('.original-text--legend'), - $sentenceRankTable = $('.sentence-rank--table'), - $sentenceJson = $('.json .json--code'), - $outputResetButton = $('.output--reset-button'), - barGraph_template = barGraphTemplate.innerHTML, - emotionBarGraph_template = emotionBarGraphTemplate.innerHTML, - verticalBarGraph_template = verticalBarGraphTemplate.innerHTML, - filters_template = filtersTemplate.innerHTML, - originalText_template = originalTextTemplate.innerHTML, - sentenceRank_template = sentenceRankTemplate.innerHTML, - originalTextTooltip_template = originalTextTooltipTemplate.innerHTML, - originalTextLegend_template = originalTextLegendTemplate.innerHTML, - lastSentenceID = 0; + $output = $('.output'), + $loading = $('.loading'), + $error = $('.error'), + $errorMessage = $('.error--message'), + $inputRadio = $('.input--radio'), + $textarea = $('.input--textarea'), + $submitButton = $('.input--submit-button'), + $emotionGraph = $('.summary-emotion-graph'), + $writingGraph = $('.summary-writing-graph'), + $socialGraph = $('.summary-social-graph'), + $summaryJsonButton = $('.js-toggle-summary-json'), + $summaryJson = $('.js-summary-json'), + $summaryJsonView = $('.js-toggle-summary-json_show'), + $summaryJsonHide = $('.js-toggle-summary-json_hide'), + $summaryJsonCode = $('.js-summary-json .json--code'), + $emotionFilters = $('.filters--emotion'), + $writingFilters = $('.filters--writing'), + $socialFilters = $('.filters--social'), + $originalText = $('.original-text'), + $originalTexts = $('.original-text--texts'), + $originalTextTooltipContainer = $('.original-text--tooltip-container'), + $legend = $('.original-text--legend'), + $sentenceRankTable = $('.sentence-rank--table'), + $sentenceJson = $('.json .json--code'), + $outputResetButton = $('.output--reset-button'), + barGraph_template = barGraphTemplate.innerHTML, + emotionBarGraph_template = emotionBarGraphTemplate.innerHTML, + verticalBarGraph_template = verticalBarGraphTemplate.innerHTML, + filters_template = filtersTemplate.innerHTML, + originalText_template = originalTextTemplate.innerHTML, + sentenceRank_template = sentenceRankTemplate.innerHTML, + originalTextTooltip_template = originalTextTooltipTemplate.innerHTML, + originalTextLegend_template = originalTextLegendTemplate.innerHTML, + lastSentenceID = 0; /** * Callback function for AJAX post to get tone analyzer data @@ -101,12 +101,12 @@ function allReady(thresholds, sampleText) { scrollTo($output); var emotionTone = data.document_tone.tone_categories[0].tones, - writingTone = data.document_tone.tone_categories[1].tones, - socialTone = data.document_tone.tone_categories[2].tones, - selectedSample = $('input[name=rb]:checked').val(), - selectedSampleText = $textarea.val(), - sentences, - app; + writingTone = data.document_tone.tone_categories[1].tones, + socialTone = data.document_tone.tone_categories[2].tones, + selectedSample = $('input[name=rb]:checked').val(), + selectedSampleText = $textarea.val(), + sentences, + app; // if only one sentence, sentences will not exist, so mutate sentences_tone manually if (data.sentences_tone === undefined) { @@ -216,10 +216,10 @@ function allReady(thresholds, sampleText) { */ function positionOriginalTextTooltip(e) { var element = app.currentHoveredOriginalSentence(), - box = element.getBoundingClientRect(), - originalText = document.querySelector('.original-text'), - top = box.top, - left = box.left + originalText.getBoundingClientRect().width * 0.05; + box = element.getBoundingClientRect(), + originalText = document.querySelector('.original-text'), + top = box.top, + left = box.left + originalText.getBoundingClientRect().width * 0.05; if (e !== undefined) left = e.clientX; @@ -265,7 +265,7 @@ function allReady(thresholds, sampleText) { updateOriginalTextTooltip(id); $originalTextTooltipContainer.removeClass('original-text--tooltip-container_hidden'); app.isHoveringOriginalText(true); - $('.original-text--sentence-container').not('[data-index="'+id+'"]'); + $('.original-text--sentence-container').not('[data-index="' + id + '"]'); positionOriginalTextTooltip(e); } @@ -377,7 +377,7 @@ function allReady(thresholds, sampleText) { if (error.responseJSON.code === 429) message = 'You\'ve sent a lot of requests in a short amount of time. ' + - 'As the CPU cores cool off a bit, wait a few seonds before sending more requests.' + 'As the CPU cores cool off a bit, wait a few seonds before sending more requests.'; $errorMessage.html(message); $input.show(); $loading.hide(); diff --git a/server.js b/server.js index 251977bd..492fe7a3 100644 --- a/server.js +++ b/server.js @@ -1,8 +1,10 @@ #! /usr/bin/env node 'use strict'; -if (process.env.GOOGLE_ANALYTICS){ - process.env.GOOGLE_ANALYTICS = process.env.GOOGLE_ANALYTICS.replace(/\"/g,''); +require('dotenv').config({silent: true}); + +if (process.env.GOOGLE_ANALYTICS) { + process.env.GOOGLE_ANALYTICS = process.env.GOOGLE_ANALYTICS.replace(/\"/g, ''); } // Deployment tracking require('cf-deployment-tracker-client').track(); @@ -10,6 +12,6 @@ require('cf-deployment-tracker-client').track(); var server = require('./app'); var port = process.env.PORT || process.env.VCAP_APP_PORT || 3000; -server.listen(port, function () { +server.listen(port, function() { console.log('Server running on port: %d', port); }); diff --git a/test/test.express.js b/test/test.express.js index 2c9f46a3..3ffc32bd 100644 --- a/test/test.express.js +++ b/test/test.express.js @@ -18,12 +18,9 @@ var app = require('../app'); var request = require('supertest'); -var nock = require('nock'); describe('express', function() { - it('load home page when GET /', function(done) { request(app).get('/').expect(200, done); }); - });