ui: De-duplicate and clarify time selection state

pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.fixture.ts

@@ -146,7 +146,7 @@ export const getStatementDetailsPropsFixture = (): StatementDetailsProps => ({
   timeScale: {
     windowSize: moment.duration(5, "day"),
     sampleSize: moment.duration(5, "minutes"),
-    windowEnd: moment.utc("2021.12.12"),
+    fixedWindowEnd: moment.utc("2021.12.12"),
     key: "Custom",
   },
   statement: {

pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.fixture.ts

@@ -786,7 +786,7 @@ const statementsPagePropsFixture: StatementsPageProps = {
   timeScale: {
     windowSize: moment.duration(5, "day"),
     sampleSize: moment.duration(5, "minutes"),
-    windowEnd: moment.utc("2021.12.12"),
+    fixedWindowEnd: moment.utc("2021.12.12"),
     key: "Custom"
   },
   apps: ["$ internal", "movr", "$ cockroach demo"],

pkg/ui/workspaces/cluster-ui/src/timeScaleDropdown/timeScaleDropdown.tsx

@@ -16,7 +16,7 @@ import {
   TimeScale,
   TimeWindow,
   ArrowDirection,
-  TimeScaleCollection,
+  TimeScaleOptions,
 } from "./timeScaleTypes";
 import TimeFrameControls from "./timeFrameControls";
 import RangeSelect, { RangeOption } from "./rangeSelect";
@@ -31,7 +31,7 @@ export const timeFormat = "h:mmA";
 
 export interface TimeScaleDropdownProps {
   currentScale: TimeScale;
-  options?: TimeScaleCollection;
+  options?: TimeScaleOptions;
   setTimeScale: (tw: TimeScale) => void;
   adjustTimeScaleOnChange?: (
     curTimeScale: TimeScale,
@@ -124,8 +124,8 @@ export const TimeScaleDropdown: React.FC<TimeScaleDropdownProps> = ({
   setTimeScale,
   adjustTimeScaleOnChange,
 }): React.ReactElement => {
-  const end = currentScale.windowEnd
-    ? moment.utc(currentScale.windowEnd)
+  const end = currentScale.fixedWindowEnd
+    ? moment.utc(currentScale.fixedWindowEnd)
     : moment().utc();
   const currentWindow: TimeWindow = {
     start: moment.utc(end).subtract(currentScale.windowSize),
@@ -136,7 +136,7 @@ export const TimeScaleDropdown: React.FC<TimeScaleDropdownProps> = ({
     let timeScale: TimeScale = {
       ...options[rangeOption.label],
       key: rangeOption.label,
-      windowEnd: null,
+      fixedWindowEnd: false,
     };
     if (adjustTimeScaleOnChange) {
       const timeWindow: TimeWindow = {
@@ -156,37 +156,43 @@ export const TimeScaleDropdown: React.FC<TimeScaleDropdownProps> = ({
     const seconds = windowSize.asSeconds();
     let selected = {};
     let key = currentScale.key;
-    let windowEnd = moment.utc(currentWindow.end);
+    let endTime = moment.utc(currentWindow.end);
 
     switch (direction) {
       case ArrowDirection.RIGHT:
-        if (windowEnd) {
-          windowEnd = windowEnd.add(seconds, "seconds");
-        }
+        endTime = endTime.add(seconds, "seconds");
         break;
       case ArrowDirection.LEFT:
-        windowEnd = windowEnd.subtract(seconds, "seconds");
+        endTime = endTime.subtract(seconds, "seconds");
         break;
       case ArrowDirection.CENTER:
         // CENTER is used to set the time window to the current time.
-        windowEnd = moment.utc();
+        endTime = moment.utc();
         break;
       default:
         console.error("Unknown direction: ", direction);
     }
+
     // If the timescale extends into the future then fallback to a default
     // timescale. Otherwise set the key to "Custom" so it appears correctly.
-    if (
-      !windowEnd ||
-      windowEnd > moment.utc().subtract(currentScale.windowValid)
-    ) {
+    // The first `!endTime` part of the if clause seems unnecessary since endTime is always a specific time.
+    // If endTime + windowValid > now. Unclear why this uses windowValid instead of windowSize.
+    if (!endTime || endTime > moment.utc().subtract(currentScale.windowValid)) {
       const foundTimeScale = Object.entries(options).find(
         // eslint-disable-next-line @typescript-eslint/no-unused-vars
         ([_, value]) => value.windowSize.asSeconds() === windowSize.asSeconds(),
       );
       if (foundTimeScale) {
+        /**
+         * This code can be hit by:
+         *  - Select a default option, then click the left arrow, then click the right arrow.
+         * This (or the parent if block) is *not* hit by:
+         *  - Select a default time, click left, select a custom time of the same range, then click right. The arrow is
+         *    not disabled, but the clause doesn't seem to be true.
+         */
         selected = { key: foundTimeScale[0], ...foundTimeScale[1] };
       } else {
+        // This code might not be possible to hit, due to the right arrow being disabled
         key = "Custom";
       }
     } else {
@@ -195,7 +201,7 @@ export const TimeScaleDropdown: React.FC<TimeScaleDropdownProps> = ({
 
     let timeScale: TimeScale = {
       ...currentScale,
-      windowEnd,
+      fixedWindowEnd: endTime,
       windowSize,
       key,
       ...selected,
@@ -225,7 +231,7 @@ export const TimeScaleDropdown: React.FC<TimeScaleDropdownProps> = ({
     let timeScale: TimeScale = {
       ...findClosestTimeScale(options, seconds),
       windowSize: moment.duration(end.diff(start)),
-      windowEnd: end,
+      fixedWindowEnd: end,
       key: "Custom",
     };
     if (adjustTimeScaleOnChange) {

pkg/ui/workspaces/cluster-ui/src/timeScaleDropdown/timeScaleTypes.ts

@@ -21,43 +21,45 @@ export interface TimeWindow {
 }
 
 /**
- * TimeScale describes the requested dimensions of TimeWindows; it
- * prescribes a length for the window, along with a period of time that a
- * newly created TimeWindow will remain valid.
+ * TimeScale describes the requested dimensions, from which one can derive concrete TimeWindows using toDateRange.
  */
 export interface TimeScale {
-  // The key used to index in to the availableTimeScales collection.
+  /**
+   * The key used to index in to the defaultTimeScaleOptions collection.
+   * The key is "Custom" when specifying a custom time that is not one of the default options
+   */
   key?: string;
   // The size of a global time window. Default is ten minutes.
   windowSize: moment.Duration;
-  // The length of time the global time window is valid. The current time window
-  // is invalid if now > (currentWindow.end + windowValid). Default is ten
-  // seconds. If windowEnd is set this is ignored.
+  // The length of time the global time window is valid. Default is ten seconds.
   windowValid?: moment.Duration;
   // The expected duration of individual samples for queries at this time scale.
   sampleSize: moment.Duration;
-  // The end time of the window if it isn't the present
-  windowEnd?: moment.Moment;
+  /**
+   * The fixed end time of the window, or false if it should be a dynamically moving "now".
+   * Typically, when the `key` property is a default option, `fixedWindowEnd` should be false.
+   * And when the `key` property is "Custom" `fixedWindowEnd` should be a specific Moment.
+   * It is unclear if there are legitimate reasons for the two being out of sync.
+   */
+  fixedWindowEnd: moment.Moment | false;
 }
 
-export class TimeWindowState {
+export class TimeScaleState {
   // Currently selected scale.
   scale: TimeScale;
-  // Currently established time window.
-  currentWindow: TimeWindow;
-  // True if scale has changed since currentWindow was generated.
-  scaleChanged: boolean;
-  useTimeRange: boolean;
   constructor() {
-    this.scale = defaultTimeScaleOptions["Past 10 Minutes"];
-    this.scale.key = "Past 10 Minutes";
-    this.useTimeRange = false;
-    this.scaleChanged = false;
+    this.scale = {
+      ...defaultTimeScaleOptions["Past 10 Minutes"],
+      fixedWindowEnd: false,
+      key: "Past 10 Minutes",
+    };
   }
 }
 
-export interface TimeScaleCollection {
-  [key: string]: TimeScale;
+export type TimeScaleOption = Omit<TimeScale, "fixedWindowEnd">;
+
+export interface TimeScaleOptions {
+  [key: string]: TimeScaleOption;
 }
 
 export type TimeRangeTitle =

pkg/ui/workspaces/cluster-ui/src/timeScaleDropdown/timescale.spec.tsx

@@ -19,7 +19,7 @@ import {
   TimeScaleDropdown,
 } from "./timeScaleDropdown";
 import { defaultTimeScaleOptions, findClosestTimeScale } from "./utils";
-import * as timewindow from "./timeScaleTypes";
+import * as timescale from "./timeScaleTypes";
 import moment from "moment";
 import { MemoryRouter } from "react-router";
 import TimeFrameControls from "./timeFrameControls";
@@ -51,7 +51,7 @@ describe("<TimeScaleDropdown>", function() {
   let currentWindow: TimeWindow;
 
   const setCurrentWindowFromTimeScale = (timeScale: TimeScale): void => {
-    const end = timeScale.windowEnd || moment.utc();
+    const end = timeScale.fixedWindowEnd || moment.utc();
     currentWindow = {
       start: moment(end).subtract(timeScale.windowSize),
       end,
@@ -69,11 +69,10 @@ describe("<TimeScaleDropdown>", function() {
 
   beforeEach(() => {
     clock = sinon.useFakeTimers(new Date(2020, 5, 1, 9, 28, 30));
-    const timewindowState = new timewindow.TimeWindowState();
-    currentWindow = timewindowState.currentWindow;
-    setCurrentWindowFromTimeScale(timewindowState.scale);
+    const timeScaleState = new timescale.TimeScaleState();
+    setCurrentWindowFromTimeScale(timeScaleState.scale);
     state = {
-      currentScale: timewindowState.scale,
+      currentScale: timeScaleState.scale,
       setTimeScale: () => {},
     };
   });
@@ -91,10 +90,12 @@ describe("<TimeScaleDropdown>", function() {
   it("Past 10 minutes must be render", () => {
     const wrapper = makeTimeScaleDropdown(state);
     wrapper.setProps({ currentScale: state.currentScale });
-    assert.equal(
-      wrapper.props().currentScale,
-      defaultTimeScaleOptions["Past 10 Minutes"],
-    );
+    const expected: TimeScale = {
+      key: "Past 10 Minutes",
+      ...defaultTimeScaleOptions["Past 10 Minutes"],
+      fixedWindowEnd: false,
+    };
+    assert.deepEqual(wrapper.props().currentScale, expected);
   });
 
   it("getTimeRangeTitle must return title Past 10 Minutes", () => {
@@ -142,7 +143,7 @@ describe("<TimeScaleDropdown>", function() {
       };
       const currentScale = {
         ...state.currentScale,
-        windowEnd: window.end,
+        fixedWindowEnd: window.end,
         windowSize: moment.duration(
           window.end.diff(window.start, "seconds"),
           "seconds",
@@ -190,7 +191,7 @@ describe("<TimeScaleDropdown>", function() {
     };
     const currentTimeScale = {
       ...state.currentScale,
-      windowEnd: window.end,
+      fixedWindowEnd: window.end,
     };
     const arrows = generateDisabledArrows(window);
     const wrapper = makeTimeScaleDropdown({
@@ -204,11 +205,21 @@ describe("<TimeScaleDropdown>", function() {
 
 describe("timescale utils", (): void => {
   describe("findClosestTimeScale", () => {
-    it("should found correctly time scale", () => {
+    it("should find the correct time scale", () => {
+      // `seconds` != window size of any of the default options, `startSeconds` not specified.
       assert.deepEqual(findClosestTimeScale(defaultTimeScaleOptions, 15), {
         ...defaultTimeScaleOptions["Past 10 Minutes"],
         key: "Custom",
       });
+      // `seconds` != window size of any of the default options, `startSeconds` not specified.
+      assert.deepEqual(
+        findClosestTimeScale(
+          defaultTimeScaleOptions,
+          moment.duration(moment().daysInMonth() * 5, "days").asSeconds(),
+        ),
+        { ...defaultTimeScaleOptions["Past 2 Months"], key: "Custom" },
+      );
+      // `seconds` == window size of one of the default options, `startSeconds` not specified.
       assert.deepEqual(
         findClosestTimeScale(
           defaultTimeScaleOptions,
@@ -219,6 +230,7 @@ describe("timescale utils", (): void => {
           key: "Past 10 Minutes",
         },
       );
+      // `seconds` == window size of one of the default options, `startSeconds` not specified.
       assert.deepEqual(
         findClosestTimeScale(
           defaultTimeScaleOptions,
@@ -229,12 +241,28 @@ describe("timescale utils", (): void => {
           key: "Past 2 Weeks",
         },
       );
+      // `seconds` == window size of one of the default options, `startSeconds` is now.
       assert.deepEqual(
         findClosestTimeScale(
           defaultTimeScaleOptions,
-          moment.duration(moment().daysInMonth() * 5, "days").asSeconds(),
+          defaultTimeScaleOptions["Past 10 Minutes"].windowSize.asSeconds(),
+          moment().unix(),
         ),
-        { ...defaultTimeScaleOptions["Past 2 Months"], key: "Custom" },
+        {
+          ...defaultTimeScaleOptions["Past 10 Minutes"],
+          key: "Past 10 Minutes",
+        },
+      );
+      // `seconds` == window size of one of the default options, `startSeconds` is in the past.
+      assert.deepEqual(
+        findClosestTimeScale(
+          defaultTimeScaleOptions,
+          defaultTimeScaleOptions["Past 10 Minutes"].windowSize.asSeconds(),
+          moment()
+            .subtract(1, "day")
+            .unix(),
+        ),
+        { ...defaultTimeScaleOptions["Past 10 Minutes"], key: "Custom" },
       );
     });
   });

pkg/ui/workspaces/cluster-ui/src/timeScaleDropdown/utils.ts

@@ -9,13 +9,13 @@
 // licenses/APL.txt.
 
 import moment from "moment";
-import { TimeScale, TimeScaleCollection } from "./timeScaleTypes";
+import { TimeScale, TimeScaleOption, TimeScaleOptions } from "./timeScaleTypes";
 
 /**
  * defaultTimeScaleOptions is a preconfigured set of time scales that can be
  * selected by the user.
  */
-export const defaultTimeScaleOptions: TimeScaleCollection = {
+export const defaultTimeScaleOptions: TimeScaleOptions = {
   "Past 10 Minutes": {
     windowSize: moment.duration(10, "minutes"),
     windowValid: moment.duration(10, "seconds"),
@@ -76,21 +76,24 @@ export const defaultTimeScaleOptions: TimeScaleCollection = {
 export const defaultTimeScaleSelected: TimeScale = {
   ...defaultTimeScaleOptions["Past 1 Hour"],
   key: "Past 1 Hour",
+  fixedWindowEnd: false,
 };
 
 export const toDateRange = (ts: TimeScale): [moment.Moment, moment.Moment] => {
-  const end = ts.windowEnd ? moment.utc(ts.windowEnd) : moment().utc();
+  const end = ts.fixedWindowEnd
+    ? moment.utc(ts.fixedWindowEnd)
+    : moment().utc();
   const start = moment.utc(end).subtract(ts.windowSize);
   return [start, end];
 };
 
 export const findClosestTimeScale = (
-  options: TimeScaleCollection,
+  options: TimeScaleOptions,
   seconds: number,
   startSeconds?: number,
-): TimeScale => {
+): TimeScaleOption => {
   const data = Object.keys(options).map(
-    (val): TimeScale => ({ ...options[val], key: val }),
+    (val): TimeScaleOption => ({ ...options[val], key: val }),
   );
 
   data.sort(
@@ -99,22 +102,22 @@ export const findClosestTimeScale = (
       Math.abs(seconds - b.windowSize.asSeconds()),
   );
 
-  const firstTimeScaleOptionSeconds = data[0].windowSize.asSeconds();
+  const closestWindowSizeSeconds = data[0].windowSize.asSeconds();
 
   // This logic covers the edge case where drag-to-timerange on a linegraph is of a duration
   // that exactly matches one of the standard available time scales e.g. selecting June 1 at
   // 0:00 to June 2 at 0:00 when the date is July 1 at 0:00 should return a custom timescale
   // instead of past day.
-  if (startSeconds && firstTimeScaleOptionSeconds === seconds) {
-    const startWindow = moment()
-      .subtract(firstTimeScaleOptionSeconds, "seconds")
-      .unix();
-    if (startSeconds < startWindow) {
+  // If the start is specified, and the window size matches.
+  if (startSeconds && closestWindowSizeSeconds === seconds) {
+    // Check if the end is before now. If so, it is a custom time.
+    const end = moment.unix(startSeconds + seconds);
+    if (end < moment()) {
       return { ...data[0], key: "Custom" };
     }
   }
 
-  return firstTimeScaleOptionSeconds === seconds
+  return closestWindowSizeSeconds === seconds
     ? data[0]
     : { ...data[0], key: "Custom" };
 };