address greptile review feedback (greploop iteration 2)

- Add PolicyVersionsData type for select output; specify TData generic
  so consumers get Policy[] (not Policy[] | undefined) for versions
- Remove empty-string queryKey fallback — use policyName! since
  enabled:false prevents fetch when policyName is null
- Add cache invalidation tests for both mutation hooks
- Add explanatory comment for ?? [] fallback in component

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
yuneng-jiang 2026-03-20 23:33:50 -07:00
parent 80e55804af
commit 41d12ed106
3 changed files with 42 additions and 3 deletions

View file

@ -172,6 +172,22 @@ describe("useCreatePolicyVersion", () => {
expect(NotificationsManager.success).toHaveBeenCalledWith("New draft version created");
});
it("invalidates the versions cache on success", async () => {
mockCreatePolicyVersion.mockResolvedValue({ policy_id: "v3" });
const invalidateSpy = vi.spyOn(queryClient, "invalidateQueries");
const { result } = renderHook(
() => useCreatePolicyVersion("my-policy"),
{ wrapper }
);
await result.current.mutateAsync();
expect(invalidateSpy).toHaveBeenCalledWith({
queryKey: ["policyVersions", "detail", "my-policy"],
});
});
it("shows error notification on failure", async () => {
mockCreatePolicyVersion.mockRejectedValue(new Error("Server error"));
@ -249,6 +265,22 @@ describe("useUpdatePolicyVersionStatus", () => {
);
});
it("invalidates the versions cache on success", async () => {
mockUpdatePolicyVersionStatus.mockResolvedValue({ policy_id: "v2" });
const invalidateSpy = vi.spyOn(queryClient, "invalidateQueries");
const { result } = renderHook(
() => useUpdatePolicyVersionStatus("my-policy"),
{ wrapper }
);
await result.current.mutateAsync({ policyId: "v2", status: "published" });
expect(invalidateSpy).toHaveBeenCalledWith({
queryKey: ["policyVersions", "detail", "my-policy"],
});
});
it("promotes to production and shows success notification", async () => {
const updatedPolicy = { policy_id: "v2", version_status: "production" };
mockUpdatePolicyVersionStatus.mockResolvedValue(updatedPolicy);

View file

@ -21,6 +21,13 @@ export interface PolicyVersionsResponse {
total_count: number;
}
/** Output type after `select` normalizes the response — versions is always defined. */
export interface PolicyVersionsData {
policy_name: string;
versions: Policy[];
total_count: number;
}
// ── Fetch function ──────────────────────────────────────────────────────────
const fetchPolicyVersions = async (
@ -43,8 +50,8 @@ export const usePolicyVersions = ({
}: UsePolicyVersionsOptions) => {
const { accessToken } = useAuthorized();
return useQuery<PolicyVersionsResponse>({
queryKey: policyVersionKeys.detail(policyName ?? ""),
return useQuery<PolicyVersionsResponse, Error, PolicyVersionsData>({
queryKey: policyVersionKeys.detail(policyName!),
queryFn: async () => await fetchPolicyVersions(accessToken!, policyName!),
enabled: Boolean(accessToken && policyName && enabled),
select: (data) => ({

View file

@ -1310,7 +1310,7 @@ export const FlowBuilderPage: React.FC<FlowBuilderPageProps> = ({
policyName: editingPolicy?.policy_name,
enabled: showVersionsSidebar,
});
const versions = versionsData?.versions ?? [];
const versions = versionsData?.versions ?? []; // versionsData?.versions is Policy[] after select, fallback covers undefined data
const createVersionMutation = useCreatePolicyVersion(editingPolicy?.policy_name);
const updateStatusMutation = useUpdatePolicyVersionStatus(editingPolicy?.policy_name);