Skip to content

Change plan feedback modal #1393

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 23 commits into from
Oct 10, 2024
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
e3398dd
Checkbox component can have its label styles
samejr Oct 3, 2024
ac81a9c
Improved the dialog footer
samejr Oct 3, 2024
d8c1872
Handle sending feedback to Slack using Plain
samejr Oct 3, 2024
22efefa
WIP making the modal conditional
samejr Oct 3, 2024
4bae709
Moved the Plain form action into the select plan file
samejr Oct 3, 2024
a0071b9
removed comment
samejr Oct 3, 2024
1d014af
Show a confirmation diaglog if you’re downgrading from Pro to Hobby
samejr Oct 4, 2024
51b437b
Downgrading to Hobby works
samejr Oct 4, 2024
431eafb
Use redirectWithErrorMessage instead of throw error
samejr Oct 6, 2024
fd01de8
The cancel form now submits the data correctly
samejr Oct 8, 2024
049d126
Modals don’t trigger when you upgrade
samejr Oct 8, 2024
ac2c6d7
Copy improvements
samejr Oct 8, 2024
9adffd1
Added a tooltip to explain the link to the pricing page
samejr Oct 8, 2024
b837b5a
Squashed commit of the following:
samejr Oct 8, 2024
c976813
Squashed commit of the following:
samejr Oct 8, 2024
cc9615d
Revert "Squashed commit of the following:"
nicktrn Oct 8, 2024
61cb4a7
Merge remote-tracking branch 'origin/main' into change-plan-feedback-…
nicktrn Oct 8, 2024
5e9f241
Removed console logs
samejr Oct 9, 2024
db7bf32
cleaned up conditionals
samejr Oct 9, 2024
255af74
Unlock free plan state
samejr Oct 9, 2024
0877a70
Fixed subscribe button if you’re already github verified
samejr Oct 10, 2024
300f14a
Simplified the downgrade reasons logic
samejr Oct 10, 2024
64b08db
made periodEnd required
samejr Oct 10, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
20 changes: 3 additions & 17 deletions .changeset/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,27 +7,13 @@
}
],
"commit": false,
"fixed": [
[
"@trigger.dev/*",
"trigger.dev"
]
],
"fixed": [["@trigger.dev/*", "trigger.dev"]],
"linked": [],
"access": "public",
"baseBranch": "main",
"updateInternalDependencies": "patch",
"ignore": [
"webapp",
"emails",
"proxy",
"@trigger.dev/database",
"coordinator",
"docker-provider",
"kubernetes-provider",
"@trigger.dev/otlp-importer"
],
"ignore": ["webapp", "proxy", "coordinator", "docker-provider", "kubernetes-provider"],
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Packages removed from the ignore array are present in the project.

The packages "emails", "@trigger.dev/database", and "@trigger.dev/otlp-importer" still exist in the project directories. Removing them from the ignore array means they will now be included in the changeset process, which could impact versioning and changelog generation.

🔗 Analysis chain

Please clarify the removal of entries from the ignore array.

The ignore array has been modified to remove several entries: "emails", "@trigger.dev/database", and "@trigger.dev/otlp-importer". This change means that these packages will now be included in the changeset process, potentially affecting versioning and changelog generation.

Could you please explain the rationale behind this change? It's important to understand:

  1. Why these specific packages are no longer being ignored?
  2. What impact this might have on the project's versioning and release process?
  3. How does this change align with the PR objectives, which don't mention updates to the changeset configuration?

To help assess the impact, let's check if these packages are still present in the project:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if the removed packages still exist in the project
echo "Checking for 'emails' directory:"
fd -t d '^emails$'
echo "Checking for '@trigger.dev/database' package:"
fd -t f 'package.json' | xargs grep -l '"name": "@trigger.dev/database"'
echo "Checking for '@trigger.dev/otlp-importer' package:"
fd -t f 'package.json' | xargs grep -l '"name": "@trigger.dev/otlp-importer"'

Length of output: 689

"___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH": {
"onlyUpdatePeerDependentsWhenOutOfRange": true
}
}
}
3 changes: 3 additions & 0 deletions .github/workflows/unit-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,8 @@ jobs:
- name: 📥 Download deps
run: pnpm install --frozen-lockfile

- name: 📀 Generate Prisma Client
run: pnpm run generate

