Skip to content

feat: add 'impending deletion' badges to workspaces page #7530

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

Merged
merged 8 commits into from
May 15, 2023
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
16 changes: 7 additions & 9 deletions coderd/workspaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/go-chi/chi/v5"
"github.com/google/uuid"
"github.com/tabbed/pqtype"
"golang.org/x/exp/slices"
"golang.org/x/xerrors"

"cdr.dev/slog"
Expand Down Expand Up @@ -1171,7 +1172,7 @@ func convertWorkspace(

var (
ttlMillis = convertWorkspaceTTLMillis(workspace.Ttl)
deletingAt = calculateDeletingAt(workspace, template)
deletingAt = calculateDeletingAt(workspace, template, workspaceBuild)
)
return codersdk.Workspace{
ID: workspace.ID,
Expand Down Expand Up @@ -1206,14 +1207,11 @@ func convertWorkspaceTTLMillis(i sql.NullInt64) *int64 {

// Calculate the time of the upcoming workspace deletion, if applicable; otherwise, return nil.
// Workspaces may have impending deletions if InactivityTTL feature is turned on and the workspace is inactive.
func calculateDeletingAt(workspace database.Workspace, template database.Template) *time.Time {
var (
year, month, day = time.Now().Date()
beginningOfToday = time.Date(year, month, day, 0, 0, 0, 0, time.Now().Location())
)
// If InactivityTTL is turned off (set to 0), if the workspace has already been deleted,
// or if the workspace was used sometime within the last day, there is no impending deletion
if template.InactivityTTL == 0 || workspace.Deleted || workspace.LastUsedAt.After(beginningOfToday) {
func calculateDeletingAt(workspace database.Workspace, template database.Template, build codersdk.WorkspaceBuild) *time.Time {
inactiveStatuses := []codersdk.WorkspaceStatus{codersdk.WorkspaceStatusStopped, codersdk.WorkspaceStatusCanceled, codersdk.WorkspaceStatusFailed, codersdk.WorkspaceStatusDeleted}
isInactive := slices.Contains(inactiveStatuses, build.Status)
// If InactivityTTL is turned off (set to 0) or if the workspace is active, there is no impending deletion
if template.InactivityTTL == 0 || !isInactive {
return nil
}

Expand Down
30 changes: 15 additions & 15 deletions coderd/workspaces_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/coder/coder/coderd/database"
"github.com/coder/coder/coderd/util/ptr"
"github.com/coder/coder/codersdk"
)

func Test_calculateDeletingAt(t *testing.T) {
Expand All @@ -17,17 +18,21 @@ func Test_calculateDeletingAt(t *testing.T) {
name string
workspace database.Workspace
template database.Template
build codersdk.WorkspaceBuild
expected *time.Time
}{
{
name: "DeletingAt",
name: "InactiveWorkspace",
workspace: database.Workspace{
Deleted: false,
LastUsedAt: time.Now().Add(time.Duration(-10) * time.Hour * 24), // 10 days ago
},
template: database.Template{
InactivityTTL: int64(9 * 24 * time.Hour), // 9 days
},
build: codersdk.WorkspaceBuild{
Status: codersdk.WorkspaceStatusStopped,
},
expected: ptr.Ref(time.Now().Add(time.Duration(-1) * time.Hour * 24)), // yesterday
},
{
Expand All @@ -39,27 +44,22 @@ func Test_calculateDeletingAt(t *testing.T) {
template: database.Template{
InactivityTTL: 0,
},
expected: nil,
},
{
name: "DeletedWorkspace",
workspace: database.Workspace{
Deleted: true,
LastUsedAt: time.Now().Add(time.Duration(-10) * time.Hour * 24),
},
template: database.Template{
InactivityTTL: int64(9 * 24 * time.Hour),
build: codersdk.WorkspaceBuild{
Status: codersdk.WorkspaceStatusStopped,
},
expected: nil,
},
{
name: "ActiveWorkspace",
workspace: database.Workspace{
Deleted: true,
LastUsedAt: time.Now().Add(time.Duration(-5) * time.Hour), // 5 hours ago
Deleted: false,
LastUsedAt: time.Now(),
},
template: database.Template{
InactivityTTL: int64(1 * 24 * time.Hour), // 1 day
InactivityTTL: int64(1 * 24 * time.Hour),
},
build: codersdk.WorkspaceBuild{
Status: codersdk.WorkspaceStatusRunning,
},
expected: nil,
},
Expand All @@ -70,7 +70,7 @@ func Test_calculateDeletingAt(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()

found := calculateDeletingAt(tc.workspace, tc.template)
found := calculateDeletingAt(tc.workspace, tc.template, tc.build)
if tc.expected == nil {
require.Nil(t, found, "impending deletion should be nil")
} else {
Expand Down
2 changes: 1 addition & 1 deletion site/src/components/WorkspaceStats/WorkspaceStats.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ export const WorkspaceStats: FC<WorkspaceStatsProps> = ({
<StatsItem
className={styles.statsItem}
label="Status"
value={<WorkspaceStatusText build={workspace.latest_build} />}
value={<WorkspaceStatusText workspace={workspace} />}
/>
<StatsItem
className={styles.statsItem}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,50 +27,50 @@ const Template: Story<WorkspaceStatusBadgeProps> = (args) => (

export const Running = Template.bind({})
Running.args = {
build: MockWorkspace.latest_build,
workspace: MockWorkspace,
}

export const Starting = Template.bind({})
Starting.args = {
build: MockStartingWorkspace.latest_build,
workspace: MockStartingWorkspace,
}

export const Stopped = Template.bind({})
Stopped.args = {
build: MockStoppedWorkspace.latest_build,
workspace: MockStoppedWorkspace,
}

export const Stopping = Template.bind({})
Stopping.args = {
build: MockStoppingWorkspace.latest_build,
workspace: MockStoppingWorkspace,
}

export const Deleting = Template.bind({})
Deleting.args = {
build: MockDeletingWorkspace.latest_build,
workspace: MockDeletingWorkspace,
}

export const Deleted = Template.bind({})
Deleted.args = {
build: MockDeletedWorkspace.latest_build,
workspace: MockDeletedWorkspace,
}

export const Canceling = Template.bind({})
Canceling.args = {
build: MockCancelingWorkspace.latest_build,
workspace: MockCancelingWorkspace,
}

export const Canceled = Template.bind({})
Canceled.args = {
build: MockCanceledWorkspace.latest_build,
workspace: MockCanceledWorkspace,
}

export const Failed = Template.bind({})
Failed.args = {
build: MockFailedWorkspace.latest_build,
workspace: MockFailedWorkspace,
}

export const Pending = Template.bind({})
Pending.args = {
build: MockPendingWorkspace.latest_build,
workspace: MockPendingWorkspace,
}
44 changes: 38 additions & 6 deletions site/src/components/WorkspaceStatusBadge/WorkspaceStatusBadge.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@ import ErrorIcon from "@mui/icons-material/ErrorOutline"
import StopIcon from "@mui/icons-material/StopOutlined"
import PlayIcon from "@mui/icons-material/PlayArrowOutlined"
import QueuedIcon from "@mui/icons-material/HourglassEmpty"
import { WorkspaceBuild } from "api/typesGenerated"
import { Workspace, WorkspaceBuild } from "api/typesGenerated"
import { Pill } from "components/Pill/Pill"
import i18next from "i18next"
import { FC, PropsWithChildren } from "react"
import { makeStyles } from "@mui/styles"
import { combineClasses } from "utils/combineClasses"
import { displayImpendingDeletion } from "utils/workspace"
import { useDashboard } from "components/Dashboard/DashboardProvider"

const LoadingIcon: FC = () => {
return <CircularProgress size={10} style={{ color: "#FFF" }} />
Expand Down Expand Up @@ -87,22 +89,52 @@ export const getStatus = (buildStatus: WorkspaceBuild["status"]) => {
}

export type WorkspaceStatusBadgeProps = {
build: WorkspaceBuild
workspace: Workspace
className?: string
}

const ImpendingDeletionBadge: FC<Partial<WorkspaceStatusBadgeProps>> = ({
className,
}) => {
const { entitlements, experiments } = useDashboard()
const allowAdvancedScheduling =
entitlements.features["advanced_template_scheduling"].enabled
// This check can be removed when https://github.com/coder/coder/milestone/19
// is merged up
const allowWorkspaceActions = experiments.includes("workspace_actions")

if (!allowAdvancedScheduling || !allowWorkspaceActions) {
return null
}
Copy link
Collaborator

@BrunoQuaresma BrunoQuaresma May 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are already checking if we should or not display the badge on displayImpendingDeletion I think we should move those checks to be there since they are doing the same thing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BrunoQuaresma those checks rely on a hook, which has to be called in a component. Would you rather I call the hook in the WorkspaceStatusBadge component? I definitely could; I just didn't want to screw up all the storybook stories.

Alternatively, I thought about calling this hook in WorkspacesPage and then passing down props from there - but there are like 7 layers of prop passing and it just seemed like a lot to me.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thing for me is to have the "should display or not" check in one single place. Splitting this check into different places looks confusing to me. But if it is hard, I would add a comment so that in the future, if we want to change the display behavior, we know there are two places to look for.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed - as the ideal behavior is to have the check in one place, I think the best path forward may be to call the hook in the badge component and then ignore Storybook's complaints, rather than let our testing tool dictate how we code.
I'll adjust.

return (
<Pill
className={className}
icon={<ErrorIcon />}
text="Impending deletion"
type="error"
/>
)
}

export const WorkspaceStatusBadge: FC<
PropsWithChildren<WorkspaceStatusBadgeProps>
> = ({ build, className }) => {
const { text, icon, type } = getStatus(build.status)
> = ({ workspace, className }) => {
// The ImpendingDeletionBadge component itself checks to see if the
// Advanced Scheduling feature is turned on and if the
// Workspace Actions flag is turned on.
if (displayImpendingDeletion(workspace)) {
return <ImpendingDeletionBadge className={className} />
}

const { text, icon, type } = getStatus(workspace.latest_build.status)
return <Pill className={className} icon={icon} text={text} type={type} />
}

export const WorkspaceStatusText: FC<
PropsWithChildren<WorkspaceStatusBadgeProps>
> = ({ build, className }) => {
> = ({ workspace, className }) => {
const styles = useStyles()
const { text, type } = getStatus(build.status)
const { text, type } = getStatus(workspace.latest_build.status)
return (
<span
role="status"
Expand Down
2 changes: 1 addition & 1 deletion site/src/components/WorkspacesTable/WorkspacesRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export const WorkspacesRow: FC<{
</TableCell>

<TableCell>
<WorkspaceStatusBadge build={workspace.latest_build} />
<WorkspaceStatusBadge workspace={workspace} />
</TableCell>

<TableCell>
Expand Down
4 changes: 4 additions & 0 deletions site/src/pages/WorkspacesPage/WorkspacesPageView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ export const WorkspacesPageView: FC<
query: workspaceFilterQuery.running,
name: Language.runningWorkspacesButton,
},
{
query: workspaceFilterQuery.failed,
name: "Failed workspaces",
},
]

return (
Expand Down
1 change: 0 additions & 1 deletion site/src/testHelpers/entities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -721,7 +721,6 @@ export const MockWorkspace: TypesGen.Workspace = {
ttl_ms: 2 * 60 * 60 * 1000,
latest_build: MockWorkspaceBuild,
last_used_at: "2022-05-16T15:29:10.302441433Z",
deleting_at: "0001-01-01T00:00:00Z",
}

export const MockStoppedWorkspace: TypesGen.Workspace = {
Expand Down
1 change: 1 addition & 0 deletions site/src/utils/filters.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ describe("queryToFilter", () => {
["me/dev", { q: "me/dev" }],
["me/", { q: "me/" }],
[" key:val owner:me ", { q: "key:val owner:me" }],
["status:failed", { q: "status:failed" }],
])(`query=%p, filter=%p`, (query, filter) => {
expect(queryToFilter(query)).toEqual(filter)
})
Expand Down
1 change: 1 addition & 0 deletions site/src/utils/filters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export const workspaceFilterQuery = {
me: "owner:me",
all: "",
running: "status:running",
failed: "status:failed",
}

export const userFilterQuery = {
Expand Down
18 changes: 18 additions & 0 deletions site/src/utils/workspace.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
getDisplayVersionStatus,
getDisplayWorkspaceBuildInitiatedBy,
getDisplayWorkspaceTemplateName,
displayImpendingDeletion,
isWorkspaceOn,
} from "./workspace"

Expand Down Expand Up @@ -139,4 +140,21 @@ describe("util > workspace", () => {
expect(displayed).toEqual(workspace.template_display_name)
})
})

describe("displayImpendingDeletion", () => {
const today = new Date()
it.each<[string, boolean]>([
[new Date(new Date().setDate(today.getDate() + 15)).toISOString(), false], // today + 15 days out
[new Date(new Date().setDate(today.getDate() + 14)).toISOString(), true], // today + 14
[new Date(new Date().setDate(today.getDate() + 13)).toISOString(), true], // today + 13
[new Date(new Date().setDate(today.getDate() + 1)).toISOString(), true], // today + 1
[new Date().toISOString(), true], // today + 0
])(`deleting_at=%p, isWorkspaceOn=%p`, (deleting_at, shouldDisplay) => {
const workspace: TypesGen.Workspace = {
...Mocks.MockWorkspace,
deleting_at,
}
expect(displayImpendingDeletion(workspace)).toBe(shouldDisplay)
})
})
})
24 changes: 24 additions & 0 deletions site/src/utils/workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,3 +185,27 @@ export const getDisplayWorkspaceTemplateName = (
? workspace.template_display_name
: workspace.template_name
}

// This const dictates how far out we alert the user that a workspace
// has an impending deletion (due to template.InactivityTTL being set)
const IMPENDING_DELETION_DISPLAY_THRESHOLD = 14 // 14 days

/**
* Returns a boolean indicating if an impending deletion indicator should be
* displayed in the UI. Impending deletions are configured by setting the
* Template.InactivityTTL
* @param {TypesGen.Workspace} workspace
* @returns {boolean}
*/
export const displayImpendingDeletion = (workspace: TypesGen.Workspace) => {
const today = new Date()
if (!workspace.deleting_at) {
return false
}
return (
new Date(workspace.deleting_at) <=
new Date(
today.setDate(today.getDate() + IMPENDING_DELETION_DISPLAY_THRESHOLD),
)
)
}