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
27 changes: 16 additions & 11 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -441,22 +441,24 @@ re-exports them so its own call sites are undisturbed.

**A domain's wire shapes belong in `@virtool/contracts`, not in
`data.ts`.** What a server function returns is read by both sides, so
`data.ts` imports those types from the package and the client feature's
`types.ts` re-exports them (`references/types.ts` is the worked example).
A client `types.ts` must never `import type ... from "@server/*"` — that
points the client at the server's emitted declarations for a shape the
server does not own, and drags a data-layer module into the browser's
type graph to get it. `data.ts` still owns what only it uses: its
`*Values` and `*Options` argument types, its `AppError` subclasses, and
its row mappers.
`data.ts` imports those types from the package and components import the
same names straight from `@virtool/contracts` — no feature `types.ts`
re-export (`references/types.ts` and `samples/types.ts` are the worked
examples; each keeps only its genuinely client-only shapes). A client
`types.ts` must never `import type ... from "@server/*"` — that points
the client at the server's emitted declarations for a shape the server
does not own, and drags a data-layer module into the browser's type
graph to get it. `data.ts` still owns what only it uses: its `*Values`
and `*Options` argument types, its `AppError` subclasses, and its row
mappers.

### Every server function declares an authorization policy

Every server function names who may call it, as middleware, from
`@server/auth/policy`:

