Fix: Refresh schedule interval count doesn't adhere to permission rules (getredash#3265)

ranbena · arikfr · commit de04a403d7c4 · 2019-01-13T11:39:57.000+02:00
* Fix: Refresh schedule interval count doesn't adhere to permission rules

* Fix “4.28 weeks” to “30 days”

* Merge interval type and count into one &lt;Select&gt;
diff --git a/client/app/components/queries/ScheduleDialog.jsx b/client/app/components/queries/ScheduleDialog.jsx
@@ -6,25 +6,17 @@ import DatePicker from 'antd/lib/date-picker';
 import TimePicker from 'antd/lib/time-picker';
 import Select from 'antd/lib/select';
 import Radio from 'antd/lib/radio';
-import { range, clone, isEqual } from 'lodash';
+import { capitalize, clone, isEqual } from 'lodash';
 import moment from 'moment';
-import { secondsToInterval, intervalToSeconds, IntervalEnum, localizeTime } from '@/filters';
+import { secondsToInterval, IntervalEnum, localizeTime } from '@/filters';
 
 import './ScheduleDialog.css';
 
 const WEEKDAYS_SHORT = moment.weekdaysShort();
 const WEEKDAYS_FULL = moment.weekdays();
-const INTERVAL_OPTIONS_MAP = {
-  [IntervalEnum.NEVER]: 1,
-  [IntervalEnum.MINUTES]: 60,
-  [IntervalEnum.HOURS]: 24,
-  [IntervalEnum.DAYS]: 7,
-  [IntervalEnum.WEEKS]: 5,
-};
-
 const DATE_FORMAT = 'YYYY-MM-DD';
 const HOUR_FORMAT = 'HH:mm';