- name: 🧪 Run Unit Tests
run: pnpm run test
4 changes: 2 additions & 2 deletions .gitmodules
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
[submodule "packages/otlp-importer/protos"]
path = packages/otlp-importer/protos
[submodule "internal-packages/otlp-importer/protos"]
path = internal-packages/otlp-importer/protos
url = https://github.com/open-telemetry/opentelemetry-proto.git
19 changes: 19 additions & 0 deletions apps/webapp/app/assets/icons/TimedOutIcon.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
export function TimedOutIcon({ className }: { className?: string }) {
return (
<svg
className={className}
width="20"
height="20"
viewBox="0 0 20 20"
fill="none"
xmlns="http://www.w3.org/2000/svg"
>
<path
fillRule="evenodd"
clipRule="evenodd"
d="M9 2H8V3H9V4.07089C5.60771 4.55612 3 7.47353 3 11C3 14.866 6.13401 18 10 18C13.866 18 17 14.866 17 11C17 7.47353 14.3923 4.55612 11 4.07089V3H12V2H11H9ZM13.7603 3.36575C14.7218 2.40422 16.2807 2.40422 17.2422 3.36575C18.2038 4.32727 18.2038 5.8862 17.2422 6.84772L17.1462 6.94375C17.1251 6.96488 17.0908 6.96488 17.0697 6.94375L13.6642 3.53831C13.6431 3.51717 13.6431 3.4829 13.6642 3.46177L13.7603 3.36575ZM6.28876 3.58524C6.33584 3.53816 6.33584 3.46184 6.28876 3.41476L6.23971 3.36571C5.27819 2.40419 3.71925 2.40419 2.75773 3.36571C1.79621 4.32723 1.79621 5.88616 2.75773 6.84769L2.80678 6.89674C2.85386 6.94382 2.93019 6.94381 2.97726 6.89674L6.28876 3.58524ZM14.5858 17L16 15.5858L16.7071 16.2929C17.0976 16.6834 17.0976 17.3166 16.7071 17.7071C16.3166 18.0976 15.6834 18.0976 15.2929 17.7071L14.5858 17ZM5.42297 17L4.00875 15.5858L3.30165 16.2929C2.91112 16.6834 2.91112 17.3166 3.30165 17.7071C3.69217 18.0977 4.32534 18.0977 4.71586 17.7071L5.42297 17ZM6.29289 7.29289C6.68342 6.90237 7.31658 6.90237 7.70711 7.29289L10 9.58579L12.2929 7.29289C12.6834 6.90237 13.3166 6.90237 13.7071 7.29289C14.0976 7.68342 14.0976 8.31658 13.7071 8.70711L11.4142 11L13.7071 13.2929C14.0976 13.6834 14.0976 14.3166 13.7071 14.7071C13.3166 15.0976 12.6834 15.0976 12.2929 14.7071L10 12.4142L7.70711 14.7071C7.31658 15.0976 6.68342 15.0976 6.29289 14.7071C5.90237 14.3166 5.90237 13.6834 6.29289 13.2929L8.58579 11L6.29289 8.70711C5.90237 8.31658 5.90237 7.68342 6.29289 7.29289Z"
fill="currentColor"
/>
</svg>
);
}
5 changes: 4 additions & 1 deletion apps/webapp/app/components/primitives/Checkbox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ export type CheckboxProps = Omit<
description?: string;
badges?: string[];
className?: string;
labelClassName?: string;
onChange?: (isChecked: boolean) => void;
};

Expand All @@ -78,6 +79,7 @@ export const CheckboxWithLabel = React.forwardRef<HTMLInputElement, CheckboxProp
badges,
disabled,
className,
labelClassName: externalLabelClassName,
...props
},
ref
Expand Down Expand Up @@ -148,7 +150,8 @@ export const CheckboxWithLabel = React.forwardRef<HTMLInputElement, CheckboxProp
htmlFor={id}
className={cn(
props.readOnly || disabled ? "cursor-default" : "cursor-pointer",
labelClassName
labelClassName,
externalLabelClassName
)}
onClick={(e) => e.preventDefault()}
>
Expand Down
2 changes: 1 addition & 1 deletion apps/webapp/app/components/primitives/Dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ DialogHeader.displayName = "DialogHeader";