```ts
export const deleteGroup = createServerFn({ method: "POST" })
export const deleteGroupFn = createServerFn({ method: "POST" })
.middleware([adminRole("base")])
.validator(groupIdSchema)
.handler(async ({ context, data }) => { ... });
Expand Down Expand Up @@ -632,8 +634,11 @@ The basics:
string literal unions over `enum`.
- **JSDoc:** Every exported `type` gets a one-line `/** ... */`.
- **Naming:** `is`/`has`/`get` for pure reads; `check`/`validate`/
`assert` for may-throw. Don't suffix exports with their layer
(`Fn`, `Core`, `Handler`, `Impl`).
`assert` for may-throw. A `createServerFn` export gets an `Fn` suffix
(`loginFn`, `getSampleFn`) — it's an RPC call, not a plain function,
and the suffix marks that at every call site. The domain function it
wraps keeps the plain name (`login`, `getSample`) and never crosses
the network.
- **Comments:** Default to none. Document *why* when non-obvious, not
*what*.
- **Concurrency:** Independent awaits go in `Promise.all` — don't pay
Expand Down
4 changes: 2 additions & 2 deletions apps/web/src/account/account.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { accountQueryKeys } from "@account/keys";
import type { Account } from "@account/types";
import { getAccount } from "@server/users/functions";
import { getAccountFn } from "@server/users/functions";
import { queryOptions, useQuery } from "@tanstack/react-query";

/**
Expand All @@ -21,7 +21,7 @@ import { queryOptions, useQuery } from "@tanstack/react-query";
export function accountQueryOptions() {
return queryOptions<Account>({
queryKey: accountQueryKeys.all(),
queryFn: () => getAccount(),
queryFn: () => getAccountFn(),
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ describe("<AccountProfile />", () => {
expect(
await screen.findByText("Please specify a username"),
).toBeInTheDocument();
expect(userServerFnMocks.updateAccountHandle).not.toHaveBeenCalled();
expect(userServerFnMocks.updateAccountHandleFn).not.toHaveBeenCalled();
});

it("should handle password changes", async () => {
Expand Down
26 changes: 13 additions & 13 deletions apps/web/src/account/queries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ import { resetClient } from "@app/utils";
import type { Permissions } from "@groups/types";
import * as Sentry from "@sentry/tanstackstart-react";
import {
createApiKey,
deleteApiKey,
findApiKeys,
updateApiKey,
createApiKeyFn,
deleteApiKeyFn,
findApiKeysFn,
updateApiKeyFn,
} from "@server/account/functions";
import { logoutFn } from "@server/auth/functions";
import { updateAccountHandle } from "@server/users/functions";
import { updateAccountHandleFn } from "@server/users/functions";
import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query";
import type { User } from "@users/types";
import type { ErrorResponse } from "@/types/api";
Expand Down Expand Up @@ -50,11 +50,11 @@ export function useUpdateHandle() {
const queryClient = useQueryClient();

return useMutation<
Awaited<ReturnType<typeof updateAccountHandle>>,
Awaited<ReturnType<typeof updateAccountHandleFn>>,
Error,
{ handle: string }
>({
mutationFn: ({ handle }) => updateAccountHandle({ data: { handle } }),
mutationFn: ({ handle }) => updateAccountHandleFn({ data: { handle } }),
onSuccess: () => {
queryClient.invalidateQueries({ queryKey: accountQueryKeys.all() });
},
Expand Down Expand Up @@ -93,7 +93,7 @@ export function useChangePassword() {
export function useFetchApiKeys() {
return useQuery<ApiKey[]>({
queryKey: accountQueryKeys.apiKeys(),
queryFn: () => findApiKeys(),
queryFn: () => findApiKeysFn(),
});
}

Expand All @@ -106,12 +106,12 @@ export function useCreateApiKey() {
const queryClient = useQueryClient();

return useMutation<
Awaited<ReturnType<typeof createApiKey>>,
Awaited<ReturnType<typeof createApiKeyFn>>,
Error,
{ name: string; permissions: Permissions }
>({
mutationFn: ({ name, permissions }) =>
createApiKey({ data: { name, permissions } }),
createApiKeyFn({ data: { name, permissions } }),
onSuccess: () => {
queryClient.invalidateQueries({ queryKey: accountQueryKeys.apiKeys() });
},
Expand All @@ -127,12 +127,12 @@ export function useUpdateApiKey() {
const queryClient = useQueryClient();

return useMutation<
Awaited<ReturnType<typeof updateApiKey>>,
Awaited<ReturnType<typeof updateApiKeyFn>>,
Error,
{ keyId: number; permissions: Permissions }
>({
mutationFn: ({ keyId, permissions }) =>
updateApiKey({ data: { keyId, permissions } }),
updateApiKeyFn({ data: { keyId, permissions } }),
onSuccess: () => {
queryClient.invalidateQueries({ queryKey: accountQueryKeys.apiKeys() });
},
Expand All @@ -148,7 +148,7 @@ export function useRemoveApiKey() {
const queryClient = useQueryClient();

return useMutation<null, Error, { keyId: number }>({
mutationFn: ({ keyId }) => deleteApiKey({ data: { keyId } }),
mutationFn: ({ keyId }) => deleteApiKeyFn({ data: { keyId } }),
onSuccess: () => {
queryClient.invalidateQueries({ queryKey: accountQueryKeys.apiKeys() });
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@ const updateMessage = vi.fn();
const deleteMessage = vi.fn();

vi.mock("@server/messages/functions", () => ({
findMessage: vi.fn(),
findMessages: (...args: unknown[]) => findMessages(...args),
setActiveMessage: (...args: unknown[]) => setActiveMessage(...args),
clearActiveMessage: (...args: unknown[]) => clearActiveMessage(...args),
createMessage: (...args: unknown[]) => createMessage(...args),
updateMessage: (...args: unknown[]) => updateMessage(...args),
deleteMessage: (...args: unknown[]) => deleteMessage(...args),
findMessageFn: vi.fn(),
findMessagesFn: (...args: unknown[]) => findMessages(...args),
setActiveMessageFn: (...args: unknown[]) => setActiveMessage(...args),
clearActiveMessageFn: (...args: unknown[]) => clearActiveMessage(...args),
createMessageFn: (...args: unknown[]) => createMessage(...args),
updateMessageFn: (...args: unknown[]) => updateMessage(...args),
deleteMessageFn: (...args: unknown[]) => deleteMessage(...args),
}));

const { default: Banners } = await import("../Banners");
Expand Down
10 changes: 5 additions & 5 deletions apps/web/src/administration/queries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import {
roleQueryKeys,
settingsQueryKeys,
} from "@administration/keys";
import { getSettings, updateSettings } from "@server/settings/functions";
import { listAdministratorRoles } from "@server/users/functions";
import { getSettingsFn, updateSettingsFn } from "@server/settings/functions";
import { listAdministratorRolesFn } from "@server/users/functions";
import {
queryOptions,
useMutation,
Expand Down Expand Up @@ -34,7 +34,7 @@ export type SettingsUpdate = {
export function settingsQueryOptions() {
return queryOptions<Settings>({
queryKey: settingsQueryKeys.all(),
queryFn: () => getSettings(),
queryFn: () => getSettingsFn(),
});
}

Expand Down Expand Up @@ -67,7 +67,7 @@ export function useUpdateSettings() {
const queryClient = useQueryClient();

return useMutation<Settings, ErrorResponse, SettingsUpdate>({
mutationFn: (update) => updateSettings({ data: update }),
mutationFn: (update) => updateSettingsFn({ data: update }),
onSuccess: () => {
queryClient.invalidateQueries({
queryKey: settingsQueryKeys.all(),
Expand All @@ -87,7 +87,7 @@ export function useUpdateSettings() {
export function administratorRolesQueryOptions() {
return queryOptions({
queryKey: roleQueryKeys.all(),
queryFn: () => listAdministratorRoles(),
queryFn: () => listAdministratorRolesFn(),
});
}

Expand Down
2 changes: 1 addition & 1 deletion apps/web/src/analyses/components/Create/QuickAnalyze.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Dialog, DialogTitle } from "@base/Dialog";
import QueryError from "@base/QueryError";
import { useListHmms } from "@hmm/queries";
import type { SampleMinimal } from "@samples/types";
import type { SampleMinimal } from "@virtool/contracts";
import HmmAlert from "../HmmAlert";
import CreateAnalysisDialogContent from "./CreateAnalysisDialogContent";
import CreateAnalysisForm from "./CreateAnalysisForm";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { cn } from "@app/cn";
import Badge from "@base/Badge";
import BoxGroupSection from "@base/BoxGroupSection";
import type { SampleMinimal } from "@samples/types";
import type { SampleMinimal } from "@virtool/contracts";
import CreateAnalysisFieldTitle from "./CreateAnalysisFieldTitle";

type SelectedSamplesProps = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ import { screen, waitFor } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { mockApiCreateAnalysis } from "@tests/api/analyses";
import { mockApiListIndexes } from "@tests/api/indexes";
import { mockApiGetSampleDetail } from "@tests/api/samples";
import { createFakeAnalysisMinimal } from "@tests/fake/analyses";
import { createFakeIndexMinimal } from "@tests/fake/indexes";
import { createFakeSample } from "@tests/fake/samples";
import { mockGetSample } from "@tests/server-fn/samples";
import { mockListSubtractionsShortlist } from "@tests/server-fn/subtractions";
import { renderWithRouter } from "@tests/setup";
import { describe, expect, it, vi } from "vitest";
Expand All @@ -23,7 +23,7 @@ async function renderForm(indexId?: number) {

mockApiListIndexes([index]);
mockListSubtractionsShortlist([]);
mockApiGetSampleDetail(sample);
mockGetSample(sample);

await renderWithRouter(
<CreateAnalysisForm
Expand Down
2 changes: 1 addition & 1 deletion apps/web/src/analyses/components/Nuvs/NuvsViewer.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import NuvsList from "@analyses/components/Nuvs/NuvsList";
import NuvsToolbar from "@analyses/components/Nuvs/NuvsToolbar";
import type { FormattedNuvsAnalysis } from "@analyses/types";
import type { Sample } from "@samples/types";
import type { Sample } from "@virtool/contracts";

type NuVsViewerProps = {
/** Complete Nuvs analysis details */
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { useSortAndFilterPathoscopeHits } from "@analyses/hooks";
import type { FormattedPathoscopeAnalysis } from "@analyses/types";
import Accordion from "@base/Accordion";
import type { Sample } from "@samples/types";
import type { Sample } from "@virtool/contracts";
import { PathoscopeItem } from "./PathoscopeItem";

type PathoscopeListProps = {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { useAnalysisSearch } from "@analyses/components/AnalysisSearchContext";
import type { FormattedPathoscopeAnalysis } from "@analyses/types";
import Alert from "@base/Alert";
import type { Sample } from "@samples/types";
import type { Sample } from "@virtool/contracts";
import { PathoscopeList } from "./PathoscopeList";
import { AnalysisMapping } from "./PathoscopeMapping";
import { PathoscopeToolbar } from "./PathoscopeToolbar";
Expand Down
20 changes: 10 additions & 10 deletions apps/web/src/analyses/components/__tests__/AnalysisList.test.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import AnalysesList from "@analyses/components/AnalysisList";
import { screen } from "@testing-library/react";
import { mockApiGetAnalyses } from "@tests/api/analyses";
import { mockApiGetSampleDetail } from "@tests/api/samples";
import { createFakeAccount } from "@tests/fake/account";
import { createFakeAnalysisMinimal } from "@tests/fake/analyses";
import { createFakeHmmSearchResults } from "@tests/fake/hmm";
import { createFakeSample } from "@tests/fake/samples";
import { mockFindHmms } from "@tests/server-fn/hmm";
import { mockGetSample } from "@tests/server-fn/samples";
import { mockGetAccount } from "@tests/server-fn/users";
import { at, MemoryRouter, renderWithProviders } from "@tests/setup";
import nock from "nock";
Expand Down Expand Up @@ -35,7 +35,7 @@ describe("<AnalysesToolbar />", () => {

it("should show analysis creation when user is full admin", async () => {
mockGetAccount(createFakeAccount({ administrator_role: "full" }));
mockApiGetSampleDetail(sample);
mockGetSample(sample);
renderList();

expect(await screen.findByText("Create")).toBeInTheDocument();
Expand All @@ -45,7 +45,7 @@ describe("<AnalysesToolbar />", () => {
const account = createFakeAccount({ administrator_role: null });
sample.user.id = account.id;
mockGetAccount(account);
mockApiGetSampleDetail(sample);
mockGetSample(sample);
renderList();

expect(await screen.findByText("Create")).toBeInTheDocument();
Expand All @@ -54,29 +54,29 @@ describe("<AnalysesToolbar />", () => {
it("should show analysis creation when user is in the correct group and write is enabled", async () => {
const account = createFakeAccount({ administrator_role: null });
sample.group = at(account.groups, 0);
sample.group_write = true;
sample.groupWrite = true;
mockGetAccount(account);
mockApiGetSampleDetail(sample);
mockGetSample(sample);
renderList();

expect(await screen.findByText("Create")).toBeInTheDocument();
});

it("should show analysis creation when all users editing a sample is permitted", async () => {
const account = createFakeAccount({ administrator_role: null });
sample.all_write = true;
sample.allWrite = true;
mockGetAccount(account);
mockApiGetSampleDetail(sample);
mockGetSample(sample);
renderList();

expect(await screen.findByText("Create")).toBeInTheDocument();
});

it("should not render analysis creation option when user has no permissions", async () => {
sample.all_write = false;
sample.group_write = false;
sample.allWrite = false;
sample.groupWrite = false;
mockGetAccount(createFakeAccount({ administrator_role: null }));
mockApiGetSampleDetail(sample);
mockGetSample(sample);
renderList();

expect(await screen.findByText("Pathoscope")).toBeInTheDocument();
Expand Down
Loading