Skip to content
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
Original file line number Diff line number Diff line change
Expand Up @@ -3,24 +3,21 @@ import { useProjectLayout } from "../../../../layout-provider";

type Props = {
deploymentId: string;
// I couldn't figure out how to make the domains revalidate on a rollback
// From my understanding it should already work, because we're using the
// .util.refetch() in the trpc mutation, but it doesn't.
// We need to investigate this later
hackyRevalidateDependency?: unknown;
};

export const DomainList = ({ deploymentId, hackyRevalidateDependency }: Props) => {
export const DomainList = ({ deploymentId }: Props) => {
const { collections } = useProjectLayout();
const domains = useLiveQuery(
(q) =>
q
.from({ domain: collections.domains })
.where(({ domain }) => eq(domain.deploymentId, deploymentId))
.orderBy(({ domain }) => domain.domain, "asc"),
[hackyRevalidateDependency],
const domains = useLiveQuery((q) =>
q
.from({ domain: collections.domains })
.where(({ domain }) => eq(domain.deploymentId, deploymentId))
.orderBy(({ domain }) => domain.domain, "asc"),
);

if (domains.isLoading || !domains.data.length) {
return <DomainListSkeleton />;
}

return (
<ul className="flex flex-col list-none py-2">
{domains.data.map((domain) => (
Expand All @@ -29,3 +26,13 @@ export const DomainList = ({ deploymentId, hackyRevalidateDependency }: Props) =
</ul>
);
};

const DomainListSkeleton = () => (
<ul className="flex flex-col list-none py-2 gap-1">
{[1, 2, 3].map((i) => (
<li key={i}>
<div className="h-3 w-64 bg-grayA-3 rounded animate-pulse" />
</li>
))}
</ul>
);
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@ import type { Column } from "@/components/virtual-table/types";
import { useIsMobile } from "@/hooks/use-mobile";
import type { Deployment, Environment } from "@/lib/collections";
import { shortenId } from "@/lib/shorten-id";
import { BookBookmark, Cloud, CodeBranch, Cube } from "@unkey/icons";
import { BookBookmark, CodeBranch, Cube } from "@unkey/icons";
import { Button, Empty, TimestampInfo } from "@unkey/ui";
import { cn } from "@unkey/ui/src/lib/utils";
import dynamic from "next/dynamic";
import { useMemo, useState } from "react";
import { Avatar } from "../../../details/active-deployment-card/git-avatar";
import { StatusIndicator } from "../../../details/active-deployment-card/status-indicator";
import { useDeployments } from "../../hooks/use-deployments";
import { DeploymentStatusBadge } from "./components/deployment-status-badge";
import { DomainList } from "./components/domain_list";
Expand Down Expand Up @@ -64,18 +65,7 @@ export const DeploymentsList = () => {
headerClassName: "pl-[18px]",
render: ({ deployment, environment }) => {
const isLive = liveDeployment?.id === deployment.id;
const isSelected = deployment.id === selectedDeployment?.deployment.id;
const iconContainer = (
<div
className={cn(
"size-5 rounded flex items-center justify-center cursor-pointer border border-grayA-3 transition-all duration-100",
"bg-grayA-3",
isSelected && "bg-grayA-5",
)}
>
<Cloud size="sm-regular" className="text-gray-12" />
</div>
);
const iconContainer = <StatusIndicator withSignal={isLive} />;
return (
<div className="flex flex-col items-start px-[18px] py-1.5">
<div className="flex gap-3 items-center w-full">
Expand Down Expand Up @@ -127,7 +117,6 @@ export const DeploymentsList = () => {
<DomainList
key={`${deployment.id}-${liveDeployment}-${project?.isRolledBack}`}
deploymentId={deployment.id}
hackyRevalidateDependency={project?.liveDeploymentId}
/>
),
},
Expand Down Expand Up @@ -237,13 +226,13 @@ export const DeploymentsList = () => {
<div className="flex gap-3 items-center w-full">
<Avatar
src={deployment.gitCommitAuthorAvatarUrl}
alt={deployment.gitCommitAuthorUsername ?? "Author"}
alt={deployment.gitCommitAuthorHandle ?? "Author"}
/>

<div className="w-[200px]">
<div className="flex items-center gap-2">
<span className="font-medium text-grayA-12 text-xs">
{deployment.gitCommitAuthorUsername}
{deployment.gitCommitAuthorHandle}
</span>
</div>
<div className={cn("font-mono text-xs mt-1", "text-gray-9")}>
Expand Down Expand Up @@ -283,10 +272,10 @@ export const DeploymentsList = () => {
<div className="flex items-center gap-2">
<Avatar
src={deployment.gitCommitAuthorAvatarUrl}
alt={deployment.gitCommitAuthorUsername ?? "Author"}
alt={deployment.gitCommitAuthorHandle ?? "Author"}
/>
<span className="font-medium text-grayA-12 text-xs">
{deployment.gitCommitAuthorName}
{deployment.gitCommitAuthorHandle}
</span>
</div>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { cn } from "@/lib/utils";
import { User } from "@unkey/icons";
import { useState } from "react";

Expand All @@ -7,12 +8,17 @@ type AvatarProps = {
className?: string;
};

export function Avatar({ src, alt, className = "size-5" }: AvatarProps) {
export function Avatar({ src, alt, className }: AvatarProps) {
const [hasError, setHasError] = useState(false);

if (!src || hasError) {
return (
<div className="size-5 border rounded-full border-grayA-5 items-center flex justify-center">
<div
className={cn(
"size-5 border rounded-full border-grayA-5 items-center flex justify-center",
className,
)}
>
<User size="md-medium" />
</div>
);
Expand All @@ -22,7 +28,7 @@ export function Avatar({ src, alt, className = "size-5" }: AvatarProps) {
<img
src={src}
alt={alt}
className={`rounded-full ${className}`}
className={cn("size-5 rounded-full object-cover", className)}
onError={() => setHasError(true)}
/>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ export const ActiveDeploymentCard = ({ deploymentId }: Props) => {
<span className="text-gray-9 text-xs">Created by</span>
<Avatar src={deployment.gitCommitAuthorAvatarUrl} alt="Author" />
<span className="font-medium text-grayA-12 text-xs">
{deployment.gitCommitAuthorName}
{deployment.gitCommitAuthorHandle}
</span>
</div>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,31 @@ export const ProjectDetailsExpandable = ({
);

const data = query.data.at(0);
const { data: domainsData } = useLiveQuery(
(q) =>
q
.from({ domain: collections.domains })
.where(({ domain }) => eq(domain.deploymentId, data?.project.liveDeploymentId))
.select(({ domain }) => ({
domain: domain.domain,
environment: domain.sticky,
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is very misleading, cause the stickyness doesn't necessarily have anything to do with environments

Copy link
Collaborator

Choose a reason for hiding this comment

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

we should pass through the sticky value as is or do isEnvironment: domain.sticky === "environment' or similar

we can defer to an issue here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll address that in my diffing PR which is already based off this PR 🫡

Copy link
Collaborator

Choose a reason for hiding this comment

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

@coderabbitai please create an issue and assign @ogzhanolguncu

Copy link
Contributor

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Create a GitHub issue for the misleading environment/sticky field naming

gh issue create \
  --title "Refactor: Clarify domain sticky field usage in project details" \
  --body "## Context

In PR #4034, specifically in the [review comment](https://github.com/unkeyed/unkey/pull/4034#discussion_r2393658153), @chronark identified that the field naming \`environment: domain.sticky\` is misleading because the stickyness doesn't necessarily have anything to do with environments.

**File:** \`apps/dashboard/app/(app)/[workspaceSlug]/projects/[projectId]/details/project-details-expandables/index.tsx\`

**Current code:**
\`\`\`typescript
.select(({ domain }) => ({
  domain: domain.domain,
  environment: domain.sticky,
}))
\`\`\`

## Suggested Solutions

1. Pass through the sticky value as-is without renaming
2. Use a boolean like \`isEnvironment: domain.sticky === 'environment'\` to make the intent clear

## Reporter

@chronark

## Related
- PR: #4034
- Comment: https://github.com/unkeyed/unkey/pull/4034#discussion_r2393658153" \
  --assignee ogzhanolguncu

Length of output: 44


@chronark I've created issue #4047 and assigned it to @ogzhanolguncu. The issue captures the concern about the misleading environment: domain.sticky field naming and includes the suggested solutions you mentioned.

}))
.orderBy(({ domain }) => domain.id, "asc"),
[data?.project.liveDeploymentId],
);

if (!data?.deployment) {
return null;
}

const detailSections = createDetailSections(data.deployment);
const detailSections = createDetailSections({
...data.deployment,
repository: data.project.gitRepositoryUrl,
});

// This "environment" domain never changes even when you do a rollback this one stays stable.
const mainDomain = domainsData.find((d) => d.environment === "environment")?.domain;
const gitShaAndBranchNameDomains = domainsData.filter((d) => d.environment !== "environment");

return (
<div className="flex">
Expand Down Expand Up @@ -101,43 +120,46 @@ export const ProjectDetailsExpandable = ({
<Cube size="2xl-medium" className="!size-[20px]" />
</Button>
<div className="flex flex-col gap-1">
<span className="text-accent-12 font-medium text-sm">dashboard</span>
<div className="gap-2 items-center flex">
<span className="text-accent-12 font-medium text-sm truncate">
{data.project.name}
</span>
<div className="gap-2 items-center flex min-w-0 max-w-[250px]">
{/* # is okay. This section is not accessible without deploy*/}
<a
href="https://api.gateway.com"
href={mainDomain ?? "#"}
target="_blank"
rel="noopener noreferrer"
className="text-gray-9 text-sm hover:underline"
className="text-gray-9 text-sm truncate block transition-all hover:underline decoration-dashed underline-offset-2"
>
api.gateway.com
{mainDomain}
</a>
<InfoTooltip
position={{
side: "bottom",
}}
content={
<div className="space-y-1">
{["staging.gateway.com", "dev.gateway.com"].map((region) => (
<div className="space-y-2 max-w-[300px] py-2">
{gitShaAndBranchNameDomains.map((d) => (
<div
key={region}
key={d.domain}
className="text-xs font-medium flex items-center gap-1.5"
>
<div className="w-1 h-1 bg-gray-8 rounded-full" />
<div className="w-1 h-1 bg-gray-8 rounded-full shrink-0" />
<a
href={`https://${region}`}
href={`https://${d.domain}`}
target="_blank"
rel="noopener noreferrer"
className="hover:underline"
className="transition-all hover:underline decoration-dashed underline-offset-2"
>
{region}
{d.domain}
</a>
</div>
))}
</div>
}
>
<div className="rounded-full px-1.5 py-0.5 bg-grayA-3 text-gray-12 text-xs leading-[18px] font-mono tabular-nums">
+2
+{gitShaAndBranchNameDomains.length}
</div>
</InfoTooltip>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
} from "@unkey/icons";
import { Badge, TimestampInfo } from "@unkey/ui";
import type { ReactNode } from "react";
import { RepoDisplay } from "../../../_components/list/repo-display";
import { Avatar } from "../active-deployment-card/git-avatar";

export type DetailItem = {
Expand All @@ -30,17 +31,21 @@ export type DetailSection = {
items: DetailItem[];
};

export const createDetailSections = (details: Deployment): DetailSection[] => [
export const createDetailSections = (
details: Deployment & { repository: string | null },
): DetailSection[] => [
{
title: "Active deployment",
items: [
{
icon: <Github className="size-[16px] text-gray-12" />,
label: "Repository",
content: (
<div className="text-grayA-10">
<span className="text-gray-12 font-medium">TODO</span>/ TODO
</div>
<RepoDisplay
url={details.repository || "—"}
showIcon={false}
className="text-gray-12 font-medium"
/>
),
},
{
Expand Down Expand Up @@ -75,9 +80,9 @@ export const createDetailSections = (details: Deployment): DetailSection[] => [
<div className="flex gap-2 items-center">
<Avatar
src={details.gitCommitAuthorAvatarUrl}
alt={details.gitCommitAuthorUsername ?? ""}
alt={details.gitCommitAuthorHandle ?? ""}
/>
<span className="font-medium text-grayA-12">{details.gitCommitAuthorUsername}</span>
<span className="font-medium text-grayA-12">{details.gitCommitAuthorHandle}</span>
</div>
),
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ export const CreateProjectDialog = () => {
updatedAt: null,
id: "will-be-replace-by-server",
author: "will-be-replace-by-server",
authorAvatar: "will-be-replace-by-server",
branch: "will-be-replace-by-server",
commitTimestamp: Date.now(),
commitTitle: "will-be-replace-by-server",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ export const ProjectsList = () => {
commitTimestamp={project.commitTimestamp}
branch={project.branch}
author={project.author}
authorAvatar={project.authorAvatar}
regions={project.regions}
repository={project.gitRepositoryUrl || undefined}
actions={
Expand Down
Loading