fix(report): improve form validation and user feedback (Closes #607, #608) #816

timothyfroehlich · 2026-01-20T00:00:56Z

This PR improves the 'Report Issue' form by ensuring validation errors are displayed without a full page reload and forcing users to make explicit selections for required fields.

Changes:

UX: now returns error states directly to , allowing for instant feedback without redirecting.
Validation: Default values for Severity, Priority, and Consistency are removed (set to ), forcing users to choose a value.
UI: , , and now support placeholders and empty states.
Testing: Added to verify Zod validation for required fields.

Fixes #607, #608

- Add owner display field for non-admin users in UpdateMachineForm - Keep editable owner select for admin users only - Show owner name with invited status indicator when applicable - Display "No owner assigned" message when no owner is set Co-authored-by: timothyfroehlich <5819722+timothyfroehlich@users.noreply.github.com>

- Add test to verify machine owner is visible to non-admin users - Verify owner name and help text are displayed correctly Co-authored-by: timothyfroehlich <5819722+timothyfroehlich@users.noreply.github.com>

- Ensure (Invited) badge only shows when owner is truly in invited state - Prevents showing badge when invited owner has accepted invitation Co-authored-by: timothyfroehlich <5819722+timothyfroehlich@users.noreply.github.com>

Co-authored-by: timothyfroehlich <5819722+timothyfroehlich@users.noreply.github.com>

…owners-to-users

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Co-authored-by: timothyfroehlich <5819722+timothyfroehlich@users.noreply.github.com>

- Return error states from submitPublicIssueAction instead of redirecting - Remove default values for severity, priority, and consistency to force selection - Add placeholders and handle empty states in Select components - Add unit tests for form validation (Fixes #607, #608)

vercel · 2026-01-20T00:01:00Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Review	Updated (UTC)
pin-point	Ready	Preview, Comment	Jan 20, 2026 0:01am

Copilot

Pull request overview

This PR improves form validation and user feedback for the Report Issue form by forcing explicit field selections and returning validation errors without page redirects. The changes enable instant client-side feedback when users fail to select required values for Severity, Priority, and Consistency fields.

Changes:

Modified form state to use empty string ("") as default instead of preset values, forcing users to make explicit selections
Updated Server Action to return error states directly rather than redirecting with URL parameters
Added placeholder support and conditional rendering to three select components (Severity, Priority, Consistency)
Added validation unit tests to verify required field enforcement
Included unrelated machine owner display improvements (scope creep)

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
`src/app/report/actions.ts`	Removed redirect-based error handling, now returns error objects directly for in-place display
`src/app/report/validation.test.ts`	New test file validating required field enforcement for machineId, severity, title, and consistency
`src/app/report/unified-report-form.tsx`	Changed state initialization from preset values to empty strings; removed button disabled state
`src/components/issues/fields/SeveritySelect.tsx`	Added support for empty string value and placeholder, with conditional icon/label rendering
`src/components/issues/fields/PrioritySelect.tsx`	Added support for empty string value and placeholder, with conditional icon/label rendering
`src/components/issues/fields/ConsistencySelect.tsx`	Added support for empty string value and placeholder, with conditional icon/label rendering
`src/app/(app)/m/[initials]/update-machine-form.tsx`	Added owner/invitedOwner properties and read-only display for non-admin users (unrelated)
`e2e/smoke/machines-crud.spec.ts`	Added E2E test for machine owner display functionality (unrelated)

Copilot · 2026-01-20T00:05:42Z

src/components/issues/fields/SeveritySelect.tsx

+  value: IssueSeverity | "";
  onValueChange: (value: IssueSeverity) => void;


Type mismatch between value and onValueChange. The value prop accepts IssueSeverity | "" but onValueChange only accepts IssueSeverity. This creates a type inconsistency where the component accepts an empty string as a valid value, but cannot properly handle that case in the callback. Consider updating onValueChange to accept (value: IssueSeverity | "") => void and handle the empty string case appropriately, or use a type assertion to narrow the value before passing to onValueChange.

Copilot · 2026-01-20T00:05:42Z

src/components/issues/fields/PrioritySelect.tsx

 interface PrioritySelectProps {
-  value: IssuePriority;
+  value: IssuePriority | "";
  onValueChange: (value: IssuePriority) => void;


Type mismatch between value and onValueChange. The value prop accepts IssuePriority | "" but onValueChange only accepts IssuePriority. This creates a type inconsistency where the component accepts an empty string as a valid value, but cannot properly handle that case in the callback. Consider updating onValueChange to accept (value: IssuePriority | "") => void and handle the empty string case appropriately, or use a type assertion to narrow the value before passing to onValueChange.

Suggested change

onValueChange: (value: IssuePriority) => void;

onValueChange: (value: IssuePriority | "") => void;

Copilot · 2026-01-20T00:05:42Z

src/components/issues/fields/ConsistencySelect.tsx

 interface ConsistencySelectProps {
-  value: IssueConsistency;
+  value: IssueConsistency | "";
  onValueChange: (value: IssueConsistency) => void;


Type mismatch between value and onValueChange. The value prop accepts IssueConsistency | "" but onValueChange only accepts IssueConsistency. This creates a type inconsistency where the component accepts an empty string as a valid value, but cannot properly handle that case in the callback. Consider updating onValueChange to accept (value: IssueConsistency | "") => void and handle the empty string case appropriately, or use a type assertion to narrow the value before passing to onValueChange.

Suggested change

onValueChange: (value: IssueConsistency) => void;

onValueChange: (value: IssueConsistency | "") => void;

Copilot · 2026-01-20T00:05:43Z

src/app/(app)/m/[initials]/update-machine-form.tsx

+    owner?: {
+      id: string;
+      name: string;
+      avatarUrl?: string | null;
+      email?: string;
+    } | null;
+    invitedOwner?: {
+      id: string;
+      name: string;
+      email?: string;
+    } | null;


This change appears unrelated to the PR's stated purpose of improving form validation and user feedback for the Report Issue form. The PR description mentions fixes for #607 and #608, which concern the report form, but this change adds owner and invitedOwner object properties to the machine interface. Consider moving these machine owner display improvements to a separate PR focused on machine management features.

Suggested change

owner?: {

id: string;

name: string;

avatarUrl?: string | null;

email?: string;

} | null;

invitedOwner?: {

id: string;

name: string;

email?: string;

} | null;

Copilot · 2026-01-20T00:05:43Z

src/app/(app)/m/[initials]/update-machine-form.tsx

+      {/* Machine Owner */}
+      {isAdmin ? (
        <OwnerSelect
          users={allUsers}
          defaultValue={machine.ownerId ?? machine.invitedOwnerId ?? null}
        />
+      ) : (
+        <div className="space-y-2" data-testid="owner-display">
+          <span className="text-sm font-semibold text-on-surface">
+            Machine Owner
+          </span>
+          <div className="rounded-md border border-outline bg-surface px-3 py-2">
+            {machine.owner || machine.invitedOwner ? (
+              <div className="flex items-center gap-2">
+                <span className="text-sm text-on-surface">
+                  {machine.owner?.name ?? machine.invitedOwner?.name}
+                </span>
+                {/* Show invited badge only for truly invited owners (not yet accepted) */}
+                {machine.invitedOwner && !machine.owner && (
+                  <span className="text-[10px] font-medium uppercase tracking-wider text-on-surface-variant/70">
+                    (Invited)
+                  </span>
+                )}
+              </div>
+            ) : (
+              <span className="text-sm text-on-surface-variant">
+                No owner assigned
+              </span>
+            )}
+          </div>
+          <p className="text-xs text-on-surface-variant">
+            The owner receives notifications for new issues on this machine.
+          </p>
+        </div>


This change appears unrelated to the PR's stated purpose of improving form validation and user feedback for the Report Issue form. The PR description mentions fixes for #607 and #608, which concern the report form, but this adds a read-only owner display section for non-admin users. Consider moving these machine owner display improvements to a separate PR focused on machine management features.

Suggested change

{/* Machine Owner */}

{isAdmin ? (

<OwnerSelect

users={allUsers}

defaultValue={machine.ownerId ?? machine.invitedOwnerId ?? null}

/>

) : (

<div className="space-y-2" data-testid="owner-display">

<span className="text-sm font-semibold text-on-surface">

Machine Owner

</span>

<div className="rounded-md border border-outline bg-surface px-3 py-2">

{machine.owner || machine.invitedOwner ? (

<div className="flex items-center gap-2">

<span className="text-sm text-on-surface">

{machine.owner?.name ?? machine.invitedOwner?.name}

</span>

{/* Show invited badge only for truly invited owners (not yet accepted) */}

{machine.invitedOwner && !machine.owner && (

<span className="text-[10px] font-medium uppercase tracking-wider text-on-surface-variant/70">

(Invited)

</span>

)}

</div>

) : (

<span className="text-sm text-on-surface-variant">

No owner assigned

</span>

)}

</div>

<p className="text-xs text-on-surface-variant">

The owner receives notifications for new issues on this machine.

</p>

</div>

{/* Machine Owner (admins only) */}

{isAdmin && (

<OwnerSelect

users={allUsers}

defaultValue={machine.ownerId ?? machine.invitedOwnerId ?? null}

/>

Copilot · 2026-01-20T00:05:43Z

e2e/smoke/machines-crud.spec.ts

+  test("should display machine owner to all logged-in users", async ({
+    page,
+  }) => {
+    // Navigate to a machine detail page
+    await page.goto(`/m/${seededMachines.medievalMadness.initials}`);
+
+    // Machine Information card should be visible
+    await expect(
+      page.getByRole("heading", { name: "Machine Information" })
+    ).toBeVisible();
+
+    // As a member (default login), owner should be displayed but read-only
+    const ownerDisplay = page.getByTestId("owner-display");
+    await expect(ownerDisplay).toBeVisible();
+
+    // Verify owner label is present
+    await expect(page.getByText("Machine Owner")).toBeVisible();
+
+    // Verify owner name is shown (Admin User owns all seeded machines)
+    await expect(page.getByText("Admin User")).toBeVisible();
+
+    // Verify the help text is shown
+    await expect(
+      page.getByText(
+        "The owner receives notifications for new issues on this machine."
+      )
+    ).toBeVisible();
+  });


This E2E test appears unrelated to the PR's stated purpose of improving form validation and user feedback for the Report Issue form. The PR description mentions fixes for #607 and #608, which concern the report form, but this test validates machine owner display functionality. Consider moving this test to a separate PR focused on machine owner display features.

Copilot · 2026-01-20T00:05:43Z

src/app/report/validation.test.ts

+      // Zod's default error for undefined/missing field when it expects a uuid string
+      expect(result.error).toContain(
+        "Invalid input: expected string, received undefined"
+      );


The test expects a specific Zod error message format ("Invalid input: expected string, received undefined"). This is brittle as it couples the test to Zod's internal error message format. If Zod updates its error messages in a future version, this test would fail even though the validation logic is correct. Consider checking for the field name and validation failure only, without asserting on the exact error message text.

Suggested change

// Zod's default error for undefined/missing field when it expects a uuid string

expect(result.error).toContain(

"Invalid input: expected string, received undefined"

);

Copilot · 2026-01-20T00:05:44Z

src/app/report/validation.test.ts

+  it("should pass validation when all required fields are present", () => {
+    const formData = new FormData();
+    formData.set("machineId", "00000000-0000-0000-0000-000000000000");
+    formData.set("title", "Valid Title");
+    formData.set("severity", "minor");
+    formData.set("consistency", "intermittent");
+
+    const result = parsePublicIssueForm(formData);
+
+    expect(result.success).toBe(true);
+  });
+});


Missing test coverage for empty string submission. When users don't select a value (value is ""), the hidden input will submit an empty string to the server. While the Zod validation should catch this (enum doesn't accept empty string), there's no test case verifying this behavior. Add a test that sets severity or consistency to an empty string and verifies the validation fails appropriately.

Copilot AI and others added 10 commits January 12, 2026 03:44

Initial plan

f82ebdb

test: Add E2E test for machine owner display

e7460a6

- Add test to verify machine owner is visible to non-admin users - Verify owner name and help text are displayed correctly Co-authored-by: timothyfroehlich <5819722+timothyfroehlich@users.noreply.github.com>

docs: Add clarifying comment for invited badge logic

db2047b

Co-authored-by: timothyfroehlich <5819722+timothyfroehlich@users.noreply.github.com>

Merge remote-tracking branch 'origin/main' into copilot/show-machine-…

7cc100f

…owners-to-users

Update e2e/smoke/machines-crud.spec.ts

5a11c0b

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update src/app/(app)/m/[initials]/update-machine-form.tsx

22e19ff

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

style: Fix prettier formatting for owner display span

f4fcffc

Co-authored-by: timothyfroehlich <5819722+timothyfroehlich@users.noreply.github.com>

Copilot AI review requested due to automatic review settings January 20, 2026 00:00

Copilot started reviewing on behalf of timothyfroehlich January 20, 2026 00:01 View session

Copilot AI reviewed Jan 20, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(report): improve form validation and user feedback (Closes #607, #608) #816

fix(report): improve form validation and user feedback (Closes #607, #608) #816

Uh oh!

timothyfroehlich commented Jan 20, 2026

Uh oh!

vercel bot commented Jan 20, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 20, 2026

Uh oh!

Copilot AI Jan 20, 2026

Uh oh!

Copilot AI Jan 20, 2026

Uh oh!

Copilot AI Jan 20, 2026

Uh oh!

Copilot AI Jan 20, 2026

Uh oh!

Copilot AI Jan 20, 2026

Uh oh!

Copilot AI Jan 20, 2026

Uh oh!

Copilot AI Jan 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		value: IssueSeverity \| "";
		onValueChange: (value: IssueSeverity) => void;

	onValueChange: (value: IssuePriority) => void;
	onValueChange: (value: IssuePriority \| "") => void;

	onValueChange: (value: IssueConsistency) => void;
	onValueChange: (value: IssueConsistency \| "") => void;

fix(report): improve form validation and user feedback (Closes #607, #608) #816

Are you sure you want to change the base?

fix(report): improve form validation and user feedback (Closes #607, #608) #816

Uh oh!

Conversation

timothyfroehlich commented Jan 20, 2026

Changes:

Uh oh!

vercel bot commented Jan 20, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants