From ffc868a0b0bc0451bb5b3f9fc7b3a6c87d10fd6b Mon Sep 17 00:00:00 2001 From: Kayla Washburn-Love Date: Mon, 17 Jun 2024 10:24:30 -0600 Subject: [PATCH 1/2] fix: fix workspace actions options (#13572) (cherry picked from commit 07cd9acb2c1e745e5024b860a50376ddacb47b5b) --- .../WorkspaceActions/Buttons.tsx | 24 +++-- .../WorkspaceActions.stories.tsx | 102 ++++++++++++------ .../WorkspaceActions/WorkspaceActions.tsx | 43 +++----- .../WorkspaceActions/constants.ts | 66 ++++++------ site/src/testHelpers/entities.ts | 4 - 5 files changed, 138 insertions(+), 101 deletions(-) diff --git a/site/src/pages/WorkspacePage/WorkspaceActions/Buttons.tsx b/site/src/pages/WorkspacePage/WorkspaceActions/Buttons.tsx index caa034d77c29f..84b99c23ff74e 100644 --- a/site/src/pages/WorkspacePage/WorkspaceActions/Buttons.tsx +++ b/site/src/pages/WorkspacePage/WorkspaceActions/Buttons.tsx @@ -97,6 +97,21 @@ export const StartButton: FC = ({ ); }; +export const UpdateAndStartButton: FC = ({ + handleAction, +}) => { + return ( + + } + onClick={() => handleAction()} + > + Update and start… + + + ); +}; + export const StopButton: FC = ({ handleAction, loading, @@ -146,16 +161,13 @@ export const RestartButton: FC = ({ ); }; -export const UpdateAndStartButton: FC = ({ +export const UpdateAndRestartButton: FC = ({ handleAction, }) => { return ( - } - onClick={() => handleAction()} - > - Update and start… + } onClick={() => handleAction()}> + Update and restart… ); diff --git a/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.stories.tsx b/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.stories.tsx index 0bdf6c1e36fab..ca0ba79980010 100644 --- a/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.stories.tsx +++ b/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.stories.tsx @@ -26,93 +26,128 @@ export const Running: Story = { }, }; -export const Stopping: Story = { +export const RunningUpdateAvailable: Story = { + name: "Running (Update available)", args: { - workspace: Mocks.MockStoppingWorkspace, + workspace: { + ...Mocks.MockWorkspace, + outdated: true, + }, }, }; -export const Stopped: Story = { +export const RunningRequireActiveVersion: Story = { + name: "Running (No required update)", args: { - workspace: Mocks.MockStoppedWorkspace, + workspace: { + ...Mocks.MockWorkspace, + template_require_active_version: true, + }, }, }; -export const Canceling: Story = { +export const RunningUpdateRequired: Story = { + name: "Running (Update Required)", args: { - workspace: Mocks.MockCancelingWorkspace, + workspace: { + ...Mocks.MockWorkspace, + template_require_active_version: true, + outdated: true, + }, }, }; -export const Canceled: Story = { +export const Stopping: Story = { args: { - workspace: Mocks.MockCanceledWorkspace, + workspace: Mocks.MockStoppingWorkspace, }, }; -export const Deleting: Story = { +export const Stopped: Story = { args: { - workspace: Mocks.MockDeletingWorkspace, + workspace: Mocks.MockStoppedWorkspace, }, }; -export const Deleted: Story = { +export const StoppedUpdateAvailable: Story = { + name: "Stopped (Update available)", args: { - workspace: Mocks.MockDeletedWorkspace, + workspace: { + ...Mocks.MockStoppedWorkspace, + outdated: true, + }, }, }; -export const Outdated: Story = { +export const StoppedRequireActiveVersion: Story = { + name: "Stopped (No required update)", + args: { + workspace: { + ...Mocks.MockStoppedWorkspace, + template_require_active_version: true, + }, + }, +}; + +export const StoppedUpdateRequired: Story = { + name: "Stopped (Update Required)", + args: { + workspace: { + ...Mocks.MockStoppedWorkspace, + template_require_active_version: true, + outdated: true, + }, + }, +}; + +export const Updating: Story = { args: { workspace: Mocks.MockOutdatedWorkspace, + isUpdating: true, }, }; -export const Failed: Story = { +export const Restarting: Story = { args: { - workspace: Mocks.MockFailedWorkspace, + workspace: Mocks.MockStoppingWorkspace, + isRestarting: true, }, }; -export const FailedWithDebug: Story = { +export const Canceling: Story = { args: { - workspace: Mocks.MockFailedWorkspace, - canDebug: true, + workspace: Mocks.MockCancelingWorkspace, }, }; -export const Updating: Story = { +export const Deleting: Story = { args: { - isUpdating: true, - workspace: Mocks.MockOutdatedWorkspace, + workspace: Mocks.MockDeletingWorkspace, }, }; -export const RequireActiveVersionStarted: Story = { +export const Deleted: Story = { args: { - workspace: Mocks.MockOutdatedRunningWorkspaceRequireActiveVersion, - canChangeVersions: false, + workspace: Mocks.MockDeletedWorkspace, }, }; -export const RequireActiveVersionStopped: Story = { +export const Outdated: Story = { args: { - workspace: Mocks.MockOutdatedStoppedWorkspaceRequireActiveVersion, - canChangeVersions: false, + workspace: Mocks.MockOutdatedWorkspace, }, }; -export const AlwaysUpdateStarted: Story = { +export const Failed: Story = { args: { - workspace: Mocks.MockOutdatedRunningWorkspaceAlwaysUpdate, - canChangeVersions: true, + workspace: Mocks.MockFailedWorkspace, }, }; -export const AlwaysUpdateStopped: Story = { +export const FailedWithDebug: Story = { args: { - workspace: Mocks.MockOutdatedStoppedWorkspaceAlwaysUpdate, - canChangeVersions: true, + workspace: Mocks.MockFailedWorkspace, + canDebug: true, }, }; @@ -125,6 +160,7 @@ export const CancelShownForOwner: Story = { isOwner: true, }, }; + export const CancelShownForUser: Story = { args: { workspace: Mocks.MockStartingWorkspace, diff --git a/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.tsx b/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.tsx index beab34de37633..a321f3cd8f4f0 100644 --- a/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.tsx +++ b/site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.tsx @@ -25,6 +25,7 @@ import { ActivateButton, FavoriteButton, UpdateAndStartButton, + UpdateAndRestartButton, } from "./Buttons"; import { type ActionType, abilitiesByWorkspaceStatus } from "./constants"; import { DebugButton } from "./DebugButton"; @@ -85,12 +86,12 @@ export const WorkspaceActions: FC = ({ const mustUpdate = mustUpdateWorkspace(workspace, canChangeVersions); const tooltipText = getTooltipText(workspace, mustUpdate, canChangeVersions); - const canBeUpdated = workspace.outdated && canAcceptJobs; // A mapping of button type to the corresponding React component const buttonMapping: Record = { update: , updateAndStart: , + updateAndRestart: , updating: , start: ( = ({ enableBuildParameters={workspace.latest_build.transition === "start"} /> ), - toggleFavorite: ( - - ), }; return ( @@ -162,29 +156,22 @@ export const WorkspaceActions: FC = ({ css={{ display: "flex", alignItems: "center", gap: 8 }} data-testid="workspace-actions" > - {canBeUpdated && ( - <> - {isUpdating - ? buttonMapping.updating - : workspace.template_require_active_version - ? buttonMapping.updateAndStart - : buttonMapping.update} - - )} - - {!canBeUpdated && - workspace.template_require_active_version && - buttonMapping.start} - - {isRestarting - ? buttonMapping.restarting - : actions.map((action) => ( - {buttonMapping[action]} - ))} + {/* Restarting must be handled separately, because it otherwise would appear as stopping */} + {isUpdating + ? buttonMapping.updating + : isRestarting + ? buttonMapping.restarting + : actions.map((action) => ( + {buttonMapping[action]} + ))} {showCancel && } - {buttonMapping.toggleFavorite} + diff --git a/site/src/pages/WorkspacePage/WorkspaceActions/constants.ts b/site/src/pages/WorkspacePage/WorkspaceActions/constants.ts index c8f564d4fe99a..329a958ee12a8 100644 --- a/site/src/pages/WorkspacePage/WorkspaceActions/constants.ts +++ b/site/src/pages/WorkspacePage/WorkspaceActions/constants.ts @@ -6,16 +6,19 @@ import type { Workspace } from "api/typesGenerated"; export const actionTypes = [ "start", "starting", + // Replaces start when an update is required. + "updateAndStart", "stop", "stopping", "restart", "restarting", + // Replaces restart when an update is required. + "updateAndRestart", "deleting", "update", "updating", "activate", "activating", - "toggleFavorite", // There's no need for a retrying state because retrying starts a transition // into one of the starting, stopping, or deleting states (based on the @@ -23,10 +26,6 @@ export const actionTypes = [ "retry", "debug", - // When a template requires updates, we aim to display a distinct update - // button that clearly indicates a mandatory update. - "updateAndStart", - // These are buttons that should be used with disabled UI elements "canceling", "deleted", @@ -54,13 +53,6 @@ export const abilitiesByWorkspaceStatus = ( } const status = workspace.latest_build.status; - if (status === "failed" && canDebug) { - return { - actions: ["retry", "debug"], - canCancel: false, - canAcceptJobs: true, - }; - } switch (status) { case "starting": { @@ -73,10 +65,12 @@ export const abilitiesByWorkspaceStatus = ( case "running": { const actions: ActionType[] = ["stop"]; - // If the template requires the latest version, we prevent the user from - // restarting the workspace without updating it first. In the Buttons - // component, we display an UpdateAndStart component to facilitate this. - if (!workspace.template_require_active_version) { + if (workspace.template_require_active_version && workspace.outdated) { + actions.push("updateAndRestart"); + } else { + if (workspace.outdated) { + actions.unshift("update"); + } actions.push("restart"); } @@ -96,10 +90,12 @@ export const abilitiesByWorkspaceStatus = ( case "stopped": { const actions: ActionType[] = []; - // If the template requires the latest version, we prevent the user from - // starting the workspace without updating it first. In the Buttons - // component, we display an UpdateAndStart component to facilitate this. - if (!workspace.template_require_active_version) { + if (workspace.template_require_active_version && workspace.outdated) { + actions.push("updateAndStart"); + } else { + if (workspace.outdated) { + actions.unshift("update"); + } actions.push("start"); } @@ -117,14 +113,31 @@ export const abilitiesByWorkspaceStatus = ( }; } case "failed": { + const actions: ActionType[] = ["retry"]; + + if (canDebug) { + actions.push("debug"); + } + + if (workspace.outdated) { + actions.unshift("update"); + } + return { - actions: ["retry"], + actions, canCancel: false, canAcceptJobs: true, }; } // Disabled states + case "pending": { + return { + actions: ["pending"], + canCancel: false, + canAcceptJobs: false, + }; + } case "canceling": { return { actions: ["canceling"], @@ -146,15 +159,8 @@ export const abilitiesByWorkspaceStatus = ( canAcceptJobs: false, }; } - case "pending": { - return { - actions: ["pending"], - canCancel: false, - canAcceptJobs: false, - }; - } - default: { + + default: throw new Error(`Unknown workspace status: ${status}`); - } } }; diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index e083c4c605b05..50db4dba39cc4 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -1173,10 +1173,6 @@ export const MockOutdatedRunningWorkspaceRequireActiveVersion: TypesGen.Workspac id: "test-outdated-workspace-require-active-version", outdated: true, template_require_active_version: true, - latest_build: { - ...MockWorkspaceBuild, - status: "running", - }, }; export const MockOutdatedRunningWorkspaceAlwaysUpdate: TypesGen.Workspace = { From 2c2b8c470a482955791557b1b599abe764d7611f Mon Sep 17 00:00:00 2001 From: Charlie Voiselle <464492+angrycub@users.noreply.github.com> Date: Fri, 26 Jul 2024 17:57:47 -0400 Subject: [PATCH 2/2] fix: change time format string from 15:40 to 15:04 (#14033) * Change string format to constant value (cherry picked from commit eacdfb9f9ce20528b2a6d0bb0a35f26a036354c5) --- enterprise/coderd/users.go | 6 ++++-- enterprise/coderd/users_test.go | 17 +++++++++++------ 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/enterprise/coderd/users.go b/enterprise/coderd/users.go index 935eeb8f6e689..07e66708b1713 100644 --- a/enterprise/coderd/users.go +++ b/enterprise/coderd/users.go @@ -14,6 +14,8 @@ import ( "github.com/coder/coder/v2/codersdk" ) +const TimeFormatHHMM = "15:04" + func (api *API) autostopRequirementEnabledMW(next http.Handler) http.Handler { return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { // Entitlement must be enabled. @@ -66,7 +68,7 @@ func (api *API) userQuietHoursSchedule(rw http.ResponseWriter, r *http.Request) RawSchedule: opts.Schedule.String(), UserSet: opts.UserSet, UserCanSet: opts.UserCanSet, - Time: opts.Schedule.TimeParsed().Format("15:40"), + Time: opts.Schedule.TimeParsed().Format(TimeFormatHHMM), Timezone: opts.Schedule.Location().String(), Next: opts.Schedule.Next(time.Now().In(opts.Schedule.Location())), }) @@ -118,7 +120,7 @@ func (api *API) putUserQuietHoursSchedule(rw http.ResponseWriter, r *http.Reques RawSchedule: opts.Schedule.String(), UserSet: opts.UserSet, UserCanSet: opts.UserCanSet, - Time: opts.Schedule.TimeParsed().Format("15:40"), + Time: opts.Schedule.TimeParsed().Format(TimeFormatHHMM), Timezone: opts.Schedule.Location().String(), Next: opts.Schedule.Next(time.Now().In(opts.Schedule.Location())), }) diff --git a/enterprise/coderd/users_test.go b/enterprise/coderd/users_test.go index ede99551ef0ec..21f54d2e8ca87 100644 --- a/enterprise/coderd/users_test.go +++ b/enterprise/coderd/users_test.go @@ -11,11 +11,14 @@ import ( "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/schedule/cron" "github.com/coder/coder/v2/codersdk" + "github.com/coder/coder/v2/enterprise/coderd" "github.com/coder/coder/v2/enterprise/coderd/coderdenttest" "github.com/coder/coder/v2/enterprise/coderd/license" "github.com/coder/coder/v2/testutil" ) +const TimeFormatHHMM = coderd.TimeFormatHHMM + func TestUserQuietHours(t *testing.T) { t.Parallel() @@ -41,15 +44,17 @@ func TestUserQuietHours(t *testing.T) { t.Run("OK", func(t *testing.T) { t.Parallel() - - defaultQuietHoursSchedule := "CRON_TZ=America/Chicago 0 1 * * *" + // Using 10 for minutes lets us test a format bug in which values greater + // than 5 were causing the API to explode because the time was returned + // incorrectly + defaultQuietHoursSchedule := "CRON_TZ=America/Chicago 10 1 * * *" defaultScheduleParsed, err := cron.Daily(defaultQuietHoursSchedule) require.NoError(t, err) nextTime := defaultScheduleParsed.Next(time.Now().In(defaultScheduleParsed.Location())) if time.Until(nextTime) < time.Hour { // Use a different default schedule instead, because we want to avoid // the schedule "ticking over" during this test run. - defaultQuietHoursSchedule = "CRON_TZ=America/Chicago 0 13 * * *" + defaultQuietHoursSchedule = "CRON_TZ=America/Chicago 10 13 * * *" defaultScheduleParsed, err = cron.Daily(defaultQuietHoursSchedule) require.NoError(t, err) } @@ -78,7 +83,7 @@ func TestUserQuietHours(t *testing.T) { require.NoError(t, err) require.Equal(t, defaultScheduleParsed.String(), sched1.RawSchedule) require.False(t, sched1.UserSet) - require.Equal(t, defaultScheduleParsed.TimeParsed().Format("15:40"), sched1.Time) + require.Equal(t, defaultScheduleParsed.TimeParsed().Format(TimeFormatHHMM), sched1.Time) require.Equal(t, defaultScheduleParsed.Location().String(), sched1.Timezone) require.WithinDuration(t, defaultScheduleParsed.Next(time.Now()), sched1.Next, 15*time.Second) @@ -101,7 +106,7 @@ func TestUserQuietHours(t *testing.T) { require.NoError(t, err) require.Equal(t, customScheduleParsed.String(), sched2.RawSchedule) require.True(t, sched2.UserSet) - require.Equal(t, customScheduleParsed.TimeParsed().Format("15:40"), sched2.Time) + require.Equal(t, customScheduleParsed.TimeParsed().Format(TimeFormatHHMM), sched2.Time) require.Equal(t, customScheduleParsed.Location().String(), sched2.Timezone) require.WithinDuration(t, customScheduleParsed.Next(time.Now()), sched2.Next, 15*time.Second) @@ -110,7 +115,7 @@ func TestUserQuietHours(t *testing.T) { require.NoError(t, err) require.Equal(t, customScheduleParsed.String(), sched3.RawSchedule) require.True(t, sched3.UserSet) - require.Equal(t, customScheduleParsed.TimeParsed().Format("15:40"), sched3.Time) + require.Equal(t, customScheduleParsed.TimeParsed().Format(TimeFormatHHMM), sched3.Time) require.Equal(t, customScheduleParsed.Location().String(), sched3.Timezone) require.WithinDuration(t, customScheduleParsed.Next(time.Now()), sched3.Next, 15*time.Second)