Merge branch 'master' into invalidate-sessions-after-email-or-password-change

.circleci/config.yml

@@ -58,6 +58,7 @@ jobs:
     environment:
       COMPOSE_FILE: .circleci/docker-compose.cypress.yml
       COMPOSE_PROJECT_NAME: cypress
+      PERCY_TOKEN_ENCODED: MWM3OGUzNzk4ZWQ2NTE4YTBhMDAwZDNiNWE1Nzc4ZjEzZjYyMzY1MjE0NjY0NDRiOGE5ODc5ZGYzYTU4ZmE4NQ==
     docker:
       - image: circleci/node:8
     steps:
@@ -65,15 +66,16 @@ jobs:
       - checkout
       - run:
           name: Install npm dependencies
-          command: npm install
+          command: |
+            npm install
       - run:
           name: Setup Redash server
           command: |
-            npm run cypress:server start-ci
-            docker-compose run cypress node ./cypress/cypress-server.js setup
+            npm run cypress start
+            docker-compose run cypress node ./cypress/cypress.js db-seed
       - run:
           name: Execute Cypress tests
-          command: docker-compose run cypress ./node_modules/.bin/cypress run
+          command: npm run cypress run-ci
   build-tarball:
     docker:
       - image: circleci/node:8

.circleci/docker-compose.cypress.yml

@@ -34,6 +34,10 @@ services:
       - worker
     environment:
       CYPRESS_baseUrl: "http://server:5000"
+      PERCY_TOKEN: ${PERCY_TOKEN}
+      PERCY_BRANCH: ${CIRCLE_BRANCH}
+      PERCY_COMMIT: ${CIRCLE_SHA1}
+      PERCY_PULL_REQUEST: ${CIRCLE_PR_NUMBER}
   redis:
     image: redis:3.0-alpine
     restart: unless-stopped

Dockerfile.cypress

@@ -1,10 +1,11 @@
 FROM cypress/browsers:chrome67
 
-WORKDIR /usr/src/app
+ENV APP /usr/src/app
+WORKDIR $APP
 
-RUN npm install cypress > /dev/null
+RUN npm install --no-save cypress @percy/cypress > /dev/null
 
-COPY cypress /usr/src/app/cypress
-COPY cypress.json /usr/src/app/cypress.json
+COPY cypress $APP/cypress
+COPY cypress.json $APP/cypress.json
 
 RUN ./node_modules/.bin/cypress verify

client/app/components/EditInPlace.jsx