-const Option = Select.Option;
+const { Option, OptGroup } = Select;
 
 class ScheduleDialog extends React.Component {
   static propTypes = {
@@ -44,31 +36,31 @@ class ScheduleDialog extends React.Component {
 
   get initState() {
     const newSchedule = clone(this.props.query.schedule);
-    const { time, interval: secs, day_of_week: day } = newSchedule;
-    const interval = secs ? secondsToInterval(secs) : {};
+    const { time, interval: seconds, day_of_week: day } = newSchedule;
+    const { interval } = secondsToInterval(seconds);
     const [hour, minute] = time ? localizeTime(time).split(':') : [null, null];
 
     return {
       hour,
       minute,
-      count: interval.count ? String(interval.count) : '1',
-      interval: interval.interval || IntervalEnum.NEVER,
+      seconds,
+      interval,
       dayOfWeek: day ? WEEKDAYS_SHORT[WEEKDAYS_FULL.indexOf(day)] : null,
       newSchedule,
     };
   }
 
   get intervals() {
-    const ret = this.props.refreshOptions
-      .map((seconds) => {
-        const { count, interval } = secondsToInterval(seconds);
-        return count === 1 ? {
-          name: interval,
-          time: seconds,
-        } : null;
-      })
-      .filter(x => x !== null);
-    ret.unshift({ name: IntervalEnum.NEVER });
+    const ret = {
+      [IntervalEnum.NEVER]: [],
+    };
+    this.props.refreshOptions.forEach((seconds) => {
+      const { count, interval } = secondsToInterval(seconds);
+      if (!(interval in ret)) {
+        ret[interval] = [];
+      }
+      ret[interval].push([count, seconds]);
+    });
 
     Object.defineProperty(this, 'intervals', { value: ret }); // memoize
 
@@ -81,16 +73,15 @@ class ScheduleDialog extends React.Component {
     });
   }
 
-  getCounts = interval => range(1, INTERVAL_OPTIONS_MAP[interval])
-
   setTime = (time) => {
     this.newSchedule = {
       time: moment(time).utc().format(HOUR_FORMAT),
     };
   };
 
-  setInterval = (newInterval) => {
+  setInterval = (newSeconds) => {
     const { newSchedule } = this.state;
+    const { interval: newInterval } = secondsToInterval(newSeconds);
 
     // resets to defaults
     if (newInterval === IntervalEnum.NEVER) {
@@ -116,23 +107,15 @@ class ScheduleDialog extends React.Component {
       newSchedule.day_of_week = WEEKDAYS_FULL[0];
     }
 
-    // reset count if out of new interval count range
-    let count = this.state.count;
-    if (this.getCounts(newInterval).indexOf(Number(count)) === -1) {
-      count = '1';
-    }
-
-    newSchedule.interval = newInterval
-      ? intervalToSeconds(Number(count), newInterval)
-      : null;
+    newSchedule.interval = newSeconds;
 
     const [hour, minute] = newSchedule.time ?
       localizeTime(newSchedule.time).split(':')
       : [null, null];
 
     this.setState({
       interval: newInterval,
-      count,
+      seconds: newSeconds,
       hour,
       minute,
       dayOfWeek: newSchedule.day_of_week
@@ -143,12 +126,6 @@ class ScheduleDialog extends React.Component {
     this.newSchedule = newSchedule;
   };
 
-  setCount = (newCount) => {
-    const totalSeconds = intervalToSeconds(parseInt(newCount, 10), this.state.interval);
-    this.setState({ count: newCount });
-    this.newSchedule = { interval: totalSeconds };
-  };
-
   setScheduleUntil = (_, date) => {
     this.newSchedule = { until: date };
   };
@@ -182,7 +159,7 @@ class ScheduleDialog extends React.Component {
 
   render() {
     const {
-      interval, minute, hour, count, newSchedule: { until },
+      interval, minute, hour, seconds, newSchedule: { until },
     } = this.state;
     const fixZIndex = { getPopupContainer: () => this.modalRef.current };
     const selectProps = {
@@ -202,17 +179,15 @@ class ScheduleDialog extends React.Component {
         <div className="schedule-component" ref={this.modalRef}>
           <h5>Refresh every</h5>
           <div>
-            {interval !== IntervalEnum.NEVER ? (
-              <Select value={count} onChange={this.setCount} {...selectProps}>
-                {this.getCounts(this.state.interval).map(cnt => (
-                  <Option value={String(cnt)} key={cnt}>{cnt}</Option>
+            <Select value={seconds} onChange={this.setInterval} {...selectProps}>
+              <Option value={null} key="never">Never</Option>
+              {Object.keys(this.intervals).map(int => (
+                <OptGroup label={capitalize(int)} key={int}>
+                  {this.intervals[int].map(([cnt, secs]) => (
+                    <Option value={secs} key={cnt}>{cnt} {int}</Option>
+                  ))}
+                </OptGroup>
                 ))}
-              </Select>
-            ) : null}
-            <Select value={interval} onChange={this.setInterval} {...selectProps}>
-              {this.intervals.map(iv => (
-                <Option value={iv.name} key={iv.name}>{iv.name}</Option>
-              ))}
             </Select>
           </div>
         </div>
diff --git a/client/app/filters/index.js b/client/app/filters/index.js
@@ -20,6 +20,9 @@ export function localizeTime(time) {
 }
 
 export function secondsToInterval(seconds) {
+  if (!seconds) {
+    return { interval: IntervalEnum.NEVER };
+  }
   let interval = IntervalEnum.MINUTES;
   let count = seconds / 60;
   if (count >= 60) {
@@ -30,7 +33,7 @@ export function secondsToInterval(seconds) {
     count /= 24;
     interval = IntervalEnum.DAYS;
   }
-  if (count >= 7 && interval === IntervalEnum.DAYS) {
+  if (count >= 7 && !(count % 7) && interval === IntervalEnum.DAYS) {
     count /= 7;
     interval = IntervalEnum.WEEKS;
   }

Original file line number	Diff line number	Diff line change
`@@ -20,6 +20,9 @@ export function localizeTime(time) {`
`20`	`20`	`}`
`21`	`21`
`22`	`22`	`export function secondsToInterval(seconds) {`
	`23`	`+ if (!seconds) {`
	`24`	`+ return { interval: IntervalEnum.NEVER };`
	`25`	`+ }`
`23`	`26`	`let interval = IntervalEnum.MINUTES;`
`24`	`27`	`let count = seconds / 60;`
`25`	`28`	`if (count >= 60) {`
`@@ -30,7 +33,7 @@ export function secondsToInterval(seconds) {`
`30`	`33`	`count /= 24;`
`31`	`34`	`interval = IntervalEnum.DAYS;`
`32`	`35`	`}`
`33`		`- if (count >= 7 && interval === IntervalEnum.DAYS) {`
	`36`	`+ if (count >= 7 && !(count % 7) && interval === IntervalEnum.DAYS) {`
`34`	`37`	`count /= 7;`
`35`	`38`	`interval = IntervalEnum.WEEKS;`
`36`	`39`	`}`