Use None as "not scheduled" default value of a query (#3277)

* Use null as the default scheduled value.

* Don't serialize None to json, so we can use SQL is not null predicate.

* Fix warning about unicode in tests

* Handling empty query.schedule in UI (#3283)

* Add migration to convert empty schedules to null and drop the not null contraint.

client/app/components/proptypes.js

@@ -15,6 +15,20 @@ export const Table = PropTypes.shape({
 
 export const Schema = PropTypes.arrayOf(Table);
 
+export const RefreshScheduleType = PropTypes.shape({
+  interval: PropTypes.number,
+  time: PropTypes.string,
+  day_of_week: PropTypes.string,
+  until: PropTypes.string,
+});
+
+export const RefreshScheduleDefault = {
+  interval: null,
+  time: null,
+  day_of_week: null,
+  until: null,
+};
+
 export const Field = PropTypes.shape({
   name: PropTypes.string.isRequired,
   title: PropTypes.string,

client/app/components/queries/ScheduleDialog.jsx

@@ -9,6 +9,7 @@ import Radio from 'antd/lib/radio';
 import { capitalize, clone, isEqual } from 'lodash';
 import moment from 'moment';
 import { secondsToInterval, durationHumanize, pluralize, IntervalEnum, localizeTime } from '@/filters';
+import { RefreshScheduleType, RefreshScheduleDefault } from '../proptypes';
 
 import './ScheduleDialog.css';
 
@@ -21,21 +22,24 @@ const { Option, OptGroup } = Select;
 export class ScheduleDialog extends React.Component {
   static propTypes = {
     show: PropTypes.bool.isRequired,
-    // eslint-disable-next-line react/forbid-prop-types
-    query: PropTypes.object.isRequired,
+    schedule: RefreshScheduleType,
     refreshOptions: PropTypes.arrayOf(PropTypes.number).isRequired,
     updateQuery: PropTypes.func.isRequired,
     onClose: PropTypes.func.isRequired,
   };
 
+  static defaultProps = {
+    schedule: RefreshScheduleDefault,
+  };
+
   constructor(props) {
     super(props);
     this.state = this.initState;
     this.modalRef = React.createRef(); // used by <Select>
   }
 
   get initState() {
-    const newSchedule = clone(this.props.query.schedule);
+    const newSchedule = clone(this.props.schedule || ScheduleDialog.defaultProps.schedule);
     const { time, interval: seconds, day_of_week: day } = newSchedule;
     const { interval } = secondsToInterval(seconds);
     const [hour, minute] = time ? localizeTime(time).split(':') : [null, null];
@@ -144,9 +148,15 @@ export class ScheduleDialog extends React.Component {
   }
 
   save() {
+    const { newSchedule } = this.state;
+
     // save if changed
-    if (!isEqual(this.state.newSchedule, this.props.query.schedule)) {
-      this.props.updateQuery({ schedule: clone(this.state.newSchedule) });
+    if (!isEqual(newSchedule, this.props.schedule)) {
+      if (newSchedule.interval) {
+        this.props.updateQuery({ schedule: clone(newSchedule) });
+      } else {
+        this.props.updateQuery({ schedule: null });
+      }
     }
     this.props.onClose();
   }

client/app/components/queries/ScheduleDialog.test.js

@@ -1,17 +1,11 @@
 import React from 'react';
 import { mount } from 'enzyme';
 import { ScheduleDialog } from './ScheduleDialog';
+import RefreshScheduleDefault from '../proptypes';
 
 const defaultProps = {
   show: true,
-  query: {
-    schedule: {
-      time: null,
-      until: null,
-      interval: null,
-      day_of_week: null,
-    },
-  },
+  schedule: RefreshScheduleDefault,
   refreshOptions: [
     60, 300, 600, // 1, 5 ,10 mins
     3600, 36000, 82800, // 1, 10, 23 hours
@@ -23,12 +17,11 @@ const defaultProps = {
 };
 
 function getWrapper(schedule = {}, props = {}) {
-  const defaultSchedule = defaultProps.query.schedule;
   props = Object.assign(
     {},
     defaultProps,
     props,
-    { query: { schedule: Object.assign({}, defaultSchedule, schedule) } },
+    { schedule: Object.assign({}, RefreshScheduleDefault, schedule) },
   );
   return [mount(<ScheduleDialog {...props} />), props];
 }
@@ -78,7 +71,7 @@ describe('ScheduleDialog', () => {
       const [wrapper] = getWrapper({
         interval: 1209600,
         time: '22:15',
-        day_of_week: 2,
+        day_of_week: 'Monday',
       });
 
       test('Sets to correct interval', () => {

client/app/components/queries/SchedulePhrase.jsx

@@ -3,23 +3,24 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import Tooltip from 'antd/lib/tooltip';
 import { localizeTime, durationHumanize } from '@/filters';
+import { RefreshScheduleType, RefreshScheduleDefault } from '../proptypes';
 
 import './ScheduleDialog.css';
 
 class SchedulePhrase extends React.Component {
   static propTypes = {
-    // eslint-disable-next-line react/forbid-prop-types
-    schedule: PropTypes.object.isRequired,
+    schedule: RefreshScheduleType,
     isNew: PropTypes.bool.isRequired,
     isLink: PropTypes.bool,
   };
 
   static defaultProps = {
+    schedule: RefreshScheduleDefault,
     isLink: false,
   };
 
   get content() {
-    const { interval: seconds } = this.props.schedule;
+    const { interval: seconds } = this.props.schedule || SchedulePhrase.defaultProps.schedule;
     if (!seconds) {
       return ['Never'];
     }

client/app/components/queries/__snapshots__/ScheduleDialog.test.js.snap

@@ -1632,6 +1632,7 @@ exports[`ScheduleDialog Sets correct schedule settings Sets to "2 Weeks 22:15 Tu
 >
   <RadioGroup
     buttonStyle="outline"
+    defaultValue="Mon"
     disabled={false}
     onChange={[Function]}
     size="medium"
@@ -1700,7 +1701,7 @@ exports[`ScheduleDialog Sets correct schedule settings Sets to "2 Weeks 22:15 Tu
         value="Mon"
       >
         <Radio
-          checked={false}
+          checked={true}
           className="input"
           disabled={false}
           onChange={[Function]}
@@ -1709,10 +1710,10 @@ exports[`ScheduleDialog Sets correct schedule settings Sets to "2 Weeks 22:15 Tu
           value="Mon"
         >
           <label
-            className="input ant-radio-button-wrapper"
+            className="input ant-radio-button-wrapper ant-radio-button-wrapper-checked"
           >
             <Checkbox
-              checked={false}
+              checked={true}
               className=""
               defaultChecked={false}
               disabled={false}
@@ -1725,11 +1726,11 @@ exports[`ScheduleDialog Sets correct schedule settings Sets to "2 Weeks 22:15 Tu
               value="Mon"
             >
               <span
-                className="ant-radio-button"
+                className="ant-radio-button ant-radio-button-checked"
                 style={Object {}}
               >
                 <input
-                  checked={false}
+                  checked={true}
                   className="ant-radio-button-input"
                   disabled={false}
                   onBlur={[Function]}
@@ -2375,7 +2376,6 @@ exports[`ScheduleDialog Sets correct schedule settings Sets to "Never" 1`] = `
     onChange={[Function]}
     showSearch={false}
     transitionName="slide-up"
-    value={null}
   >
     <Select
       allowClear={false}
@@ -2445,7 +2445,6 @@ exports[`ScheduleDialog Sets correct schedule settings Sets to "Never" 1`] = `
       tags={false}
       tokenSeparators={Array []}
       transitionName="slide-up"
-      value={null}
     >
       <SelectTrigger
         ariaId="test-uuid"
@@ -2478,11 +2477,7 @@ exports[`ScheduleDialog Sets correct schedule settings Sets to "Never" 1`] = `
         }
         showSearch={false}
         transitionName="slide-up"
-        value={
-          Array [
-            null,
-          ]
-        }
+        value={Array []}
         visible={false}
       >
         <Trigger
@@ -2577,11 +2572,7 @@ exports[`ScheduleDialog Sets correct schedule settings Sets to "Never" 1`] = `
               onMenuSelect={[Function]}
               onPopupFocus={[Function]}
               prefixCls="ant-select-dropdown"
-              value={
-                Array [
-                  null,
-                ]
-              }
+              value={Array []}
               visible={false}
             />
           }
@@ -2599,11 +2590,7 @@ exports[`ScheduleDialog Sets correct schedule settings Sets to "Never" 1`] = `
           }
           showSearch={false}
           transitionName="slide-up"
-          value={
-            Array [
-              null,
-            ]
-          }
+          value={Array []}
           visible={false}
         >
           <div
@@ -2631,21 +2618,7 @@ exports[`ScheduleDialog Sets correct schedule settings Sets to "Never" 1`] = `
             >
               <div
                 className="ant-select-selection__rendered"
-              >
-                <div
-                  className="ant-select-selection-selected-value"
-                  key="value"
-                  style={
-                    Object {
-                      "display": "block",
-                      "opacity": 1,
-                    }
-                  }
-                  title="Never"
-                >
-                  Never
-                </div>
-              </div>
+              />
               <span
                 className="ant-select-arrow"
                 key="arrow"

client/app/pages/queries/query.html

@@ -135,7 +135,7 @@ <h3>
                 <span class="zmdi zmdi-refresh"></span> Refresh Schedule</span>
             </td>
             <td class="p-t-15 text-right">
-              <schedule-dialog show="showScheduleForm" query="query" refresh-options="refreshOptions" update-query="updateQueryMetadata" on-close="closeScheduleForm"></schedule-dialog>
+              <schedule-dialog show="showScheduleForm" schedule="query.schedule" refresh-options="refreshOptions" update-query="updateQueryMetadata" on-close="closeScheduleForm"></schedule-dialog>
               <schedule-phrase ng-click="openScheduleForm()" is-link="true" schedule="query.schedule" is-new="query.isNew()" />
             </td>
           </tr>

client/app/services/query.js

@@ -375,12 +375,7 @@ function QueryResource(
     return new Query({
       query: '',
       name: 'New Query',
-      schedule: {
-        time: null,
-        until: null,
-        interval: null,
-        day_of_week: null,
-      },
+      schedule: null,
       user: currentUser,
       options: {},
     });

migrations/versions/73beceabb948_bring_back_null_schedule.py

@@ -0,0 +1,56 @@
+"""bring_back_null_schedule
+
+Revision ID: 73beceabb948
+Revises: e7f8a917aa8e
+Create Date: 2019-01-17 13:22:21.729334
+
+"""
+from alembic import op
+import sqlalchemy as sa
+from sqlalchemy.dialects import postgresql
+from sqlalchemy.sql import table
+
+from redash.models import MutableDict, PseudoJSON
+
+# revision identifiers, used by Alembic.
+revision = '73beceabb948'
+down_revision = 'e7f8a917aa8e'
+branch_labels = None
+depends_on = None
+
+
+def is_empty_schedule(schedule):
+    if schedule is None:
+        return False
+
+    if schedule == {}:
+        return True
+
+    if schedule.get('interval') is None and schedule.get('until') is None and schedule.get('day_of_week') is None and schedule.get('time') is None:
+        return True
+
+    return False
+
+
+def upgrade():
+    op.alter_column('queries', 'schedule',
+                    nullable=True,
+                    server_default=None)
+
+    queries = table(
+        'queries',
+        sa.Column('id', sa.Integer, primary_key=True),
+        sa.Column('schedule', MutableDict.as_mutable(PseudoJSON)))
+
+    conn = op.get_bind()
+    for query in conn.execute(queries.select()):
+        if is_empty_schedule(query.schedule):
+            conn.execute(
+                queries
+                .update()
+                .where(queries.c.id == query.id)
+                .values(schedule=None))
+
+
+def downgrade():
+    pass

redash/models/__init__.py

@@ -443,7 +443,7 @@ def __str__(self):
     def archive(self, user=None):
         db.session.add(self)
         self.is_archived = True
-        self.schedule = {}
+        self.schedule = None
 
         for vis in self.visualizations:
             for w in vis.widgets:
@@ -550,11 +550,11 @@ def by_user(cls, user):
 
     @classmethod
     def outdated_queries(cls):
-        queries = (db.session.query(Query)
-                   .options(joinedload(Query.latest_query_data).load_only('retrieved_at'))
-                   .filter(Query.schedule != {})
-                   .order_by(Query.id))
-
+        queries = (Query.query
+                        .options(joinedload(Query.latest_query_data).load_only('retrieved_at'))
+                        .filter(Query.schedule.isnot(None))
+                        .order_by(Query.id))
+        
         now = utils.utcnow()
         outdated_queries = {}
         scheduled_queries_executions.refresh()

redash/models/types.py

@@ -24,6 +24,9 @@ class PseudoJSON(TypeDecorator):
     impl = db.Text
 
     def process_bind_param(self, value, dialect):
+        if value is None:
+            return value
+
         return json_dumps(value)
 
     def process_result_value(self, value, dialect):

tests/factories.py

@@ -75,7 +75,7 @@ def __call__(self):
                              user=user_factory.create,
                              is_archived=False,
                              is_draft=False,
-                             schedule={},
+                             schedule=None,
                              data_source=data_source_factory.create,
                              org_id=1)