Addressing coments

.codeclimate.yml

@@ -32,3 +32,4 @@ exclude_paths:
 - "caravel/assets/node_modules/"
 - "caravel/assets/javascripts/dist/"
 - "caravel/migrations"
+- "docs/"

caravel/assets/javascripts/SqlLab/components/HighlightedSql.jsx

@@ -2,7 +2,7 @@ import React from 'react';
 import { Well } from 'react-bootstrap';
 import SyntaxHighlighter from 'react-syntax-highlighter';
 import { github } from 'react-syntax-highlighter/dist/styles';
-import ModalTrigger from '../../components/ModalTrigger'
+import ModalTrigger from '../../components/ModalTrigger';
 
 const defaultProps = {
   maxWidth: 50,
@@ -18,52 +18,75 @@ const propTypes = {
   shrink: React.PropTypes.bool,
 };
 
-const HighlightedSql = (props) => {
-  const sql = props.sql || '';
-  let lines = sql.split('\n');
-  if (lines.length >= props.maxLines) {
-    lines = lines.slice(0, props.maxLines);
-    lines.push('{...}');
+class HighlightedSql extends React.Component {
+  constructor(props) {
+    super(props);
+    this.state = {
+      modalBody: null,
+    };
   }
-  let shownSql = sql;
-  if (props.shrink) {
-    shownSql = lines.map((line) => {
+  shrinkSql() {
+    const props = this.props;
+    const sql = props.sql || '';
+    let lines = sql.split('\n');
+    if (lines.length >= props.maxLines) {
+      lines = lines.slice(0, props.maxLines);
+      lines.push('{...}');
+    }
+    return lines.map((line) => {
       if (line.length > props.maxWidth) {
         return line.slice(0, props.maxWidth) + '{...}';
       }
       return line;
     })
     .join('\n');
   }
-  return (
-    <ModalTrigger
-      modalTitle="SQL"
-      triggerNode={
-        <Well>
+  triggerNode() {
+    const props = this.props;
+    let shownSql = props.shrink ? this.shrinkSql(props.sql) : props.sql;
+    return (
+      <Well>
+        <SyntaxHighlighter language="sql" style={github}>
+          {shownSql}
+        </SyntaxHighlighter>
+      </Well>);
+  }
+  generateModal() {
+    const props = this.props;
+    let rawSql;
+    if (props.rawSql && props.rawSql !== this.props.sql) {
+      rawSql = (
+        <div>
+          <h4>Raw SQL</h4>
           <SyntaxHighlighter language="sql" style={github}>
-            {shownSql}
+            {props.rawSql}
           </SyntaxHighlighter>
-        </Well>
-      }
-      modalBody={
+        </div>
+      );
+    }
+    this.setState({
+      modalBody: (
         <div>
           <h4>Source SQL</h4>
           <SyntaxHighlighter language="sql" style={github}>
-            {props.sql}
+            {this.props.sql}
           </SyntaxHighlighter>
-          {props.rawSql &&
-            <div>
-              <h4>Raw SQL</h4>
-              <SyntaxHighlighter language="sql" style={github}>
-                {props.rawSql}
-              </SyntaxHighlighter>
-            </div>
-          }
+          {rawSql}
         </div>
-      }
-    />
-  );
-};
+      ),
+    });
+  }
+  render() {
+    return (
+      <ModalTrigger
+        modalTitle="SQL"
+        triggerNode={this.triggerNode()}
+        modalBody={this.state.modalBody}
+        beforeOpen={this.generateModal.bind(this)}
+      />
+    );
+  }
+}
 HighlightedSql.propTypes = propTypes;
 HighlightedSql.defaultProps = defaultProps;
 

caravel/assets/javascripts/SqlLab/components/QueryTable.jsx

@@ -90,7 +90,6 @@ class QueryTable extends React.Component {
         </button>
       );
       q.started = moment(q.startDttm).format('HH:mm:ss');
-      const source = (q.ctas) ? q.executedSql : q.sql;
       q.sql = (
         <HighlightedSql sql={q.sql} rawSql={q.executedSql} shrink maxWidth={60} />
       );

caravel/assets/javascripts/components/ModalTrigger.jsx

@@ -5,7 +5,7 @@ import cx from 'classnames';
 const propTypes = {
   triggerNode: PropTypes.node.isRequired,
   modalTitle: PropTypes.node.isRequired,
-  modalBody: PropTypes.node.isRequired,
+  modalBody: PropTypes.node,  // not required because it can be generated by beforeOpen
   beforeOpen: PropTypes.func,
   onExit: PropTypes.func,
   isButton: PropTypes.bool,
@@ -46,10 +46,8 @@ export default class ModalTrigger extends React.Component {
       'btn btn-default btn-sm': this.props.isButton,
     });
     return (
-      <div>
-        <span className={classNames} onClick={this.open} style={{ cursor: 'pointer' }}>
+      <span className={classNames} onClick={this.open} style={{ cursor: 'pointer' }}>
           {this.props.triggerNode}
-        </span>
         <Modal
           show={this.state.showModal}
           onHide={this.close}
@@ -64,7 +62,7 @@ export default class ModalTrigger extends React.Component {
             {this.props.modalBody}
           </Modal.Body>
         </Modal>
-      </div>
+      </span>
     );
   }
 }

caravel/assets/spec/javascripts/sqllab/HighlightedSql_spec.jsx

@@ -1,26 +1,33 @@
 import React from 'react';
 import HighlightedSql from '../../../javascripts/SqlLab/components/HighlightedSql';
+import ModalTrigger from '../../../javascripts/components/ModalTrigger';
 import SyntaxHighlighter from 'react-syntax-highlighter';
-import { shallow } from 'enzyme';
+import { mount, shallow } from 'enzyme';
 import { describe, it } from 'mocha';
 import { expect } from 'chai';
 
 
 describe('HighlightedSql', () => {
   const sql = "SELECT * FROM test WHERE something='fkldasjfklajdslfkjadlskfjkldasjfkladsjfkdjsa'";
-  it('renders', () => {
-    expect(React.isValidElement(<HighlightedSql />)).to.equal(true);
-  });
   it('renders with props', () => {
     expect(React.isValidElement(<HighlightedSql sql={sql} />))
     .to.equal(true);
   });
-  it('renders a SyntaxHighlighter', () => {
+  it('renders a ModalTrigger', () => {
     const wrapper = shallow(<HighlightedSql sql={sql} />);
-    expect(wrapper.find(SyntaxHighlighter)).to.have.length(1);
+    expect(wrapper.find(ModalTrigger)).to.have.length(1);
   });
-  it('renders a SyntaxHighlighter while using shrink', () => {
+  it('renders a ModalTrigger while using shrink', () => {
     const wrapper = shallow(<HighlightedSql sql={sql} shrink maxWidth={20} />);
-    expect(wrapper.find(SyntaxHighlighter)).to.have.length(1);
+    expect(wrapper.find(ModalTrigger)).to.have.length(1);
+  });
+  it('renders two SyntaxHighlighter in modal', () => {
+    const wrapper = mount(
+      <HighlightedSql sql={sql} rawSql="SELECT * FORM foo" shrink maxWidth={5} />);
+    const well = wrapper.find('.well');
+    expect(well).to.have.length(1);
+    well.simulate('click');
+    const modalBody = mount(wrapper.state().modalBody);
+    expect(modalBody.find(SyntaxHighlighter)).to.have.length(2);
   });
 });

caravel/jinja_context.py

@@ -7,10 +7,8 @@
 import inspect
 import jinja2
 
-from copy import copy
 from datetime import datetime, timedelta
 from dateutil.relativedelta import relativedelta
-import logging
 import time
 import textwrap
 import uuid
@@ -93,8 +91,6 @@ def _partition_query(table_name, limit=0, order_by=None, filters=None):
 
     @staticmethod
     def _schema_table(table_name, schema):
-        for i in range(10):
-            print([table_name, schema])
         if '.' in table_name:
             schema, table_name = table_name.split('.')
         return table_name, schema

docs/conf.py

@@ -16,7 +16,6 @@
 import os
 import shlex
 import sphinx_rtd_theme
-# import sphinx_bootstrap_theme
 
 # If extensions (or modules to document with autodoc) are in another directory,
 # add these directories to sys.path here. If the directory is relative to the
@@ -114,26 +113,15 @@
 
 # The theme to use for HTML and HTML Help pages.  See the documentation for
 # a list of builtin themes.
-# html_theme = 'bootstrap'
-# html_theme_path = sphinx_bootstrap_theme.get_html_theme_path()
 html_theme = "sphinx_rtd_theme"
 html_theme_path = [sphinx_rtd_theme.get_html_theme_path()]
 
 # Theme options are theme-specific and customize the look and feel of a theme
 # further.  For a list of options available for each theme, see the
 # documentation.
-html_theme_options = {
-    # 'bootswatch_theme': 'cosmo',
-    #'navbar_title': 'Caravel Documentation',
-    #'navbar_fixed_top': "false",
-    #'navbar_sidebarrel': False,
-    #'navbar_site_name': "Topics",
-    #'navbar_class': "navbar navbar-left",
-}
 html_theme_options = {
     'collapse_navigation': False,
     'display_version': False,
-    #'navigation_depth': 3,
 }
 
 # Add any paths that contain custom themes here, relative to this directory.

run_specific_test.sh

@@ -5,4 +5,4 @@ export CARAVEL_CONFIG=tests.caravel_test_config
 set -e
 caravel/bin/caravel version -v
 export SOLO_TEST=1
-nosetests tests.core_tests:CoreTests
+nosetests tests.core_tests:CoreTests.test_templated_sql_json

tests/core_tests.py

@@ -443,6 +443,11 @@ def test_process_template(self):
         rendered = jinja_context.process_template(sql)
         self.assertEqual("SELECT '2017-01-01T00:00:00'", rendered)
 
+    def test_templated_sql_json(self):
+        sql = "SELECT '{{ datetime(2017, 1, 1).isoformat() }}' as test"
+        data = self.run_sql(sql, "admin", "fdaklj3ws")
+        self.assertEqual(data['data'][0]['test'], "2017-01-01T00:00:00")
+
 
 if __name__ == '__main__':
     unittest.main()