const DialogFooter = ({ className, ...props }: React.HTMLAttributes<HTMLDivElement>) => (
<div
className={cn("flex flex-col-reverse sm:flex-row sm:justify-end sm:space-x-2", className)}
className={cn("flex flex-col-reverse sm:flex-row sm:justify-between sm:space-x-2", className)}
{...props}
/>
);
Expand Down
2 changes: 1 addition & 1 deletion apps/webapp/app/components/primitives/Select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -613,7 +613,7 @@ export function SelectPopover({
"z-50 flex flex-col overflow-clip rounded border border-charcoal-700 bg-background-bright shadow-md outline-none animate-in fade-in-40",
"min-w-[max(180px,calc(var(--popover-anchor-width)+0.5rem))]",
"max-w-[min(480px,var(--popover-available-width))]",
"max-h-[min(480px,var(--popover-available-height))]",
"max-h-[min(520px,var(--popover-available-height))]",
"origin-[var(--popover-transform-origin)]",
className
)}
Expand Down
6 changes: 4 additions & 2 deletions apps/webapp/app/components/primitives/TreeView/TreeView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -235,9 +235,11 @@ export function useTree<TData, TFilterValue>({
getItemKey: (index) => state.visibleNodeIds[index],
getScrollElement: () => parentRef.current,
estimateSize: (index: number) => {
const treeItem = tree[index];
if (!treeItem) return 0;
return estimatedRowHeight({
node: tree[index],
state: state.nodes[tree[index].id],
node: treeItem,
state: state.nodes[treeItem.id],
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix incorrect tree item retrieval in estimateSize function

The index in the estimateSize function corresponds to the position in state.visibleNodeIds, not the tree array. Using tree[index] may retrieve incorrect tree items or cause errors.

To correctly retrieve the tree item, use the ID from state.visibleNodeIds[index] to find the corresponding tree item in tree.

Apply this diff to fix the issue:

-const treeItem = tree[index];
+const nodeId = state.visibleNodeIds[index];
+const treeItem = tree.find((item) => item.id === nodeId);
 if (!treeItem) return 0;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const treeItem = tree[index];
if (!treeItem) return 0;
return estimatedRowHeight({
node: tree[index],
state: state.nodes[tree[index].id],
node: treeItem,
state: state.nodes[treeItem.id],
const nodeId = state.visibleNodeIds[index];
const treeItem = tree.find((item) => item.id === nodeId);
if (!treeItem) return 0;
return estimatedRowHeight({
node: treeItem,
state: state.nodes[treeItem.id],

index,
});
},
Expand Down
6 changes: 6 additions & 0 deletions apps/webapp/app/components/runs/v3/RunInspector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,12 @@ export function RunInspector({
</Property.Value>
</Property.Item>
)}
<Property.Item>
<Property.Label>Max duration</Property.Label>
<Property.Value>
{run.maxDurationInSeconds ? `${run.maxDurationInSeconds}s` : "–"}
</Property.Value>
</Property.Item>
<Property.Item>
<Property.Label>Run invocation cost</Property.Label>
<Property.Value>
Expand Down
11 changes: 11 additions & 0 deletions apps/webapp/app/components/runs/v3/TaskRunStatus.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,14 @@ import {
NoSymbolIcon,
PauseCircleIcon,
RectangleStackIcon,
StopIcon,
TrashIcon,
XCircleIcon,
} from "@heroicons/react/20/solid";
import { TaskRunStatus } from "@trigger.dev/database";
import assertNever from "assert-never";
import { SnowflakeIcon } from "lucide-react";
import { TimedOutIcon } from "~/assets/icons/TimedOutIcon";
import { Spinner } from "~/components/primitives/Spinner";
import { cn } from "~/utils/cn";

Expand All @@ -27,6 +29,7 @@ export const allTaskRunStatuses = [
"COMPLETED_SUCCESSFULLY",
"CANCELED",
"COMPLETED_WITH_ERRORS",
"TIMED_OUT",
"CRASHED",
"PAUSED",
"INTERRUPTED",
Expand All @@ -44,6 +47,7 @@ export const filterableTaskRunStatuses = [
"COMPLETED_SUCCESSFULLY",
"CANCELED",
"COMPLETED_WITH_ERRORS",
"TIMED_OUT",
"CRASHED",
"INTERRUPTED",
"SYSTEM_FAILURE",
Expand All @@ -65,6 +69,7 @@ const taskRunStatusDescriptions: Record<TaskRunStatus, string> = {
PAUSED: "Task has been paused by the user",
CRASHED: "Task has crashed and won't be retried",
EXPIRED: "Task has surpassed its ttl and won't be executed",
TIMED_OUT: "Task has failed because it exceeded its maxDuration",
};

export const QUEUED_STATUSES = [
Expand Down Expand Up @@ -140,6 +145,8 @@ export function TaskRunStatusIcon({
return <FireIcon className={cn(runStatusClassNameColor(status), className)} />;
case "EXPIRED":
return <TrashIcon className={cn(runStatusClassNameColor(status), className)} />;
case "TIMED_OUT":
return <TimedOutIcon className={cn(runStatusClassNameColor(status), className)} />;

default: {
assertNever(status);
Expand Down Expand Up @@ -174,6 +181,8 @@ export function runStatusClassNameColor(status: TaskRunStatus): string {
return "text-error";
case "CRASHED":
return "text-error";
case "TIMED_OUT":
return "text-error";
default: {
assertNever(status);
}
Expand Down Expand Up @@ -210,6 +219,8 @@ export function runStatusTitle(status: TaskRunStatus): string {
return "Crashed";
case "EXPIRED":
return "Expired";
case "TIMED_OUT":
return "Timed out";
default: {
assertNever(status);
}
Expand Down
37 changes: 33 additions & 4 deletions apps/webapp/app/components/runs/v3/TaskRunsTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@ import { BeakerIcon, BookOpenIcon, CheckIcon } from "@heroicons/react/24/solid";
import { useLocation } from "@remix-run/react";
import { formatDuration, formatDurationMilliseconds } from "@trigger.dev/core/v3";
import { useCallback, useRef } from "react";
import { Badge } from "~/components/primitives/Badge";
import { Button, LinkButton } from "~/components/primitives/Buttons";
import { Checkbox } from "~/components/primitives/Checkbox";
import { Dialog, DialogTrigger } from "~/components/primitives/Dialog";
import { Header3 } from "~/components/primitives/Headers";
import { useSelectedItems } from "~/components/primitives/SelectedItemsProvider";
import { SimpleTooltip } from "~/components/primitives/Tooltip";
import { useEnvironments } from "~/hooks/useEnvironments";
import { useFeatures } from "~/hooks/useFeatures";
import { useOrganization } from "~/hooks/useOrganizations";
Expand All @@ -39,9 +41,12 @@ import {
import { CancelRunDialog } from "./CancelRunDialog";
import { LiveTimer } from "./LiveTimer";
import { ReplayRunDialog } from "./ReplayRunDialog";
import { TaskRunStatusCombo } from "./TaskRunStatus";
import { RunTag } from "./RunTag";
import { Badge } from "~/components/primitives/Badge";
import {
descriptionForTaskRunStatus,
filterableTaskRunStatuses,
TaskRunStatusCombo,
} from "./TaskRunStatus";

type RunsTableProps = {
total: number;
Expand Down Expand Up @@ -126,7 +131,27 @@ export function TaskRunsTable({
<TableHeaderCell>Env</TableHeaderCell>
<TableHeaderCell>Task</TableHeaderCell>
<TableHeaderCell>Version</TableHeaderCell>
<TableHeaderCell>Status</TableHeaderCell>
<TableHeaderCell
tooltip={
<div className="flex flex-col divide-y divide-grid-dimmed">
{filterableTaskRunStatuses.map((status) => (
<div
key={status}
className="grid grid-cols-[8rem_1fr] gap-x-2 py-2 first:pt-1 last:pb-1"
>
<div className="mb-0.5 flex items-center gap-1.5 whitespace-nowrap">
<TaskRunStatusCombo status={status} />
</div>
<Paragraph variant="extra-small" className="!text-wrap text-text-dimmed">
{descriptionForTaskRunStatus(status)}
</Paragraph>
</div>
))}
</div>
}
>
Status
</TableHeaderCell>
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider extracting the tooltip content into a separate component for better maintainability.

The tooltip prop of the TableHeaderCell contains complex JSX code that generates the tooltip content for the "Status" column header. Extracting this into a separate component or a constant could improve readability and make the codebase easier to maintain.

You could create a new component like StatusColumnTooltip and use it as:

import { StatusColumnTooltip } from "./StatusColumnTooltip";

// ...

<TableHeaderCell tooltip={<StatusColumnTooltip />}>
  Status
</TableHeaderCell>

This refactor promotes reusability and keeps the TaskRunsTable component cleaner.

<TableHeaderCell>Started</TableHeaderCell>
<TableHeaderCell
colSpan={3}
Expand Down Expand Up @@ -287,7 +312,11 @@ export function TaskRunsTable({
</TableCell>
<TableCell to={path}>{run.version ?? "–"}</TableCell>
<TableCell to={path}>
<TaskRunStatusCombo status={run.status} />
<SimpleTooltip
content={descriptionForTaskRunStatus(run.status)}
disableHoverableContent
button={<TaskRunStatusCombo status={run.status} />}
/>
</TableCell>
<TableCell to={path}>
{run.startedAt ? <DateTime date={run.startedAt} /> : "–"}
Expand Down
1 change: 1 addition & 0 deletions apps/webapp/app/database-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export const TaskRunStatus = {
CRASHED: "CRASHED",
DELAYED: "DELAYED",
EXPIRED: "EXPIRED",
TIMED_OUT: "TIMED_OUT",
} as const satisfies Record<TaskRunStatusType, TaskRunStatusType>;

export const JobRunStatus = {
Expand Down
68 changes: 21 additions & 47 deletions apps/webapp/app/db.server.ts
Original file line number Diff line number Diff line change
@@ -1,77 +1,51 @@
import { Prisma, PrismaClient } from "@trigger.dev/database";
import {
Prisma,
PrismaClient,
PrismaClientOrTransaction,
PrismaReplicaClient,
PrismaTransactionClient,
PrismaTransactionOptions,
} from "@trigger.dev/database";
import invariant from "tiny-invariant";
import { z } from "zod";
import { env } from "./env.server";
import { logger } from "./services/logger.server";
import { isValidDatabaseUrl } from "./utils/db";
import { singleton } from "./utils/singleton";
import { $transaction as transac } from "@trigger.dev/database";

export type PrismaTransactionClient = Omit<
PrismaClient,
"$connect" | "$disconnect" | "$on" | "$transaction" | "$use" | "$extends"
>;

export type PrismaClientOrTransaction = PrismaClient | PrismaTransactionClient;

function isTransactionClient(prisma: PrismaClientOrTransaction): prisma is PrismaTransactionClient {
return !("$transaction" in prisma);
}

function isPrismaKnownError(error: unknown): error is Prisma.PrismaClientKnownRequestError {
return (
typeof error === "object" && error !== null && "code" in error && typeof error.code === "string"
);
}

export type PrismaTransactionOptions = {
/** The maximum amount of time (in ms) Prisma Client will wait to acquire a transaction from the database. The default value is 2000ms. */
maxWait?: number;

/** The maximum amount of time (in ms) the interactive transaction can run before being canceled and rolled back. The default value is 5000ms. */
timeout?: number;

/** Sets the transaction isolation level. By default this is set to the value currently configured in your database. */
isolationLevel?: Prisma.TransactionIsolationLevel;

swallowPrismaErrors?: boolean;
export type {
PrismaTransactionClient,
PrismaClientOrTransaction,
PrismaTransactionOptions,
PrismaReplicaClient,
};

export async function $transaction<R>(
prisma: PrismaClientOrTransaction,
fn: (prisma: PrismaTransactionClient) => Promise<R>,
options?: PrismaTransactionOptions
): Promise<R | undefined> {
if (isTransactionClient(prisma)) {
return fn(prisma);
}

try {
return await (prisma as PrismaClient).$transaction(fn, options);
} catch (error) {
if (isPrismaKnownError(error)) {
return transac(
prisma,
fn,
(error) => {
logger.error("prisma.$transaction error", {
code: error.code,
meta: error.meta,
stack: error.stack,
message: error.message,
name: error.name,
});

if (options?.swallowPrismaErrors) {
return;
}
}

throw error;
}
},
options
);
}

export { Prisma };

export const prisma = singleton("prisma", getClient);

export type PrismaReplicaClient = Omit<PrismaClient, "$transaction">;

export const $replica: PrismaReplicaClient = singleton(
"replica",
() => getReplicaClient() ?? prisma
Expand Down
3 changes: 3 additions & 0 deletions apps/webapp/app/env.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,9 @@ const EnvironmentSchema = z.object({
TASK_PAYLOAD_OFFLOAD_THRESHOLD: z.coerce.number().int().default(524_288), // 512KB
TASK_PAYLOAD_MAXIMUM_SIZE: z.coerce.number().int().default(3_145_728), // 3MB
TASK_RUN_METADATA_MAXIMUM_SIZE: z.coerce.number().int().default(4_096), // 4KB

MAXIMUM_DEV_QUEUE_SIZE: z.coerce.number().int().optional(),
MAXIMUM_DEPLOYED_QUEUE_SIZE: z.coerce.number().int().optional(),
});

export type Environment = z.infer<typeof EnvironmentSchema>;
Expand Down
1 change: 1 addition & 0 deletions apps/webapp/app/models/taskRun.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ export function batchTaskRunItemStatusForRunStatus(
case TaskRunStatus.SYSTEM_FAILURE:
case TaskRunStatus.CRASHED:
case TaskRunStatus.EXPIRED:
case TaskRunStatus.TIMED_OUT:
return BatchTaskRunItemStatus.FAILED;
case TaskRunStatus.PENDING:
case TaskRunStatus.WAITING_FOR_DEPLOY:
Expand Down
Loading