@@ -40,7 +40,7 @@ export class EditInPlace extends React.Component {
 
   stopEditing = () => {
     const newValue = this.inputRef.current.value;
-    const ignorableBlank = this.props.ignoreBlanks && this.props.value === '';
+    const ignorableBlank = this.props.ignoreBlanks && newValue === '';
     if (!ignorableBlank && newValue !== this.props.value) {
       this.props.onDone(newValue);
     }

client/app/components/QueryEditor.jsx

@@ -179,7 +179,7 @@ class QueryEditor extends React.Component {
     const isExecuteDisabled = this.props.queryExecuting || !this.props.canExecuteQuery();
 
     return (
-      <section style={{ height: '100%' }}>
+      <section style={{ height: '100%' }} data-test="QueryEditor">
         <div className="container p-15 m-b-10" style={{ height: '100%' }}>
           <div style={{ height: 'calc(100% - 40px)', marginBottom: '0px' }} className="editor__container">
             <AceEditor
@@ -262,6 +262,7 @@ class QueryEditor extends React.Component {
                   className={'btn btn-primary m-l-5' + (isExecuteDisabled ? ' disabled' : '')}
                   disabled={isExecuteDisabled}
                   onClick={this.props.executeQuery}
+                  data-test="ExecuteButton"
                 >
                   <span className="zmdi zmdi-play" />
                   <span className="hidden-xs m-l-5">Execute</span>

client/app/components/app-header/app-header.html

@@ -86,7 +86,7 @@
           <!--<a href="users" title="Settings"><i class="fa fa-cog"></i></a>-->
         <!--</li>-->
         <li class="dropdown" uib-dropdown>
-          <a href="#" class="dropdown-toggle dropdown--profile" uib-dropdown-toggle data-cy="dropdown-profile">
+          <a href="#" class="dropdown-toggle dropdown--profile" uib-dropdown-toggle data-test="ProfileDropdown">
             <img ng-src="{{ $ctrl.currentUser.profile_image_url }}" class="profile__image--navbar" width="20"/>
             <span class="dropdown--profile__username" ng-bind="$ctrl.currentUser.name"></span> <span
             class="caret caret--nav"></span></a>

client/app/components/dynamic-form.html

@@ -1,7 +1,8 @@
 <form name="dynamicForm">
     <div class="form-group required" ng-class='{"has-error": (dynamicForm.targetName | showError)}'>
         <label class="control-label" for="dataSourceName">Name</label>
-        <input type="string" class="form-control" name="targetName" ng-model="target.name" autofocus required>
+        <input type="string" class="form-control" name="targetName" ng-model="target.name"
+               data-test="TargetName" autofocus required>
         <error-messages input="dynamicForm.targetName" form="dynamicForm"></error-messages>
     </div>
     <hr>
@@ -9,12 +10,12 @@
         <label ng-if="field.property.type !== 'checkbox'" class="control-label">{{field.property.title || field.name | toHuman}}</label>
         <input name="input" type="{{field.property.type}}" class="form-control" ng-model="target.options[field.name]" ng-required="field.property.required"
                ng-if="field.property.type !== 'file' && field.property.type !== 'checkbox'" accesskey="tab" placeholder="{{field.property.default}}"
-               data-cy="{{field.property.title || field.name | toHuman}}">
+               data-test="{{field.property.title || field.name | toHuman}}">
 
         <label ng-if="field.property.type=='checkbox'">
             <input name="input" type="{{field.property.type}}" ng-model="target.options[field.name]" ng-required="field.property.required"
                    ng-if="field.property.type !== 'file'" accesskey="tab" placeholder="{{field.property.default}}"
-                   data-cy="{{field.property.title || field.name | toHuman}}">
+                   data-test="{{field.property.title || field.name | toHuman}}">
             {{field.property.title || field.name | toHuman}}
         </label>
 
@@ -25,7 +26,7 @@
         <error-messages input="inner.input" form="inner"></error-messages>
     </div>
 
-    <button class="btn btn-block btn-primary m-b-10" ng-disabled="!dynamicForm.$valid" ng-click="saveChanges()">Save</button>
+    <button class="btn btn-block btn-primary m-b-10" ng-disabled="!dynamicForm.$valid" ng-click="saveChanges()" data-test="Submit">Save</button>
     <span ng-repeat="action in actions">
         <button class="btn"
                 ng-class="action.class"

client/app/components/dynamic-table/dynamic-table.html

@@ -1,5 +1,5 @@
 <div class="dynamic-table-container">
-  <table class="table table-condensed table-hover">
+  <table class="table table-condensed table-hover" data-test="DynamicTable">
     <thead>
       <tr>
         <th ng-repeat="column in $ctrl.columns" ng-click="$ctrl.onColumnHeaderClick($event, column)"

client/app/components/type-picker.html

@@ -10,7 +10,7 @@
     </div>
 
     <div class="row">
-        <div class="col-lg-12 database-source">
+        <div class="col-lg-12 database-source" data-test="DatabaseSource">
             <div class="visual-card" ng-repeat="type in $ctrl.types | filter:$ctrl.filter:strict" ng-click="$ctrl.onTypeSelect(type)">
             <img ng-src="{{$ctrl.imgRoot}}/{{type.type}}.png" alt="{{type.name}}">
             <h3>{{type.name}}</h3>

client/app/pages/queries/query.html

@@ -83,7 +83,8 @@
   <main class="query-fullscreen">
     <nav resizable r-directions="['right']" r-flex="true" resizable-toggle toggle-shortcut="Alt+Shift+D, Alt+D">
       <div class="editor__left__data-source">
-        <ui-select ng-model="query.data_source_id" remove-selected="false" ng-disabled="!isQueryOwner || !sourceMode" on-select="updateDataSource()">
+        <ui-select ng-model="query.data_source_id" remove-selected="false" ng-disabled="!isQueryOwner || !sourceMode"
+          on-select="updateDataSource()" data-test="SelectDataSource">
           <ui-select-match placeholder="Select Data Source...">{{$select.selected.name}}</ui-select-match>
           <ui-select-choices repeat="ds.id as ds in dataSources | filter:$select.search">
             {{ds.name}}

client/app/services/user.js

@@ -35,12 +35,13 @@ function disableUser(user, toastr, $sanitize) {
       user.profile_image_url = data.data.profile_image_url;
       return data;
     })
-    .catch((response) => {
-      let message = response instanceof Error ? response.message : response.statusText;
-      if (!isString(message)) {
-        message = 'Unknown error';
-      }
-      toastr.error(`Cannot disable user <b>${userName}</b><br>${message}`, { allowHtml: true });
+    .catch(response => {
+      let message =
+        response.data && response.data.message
+          ? response.data.message
+          : `Cannot disable user <b>${userName}</b><br>${response.statusText}`
+
+      toastr.error(message, { allowHtml: true });
     });
 }
 

cypress.json

@@ -1,3 +1,4 @@
 {
-  "baseUrl": "http://localhost:5000"
+  "baseUrl": "http://localhost:5000",
+  "video": false
 }

cypress/cypress-server.js

@@ -1,52 +0,0 @@
-/* eslint-disable import/no-extraneous-dependencies, no-console */
-const { execSync } = require('child_process');
-const { post } = require('request');
-
-function execSetup() {
-  console.log('Running setup...');
-
-  const setupData = {
-    name: 'Example Admin',
-    email: 'admin@redash.io',
-    password: 'password',
-    org_name: 'Redash',
-  };
-
-  const baseUrl = process.env.CYPRESS_baseUrl || 'http://localhost:5000';
-
-  post(baseUrl + '/setup', { formData: setupData });
-}
-
-function startServer() {
-  console.log('Starting the server...');
-            
-  execSync('docker-compose -p cypress build --build-arg skip_ds_deps=true', { stdio: 'inherit' });
-  execSync('docker-compose -p cypress up -d', { stdio: 'inherit' });
-  execSync('docker-compose -p cypress run server create_db', { stdio: 'inherit' });
-}
-
-function stopServer() {
-  console.log('Stopping the server...');
-  execSync('docker-compose -p cypress down', { stdio: 'inherit' });
-}
-
-const command = process.argv[2];
-
-switch (command) {
-  case 'start':
-    startServer();
-    execSetup();
-    break;
-  case 'start-ci':
-    startServer();
-    break;
-  case 'setup':
-    execSetup();
-    break;
-  case 'stop':
-    stopServer();
-    break;
-  default:
-    console.log('Usage: npm run cypress:server start|stop');
-    break;
-}

cypress/cypress.js

@@ -0,0 +1,72 @@
+/* eslint-disable import/no-extraneous-dependencies, no-console */
+const atob = require('atob');
+const { execSync } = require('child_process');
+const { post } = require('request').defaults({ jar: true });
+const { seedData } = require('./seed-data');
+
+const baseUrl = process.env.CYPRESS_baseUrl || 'http://localhost:5000';
+
+function seedDatabase(seedValues) {
+  const request = seedValues.shift();
+  const data = request.type === 'form' ? { formData: request.data } : { json: request.data };
+
+  post(baseUrl + request.route, data, (err, response) => {
+    const result = response ? response.statusCode : err;
+    console.log('POST ' + request.route + ' - ' + result);
+    if (seedValues.length) {
+      seedDatabase(seedValues);
+    }
+  });
+}
+
+function startServer() {
+  console.log('Starting the server...');
+
+  execSync('docker-compose -p cypress build --build-arg skip_ds_deps=true', { stdio: 'inherit' });
+  execSync('docker-compose -p cypress up -d', { stdio: 'inherit' });
+  execSync('docker-compose -p cypress run server create_db', { stdio: 'inherit' });
+}
+
+function stopServer() {
+  console.log('Stopping the server...');
+  execSync('docker-compose -p cypress down', { stdio: 'inherit' });
+}
+
+function runCypressCI() {
+  if (process.env.PERCY_TOKEN_ENCODED) {
+    process.env.PERCY_TOKEN = atob(`${process.env.PERCY_TOKEN_ENCODED}`);
+  }
+  execSync('docker-compose run cypress ./node_modules/.bin/percy exec -- ./node_modules/.bin/cypress run --browser chrome', { stdio: 'inherit' });
+}
+
+const command = process.argv[2] || 'all';
+
+switch (command) {
+  case 'start':
+    startServer();
+    break;
+  case 'db-seed':
+    seedDatabase(seedData);
+    break;
+  case 'run':
+    execSync('cypress run --browser chrome', { stdio: 'inherit' });
+    break;
+  case 'open':
+    execSync('cypress open', { stdio: 'inherit' });
+    break;
+  case 'run-ci':
+    runCypressCI();
+    break;
+  case 'stop':
+    stopServer();
+    break;
+  case 'all':
+    startServer();
+    seedDatabase(seedData);
+    execSync('cypress run --browser chrome', { stdio: 'inherit' });
+    stopServer();
+    break;
+  default:
+    console.log('Usage: npm run cypress [start|db-seed|open|run|stop]');
+    break;
+}

cypress/integration/data-source/create_data_source_spec.js

@@ -1,18 +1,17 @@
 describe('Create Data Source', () => {
   beforeEach(() => {
     cy.login();
-    cy.visit('/data_sources');
+    cy.visit('/data_sources/new');
   });
 
   it('creates a new PostgreSQL data source', () => {
-    cy.contains('New Data Source').click();
-    cy.contains('PostgreSQL').click();
+    cy.getByTestId('DatabaseSource').contains('PostgreSQL').click();
 
-    cy.get('[name=targetName]').type('Redash');
-    cy.get('[data-cy=Host]').type('{selectall}localhost');
-    cy.get('[data-cy=User]').type('postgres');
-    cy.get('[data-cy=Password]').type('postgres');
-    cy.get('[data-cy="Database Name"]').type('postgres{enter}');
+    cy.getByTestId('TargetName').type('Redash');
+    cy.getByTestId('Host').type('{selectall}postgres');
+    cy.getByTestId('User').type('postgres');
+    cy.getByTestId('Password').type('postgres');
+    cy.getByTestId('Database Name').type('postgres{enter}');
 
     cy.contains('Saved.');
   });

cypress/integration/query/create_query_spec.js

@@ -0,0 +1,21 @@
+describe('Create Query', () => {
+  beforeEach(() => {
+    cy.login();
+    cy.visit('/queries/new');
+  });
+
+  it('executes the query', () => {
+    cy.getByTestId('SelectDataSource')
+      .click()
+      .contains('Test PostgreSQL').click();
+
+    cy.getByTestId('QueryEditor')
+      .get('.ace_text-input')
+      .type('SELECT id, name FROM organizations{esc}', { force: true });
+
+    cy.getByTestId('ExecuteButton').click();
+
+    cy.getByTestId('DynamicTable').should('exist');
+    cy.percySnapshot('Edit Query page');
+  });
+});

cypress/integration/user/login_spec.js

@@ -3,22 +3,28 @@ describe('Login', () => {
     cy.visit('/login');
   });
 
-  it('greets the user', () => {
+  it('greets the user and take a screenshot', () => {
     cy.contains('h3', 'Login to Redash');
+
+    cy.wait(1000);
+    cy.percySnapshot('Login page');
   });
 
   it('shows message on failed login', () => {
-    cy.get('#inputEmail').type('admin@redash.io');
-    cy.get('#inputPassword').type('wrongpassword{enter}');
+    cy.getByTestId('Email').type('admin@redash.io');
+    cy.getByTestId('Password').type('wrongpassword{enter}');
 
-    cy.get('.alert').should('contain', 'Wrong email or password.');
+    cy.getByTestId('ErrorMessage').should('contain', 'Wrong email or password.');
   });
 
   it('navigates to homepage with successful login', () => {
-    cy.get('#inputEmail').type('admin@redash.io');
-    cy.get('#inputPassword').type('password{enter}');
+    cy.getByTestId('Email').type('admin@redash.io');
+    cy.getByTestId('Password').type('password{enter}');
 
     cy.title().should('eq', 'Redash');
     cy.contains('Example Admin');
+
+    cy.wait(1000);
+    cy.percySnapshot('Homepage');
   });
 });

cypress/integration/user/logout_spec.js

@@ -5,7 +5,7 @@ describe('Logout', () => {
   });
 
   it('shows login page after logout', () => {
-    cy.get('[data-cy=dropdown-profile]').click();
+    cy.getByTestId('ProfileDropdown').click();
     cy.contains('Log out').click();
 
     cy.title().should('eq', 'Login to Redash');

cypress/seed-data.js

@@ -0,0 +1,35 @@
+exports.seedData = [
+  {
+    route: '/setup',
+    type: 'form',
+    data: {
+      name: 'Example Admin',
+      email: 'admin@redash.io',
+      password: 'password',
+      org_name: 'Redash',
+    },
+  },
+  {
+    route: '/login',
+    type: 'form',
+    data: {
+      email: 'admin@redash.io',
+      password: 'password',
+    },
+  },
+  {
+    route: '/api/data_sources',
+    type: 'json',
+    data: {
+      name: 'Test PostgreSQL',
+      options: {
+        dbname: 'postgres',
+        host: 'postgres',
+        port: 5432,
+        sslmode: 'prefer',
+        user: 'postgres',
+      },
+      type: 'pg',
+    },
+  },
+];

cypress/support/commands.js

@@ -1,3 +1,5 @@
+import '@percy/cypress';
+
 Cypress.Commands.add('login', () => {
   const users = {
     admin: {
@@ -13,3 +15,5 @@ Cypress.Commands.add('login', () => {
     body: users.admin,
   });
 });
+
+Cypress.Commands.add('getByTestId', element => cy.get('[data-test="' + element + '"]'));

package.json

@@ -13,11 +13,8 @@
     "analyze:build": "npm run clean && NODE_ENV=production BUNDLE_ANALYZER=on webpack",
     "test": "jest",
     "test:watch": "jest --watch",
-    "cypress:install": "npm install --no-save cypress",
-    "cypress:server": "node cypress/cypress-server.js",
-    "cypress:run": "cypress run",
-    "cypress:open": "cypress open",
-    "cypress": "npm run cypress:server start && cypress run && npm run cypress:server stop"
+    "cypress:install": "npm install --no-save cypress @percy/cypress",
+    "cypress": "node cypress/cypress.js"
   },
   "repository": {
     "type": "git",

redash/handlers/users.py

@@ -230,7 +230,7 @@ class UserDisableResource(BaseResource):
         # admin cannot disable self; current user is an admin (`@require_admin`)
         # so just check user id
         if user.id == current_user.id:
-            abort(400, message="You cannot disable your own account. "
+            abort(403, message="You cannot disable your own account. "
                                "Please ask another admin to do this for you.")
         user.disable()
         models.db.session.commit()

redash/templates/login.html

@@ -8,7 +8,7 @@
     {% with messages = get_flashed_messages() %}
       {% if messages %}
         {% for message in messages %}
-          <div class="alert alert-danger" role="alert">{{ message }}</div>
+          <div class="alert alert-danger" role="alert" data-test="ErrorMessage">{{ message }}</div>
         {% endfor %}
       {% endif %}
     {% endwith %}
@@ -41,11 +41,11 @@
       <input type="hidden" name="remember" value="on">
       <div class="form-group">
         <label for="inputEmail">{{ username_prompt or 'Email' }}</label>
-        <input type="text" class="form-control" id="inputEmail" name="email" value="{{email}}">
+        <input type="text" class="form-control" id="inputEmail" name="email" value="{{email}}" data-test="Email">
       </div>
       <div class="form-group">
         <label for="inputPassword">Password</label>
-        <input type="password" class="form-control" id="inputPassword" name="password">
+        <input type="password" class="form-control" id="inputPassword" name="password" data-test="Password">
       </div>
 
       <button type="submit" class="btn btn-primary btn-block m-t-25">Log In</button>

tests/handlers/test_users.py

@@ -203,7 +203,7 @@ class TestUserDisable(BaseTestCase):
         self.assertFalse(admin_user.is_disabled)
 
         rv = self.make_request('post', "/api/users/{}/disable".format(admin_user.id), user=admin_user)
-        self.assertEqual(rv.status_code, 400)
+        self.assertEqual(rv.status_code, 403)
 
         # user should stay enabled
         admin_user = models.User.query.get(admin_user.id)