Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Jan 18, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions client/app/components/proptypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
20 changes: 15 additions & 5 deletions client/app/components/queries/ScheduleDialog.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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];
Expand Down Expand Up @@ -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();
}
Expand Down
15 changes: 4 additions & 11 deletions client/app/components/queries/ScheduleDialog.test.js
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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];
}
Expand Down Expand Up @@ -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', () => {
Expand Down
7 changes: 4 additions & 3 deletions client/app/components/queries/SchedulePhrase.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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'];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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]}
Expand All @@ -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}
Expand All @@ -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]}
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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}
/>
}
Expand All @@ -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
Expand Down Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion client/app/pages/queries/query.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
7 changes: 1 addition & 6 deletions client/app/services/query.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: {},
});
Expand Down
56 changes: 56 additions & 0 deletions migrations/versions/73beceabb948_bring_back_null_schedule.py
Original file line number Diff line number Diff line change
@@ -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
12 changes: 6 additions & 6 deletions redash/models/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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()
Expand Down
3 changes: 3 additions & 0 deletions redash/models/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
2 changes: 1 addition & 1 deletion tests/factories.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
Loading