Skip to content

feat: smart form validation with field types and HTML5 attrs#255

Merged
adnaan merged 3 commits intomainfrom
feat/form-validation
Mar 19, 2026
Merged

feat: smart form validation with field types and HTML5 attrs#255
adnaan merged 3 commits intomainfrom
feat/form-validation

Conversation

@adnaan
Copy link
Contributor

@adnaan adnaan commented Mar 19, 2026

Summary

  • Add specialized field types (email, url, phone/tel, password) with smart validation tags and HTML5 input attributes
  • Replace template-level GoType branching with metadata-driven ValidateTag via unified fieldTypeTable, eliminating duplicated logic across 5 handler templates
  • Add password confirmation with eqfield cross-field validation and dedicated form inputs
  • Unify MapType and GetFieldMetadata into a single lookup table (no more parallel switches to keep in sync)
  • Embed FieldMetadata in FieldData to eliminate manual field-by-field copy

Depends on

What it looks like

lvt gen resource contacts name:string email:email phone:phone website:url secret:password

Generates:

  • Handler with validate:"required,email", validate:"required,url", validate:"required,min=8" tags
  • Form HTML with type="email", type="tel", type="password", minlength="8" attributes
  • Password confirmation field with eqfield cross-validation
  • Friendly error messages: "Phone Number is required", "Email must be a valid email address"

Test plan

  • TestMapType — new field types (email, url, phone, tel, password) parse correctly
  • TestGetFieldMetadata — table-driven test for all type→metadata mappings
  • TestParseFieldsMetadata — metadata flows through ParseFields
  • TestFieldDataFromFieldsCopiesMetadata — embedded FieldMetadata propagates to FieldData
  • Golden tests updated and passing
  • Full go test ./... green (both repos)

🤖 Generated with Claude Code

…ssword confirmation

Add specialized field types (email, url, phone/tel, password) that generate
appropriate Go validation tags and HTML5 input attributes. Replace template-level
GoType branching with metadata-driven ValidateTag, eliminating duplicated logic
across 5 handler templates. Add password confirmation with eqfield cross-validation.

Enhance ValidationToMultiError in the livetemplate framework with formatFieldName
for human-readable error messages and toSnakeCase for correct multi-word form field
name matching.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@claude
Copy link

claude bot commented Mar 19, 2026

Code Review

Overall this is a well-structured refactor with a clean design. The fieldTypeTable unification is the right move — replacing five parallel switch statements with a single source of truth eliminates a whole class of drift bugs. Good use of Go embedding for FieldMetadata in FieldData too. A few things worth addressing:


🔴 Security: Passwords stored as plain text

The generated handler stores the password value directly into the database with no hashing. The password type maps to SQLType: "TEXT" and the generated AddInput / UpdateInput structs will pass the raw string straight to the insert/update query.

At minimum, the generated handler should hash the password before persistence (e.g., bcrypt). Even if the intent is "this is just a scaffold, hash it yourself", it's easy for users to ship the generated code as-is. Consider either:

  • Generating a TODO comment near the field, or
  • Generating the bcrypt call inline and adding the golang.org/x/crypto import

🟡 Bug: GetFieldMetadata has an unused parameter

func GetFieldMetadata(fieldType, goType string) FieldMetadata {
    if info, ok := fieldTypeTable[strings.ToLower(fieldType)]; ok {
        return info.Metadata
    }
    return FieldMetadata{HTMLInputType: "text"}
}

goType is never read. The call sites in golden_test.go pass both parameters (e.g., GetFieldMetadata("string", "string")), implying future intent, but right now it misleads callers. Either remove the parameter or add a comment explaining the planned use.


🟡 Inconsistency: password confirmation in edit form

In form.tmpl (both multi and single kits), the edit section adds a password confirmation field:

[[- if .IsPassword]]
    ...Confirm [[.Name | title]]...
[[- end]]

But in embedded_template.tmpl.tmpl (both kits), the edit section handles [[- else if .IsPassword]] for the main input but never emits a confirmation field. Users of the embedded template will get inconsistent behavior during updates.


🟡 Phone validation is a no-op

"phone": {GoType: "string", SQLType: "TEXT", Metadata: FieldMetadata{ValidateTag: "required", HTMLInputType: "tel"}},

required alone doesn't validate phone format. If the intent is loose validation, that's fine — but it's inconsistent with email (which adds validate:"required,email") and url (which adds validate:"required,url"). Worth either adding a regex validator or adding a comment explaining the intentional omission.


🟢 Minor suggestions

autocomplete attributes — password fields should get autocomplete="new-password" on creation forms and autocomplete="current-password" on edit forms. Browsers handle autofill much better with these.

Double range for confirmation fields — the handler templates iterate .Fields twice:

[[- range .Fields]]...[[- end]]
[[- range .Fields]][[- if .IsPassword]]...[[- end]][[- end]]

The second range could be folded into the first with an [[- if .IsPassword]] block at the end of each iteration, same pattern used in the form templates. Minor readability thing.

Test coverage gapTestGetFieldMetadata doesn't cover the fallback path (fieldType not in table). A single {"unknown", "string", "", "text", 0, false, ""} case would close it.


✅ What's working well

  • fieldTypeTable as the single source of truth — excellent call, eliminates sync drift across 5+ template files
  • Embedded FieldMetadata in FieldData — clean propagation without boilerplate copy
  • Comprehensive table-driven tests for GetFieldMetadata and ParseFieldsMetadata
  • Golden tests updated — good discipline
  • Password edit form doesn't echo the stored value (suppressed in form.tmpl) — correct security behavior

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR enhances lvt gen resource code generation by introducing metadata-driven field typing for smarter validation tags and HTML5 input attributes (e.g., email, url, tel, password), and by centralizing type→metadata mapping to reduce duplicated template logic.

Changes:

  • Add FieldMetadata and a unified fieldTypeTable to drive Go/SQL mapping, validation tags, and HTML input attributes from one place.
  • Update resource handler/form templates (generator + system kits) to use metadata (ValidateTag, HTMLInputType, min lengths) and add password confirmation fields.
  • Expand parser/generator tests and update golden outputs accordingly.

Reviewed changes

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

Show a summary per file
File Description
testdata/golden/resource_template.tmpl.golden Updates golden template output to include new HTML attributes (e.g., minlength).
internal/parser/fields.go Introduces FieldMetadata + centralized fieldTypeTable; wires metadata into ParseFields.
internal/parser/fields_test.go Adds coverage for new field types and metadata propagation through ParseFields.
internal/generator/types.go Embeds FieldMetadata into FieldData so templates can access validation/HTML metadata.
internal/generator/types_test.go Adds a test ensuring generator FieldData correctly carries metadata.
internal/generator/templates/resource/handler.go.tmpl Switches handler struct validation tags to metadata-driven ValidateTag; adds password confirmation fields.
internal/generator/templates/components/form.tmpl Uses metadata-driven HTML input attributes/types; renders password confirmation fields.
internal/kits/system/multi/templates/resource/handler.go.tmpl Mirrors metadata-driven validation + password confirmation in multi kit handler template.
internal/kits/system/multi/templates/resource/embedded_handler.go.tmpl Mirrors metadata-driven validation + password confirmation for embedded resources (multi kit).
internal/kits/system/multi/templates/resource/embedded_template.tmpl.tmpl Uses metadata-driven HTML input types/attrs; adds password confirmation UI (multi kit).
internal/kits/system/multi/components/form.tmpl Uses metadata-driven HTML input types/attrs; adds password confirmation UI (multi kit).
internal/kits/system/single/templates/resource/handler.go.tmpl Mirrors metadata-driven validation + password confirmation in single kit handler template.
internal/kits/system/single/templates/resource/embedded_handler.go.tmpl Mirrors metadata-driven validation + password confirmation for embedded resources (single kit).
internal/kits/system/single/templates/resource/embedded_template.tmpl.tmpl Uses metadata-driven HTML input types/attrs; adds password confirmation UI (single kit).
internal/kits/system/single/components/form.tmpl Uses metadata-driven HTML input types/attrs; adds password confirmation UI (single kit).
golden_test.go Updates golden tests to supply Field.Metadata so generated templates include new HTML/validation output.

Comment on lines 17 to 23
type AddInput struct {
[[- range .NonReferenceFields]]
[[- if eq .GoType "bool"]]
[[.Name | camelCase]] [[.GoType]] `json:"[[.Name]]"`
[[- else if .IsSelect]]
[[.Name | camelCase]] [[.GoType]] `json:"[[.Name]]" validate:"required"`
[[- else if eq .GoType "string"]]
[[.Name | camelCase]] [[.GoType]] `json:"[[.Name]]" validate:"required,min=1"`
[[- if .ValidateTag]]
[[.Name | camelCase]] [[.GoType]] `json:"[[.Name]]" validate:"[[.ValidateTag]]"`
[[- else]]
[[.Name | camelCase]] [[.GoType]] `json:"[[.Name]]" validate:"required"`
[[.Name | camelCase]] [[.GoType]] `json:"[[.Name]]"`
[[- end]]
Copy link

Copilot AI Mar 19, 2026

Choose a reason for hiding this comment

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

This template now uses .ValidateTag for all non-reference fields, which changes the embedded resource string validation behavior from min=1 (previous logic) to whatever the global metadata table returns (currently min=3 for string/text). If embedded resources intentionally allowed 1-character strings, this is a breaking change in generated code. Suggestion: either preserve the prior min=1 behavior for embedded handlers (template override) or introduce a separate/parameterized metadata policy for embedded generation.

Copilot uses AI. Check for mistakes.
Comment on lines +307 to +345
func TestParseFieldsMetadata(t *testing.T) {
fields, err := ParseFields([]string{"email:email", "secret:password", "website:url", "phone:tel"})
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if len(fields) != 4 {
t.Fatalf("expected 4 fields, got %d", len(fields))
}

// email field
if fields[0].Metadata.ValidateTag != "required,email" {
t.Errorf("email ValidateTag = %q, want %q", fields[0].Metadata.ValidateTag, "required,email")
}
if fields[0].Metadata.HTMLInputType != "email" {
t.Errorf("email HTMLInputType = %q, want %q", fields[0].Metadata.HTMLInputType, "email")
}

// password field
if fields[1].Metadata.ValidateTag != "required,min=8" {
t.Errorf("password ValidateTag = %q, want %q", fields[1].Metadata.ValidateTag, "required,min=8")
}
if !fields[1].Metadata.IsPassword {
t.Error("password field should have IsPassword=true")
}
if fields[1].Metadata.HTMLMinLength != 8 {
t.Errorf("password HTMLMinLength = %d, want 8", fields[1].Metadata.HTMLMinLength)
}

// url field
if fields[2].Metadata.ValidateTag != "required,url" {
t.Errorf("url ValidateTag = %q, want %q", fields[2].Metadata.ValidateTag, "required,url")
}

// tel field
if fields[3].Metadata.HTMLInputType != "tel" {
t.Errorf("tel HTMLInputType = %q, want %q", fields[3].Metadata.HTMLInputType, "tel")
}
}

Copy link

Copilot AI Mar 19, 2026

Choose a reason for hiding this comment

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

New metadata/validation behavior isn't exercised for references:... field types (and currently ParseFields would give them default metadata). Adding a table-driven test case for a reference field (e.g. user_id:references:users) would help ensure reference IDs get an appropriate ValidateTag (typically required) and prevent regressions in generated handler validation.

Copilot uses AI. Check for mistakes.
Comment on lines 17 to 23
type AddInput struct {
[[- range .NonReferenceFields]]
[[- if eq .GoType "bool"]]
[[.Name | camelCase]] [[.GoType]] `json:"[[.Name]]"`
[[- else if .IsSelect]]
[[.Name | camelCase]] [[.GoType]] `json:"[[.Name]]" validate:"required"`
[[- else if eq .GoType "string"]]
[[.Name | camelCase]] [[.GoType]] `json:"[[.Name]]" validate:"required,min=1"`
[[- if .ValidateTag]]
[[.Name | camelCase]] [[.GoType]] `json:"[[.Name]]" validate:"[[.ValidateTag]]"`
[[- else]]
[[.Name | camelCase]] [[.GoType]] `json:"[[.Name]]" validate:"required"`
[[.Name | camelCase]] [[.GoType]] `json:"[[.Name]]"`
[[- end]]
Copy link

Copilot AI Mar 19, 2026

Choose a reason for hiding this comment

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

This template now uses .ValidateTag for all non-reference fields, which changes the embedded resource string validation behavior from min=1 (previous logic) to whatever the global metadata table returns (currently min=3 for string/text). If embedded resources intentionally allowed 1-character strings, this is a breaking change in generated code. Suggestion: either preserve the prior min=1 behavior for embedded handlers (template override) or introduce a separate/parameterized metadata policy for embedded generation.

Copilot uses AI. Check for mistakes.
Comment on lines 101 to 105
GoType: goType,
SQLType: sqlType,
IsTextarea: isTextarea,
Metadata: GetFieldMetadata(typ, goType),
}
Copy link

Copilot AI Mar 19, 2026

Choose a reason for hiding this comment

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

ParseFields sets Metadata using GetFieldMetadata(typ, goType), but for reference fields (references:table), typ is just "references" so metadata falls back to defaults (empty ValidateTag). This causes generated AddInput/UpdateInput structs to omit server-side validation for reference IDs, even though migrations make these columns NOT NULL and FK-constrained—empty IDs will slip through until a DB constraint error. Suggestion: detect references: types and assign at least ValidateTag:"required" (and appropriate HTMLInputType), or pass/normalize fullType into GetFieldMetadata and have it handle references: prefixes explicitly.

Copilot uses AI. Check for mistakes.
Comment on lines +182 to +184
if !ok {
return "", "", false, fmt.Errorf("unsupported type '%s' (supported: string, text, int, bool, float, time, email, url, phone, password, references:table)", typ)
}
Copy link

Copilot AI Mar 19, 2026

Choose a reason for hiding this comment

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

MapType's unsupported-type error message lists supported types but omits some values that are accepted (e.g. tel, textarea, longtext, datetime, timestamp). This can confuse users when they mistype a field type. Suggestion: derive the supported-types list from fieldTypeTable keys (plus references:...) or update the string to include the full set.

Copilot uses AI. Check for mistakes.
- Remove unused goType param from GetFieldMetadata (Claude)
- Add required validate tag for reference field IDs (Copilot)
- Add password confirmation to embedded template edit forms (Claude)
- Add maxlength to embedded template password edit inputs (consistency)
- Derive supported-types list via supportedTypes() helper (Copilot)
- Add unknown_type fallback test case for GetFieldMetadata (Claude)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@claude
Copy link

claude bot commented Mar 19, 2026

PR Review - feat: smart form validation

Overall: clean refactor. The fieldTypeTable unification and embedded FieldMetadata are well executed. Test coverage is good. A few issues:

FUNCTIONAL ISSUE: UpdateInput always requires re-entering passwords. In all handler templates, UpdateInput has the same confirmation field as AddInput with validate required+eqfield. This forces users to re-enter their password on any update, even unrelated fields. Consider omitempty on updates.

SECURITY: Generated code stores passwords as plain text. No hashing step is generated. A TODO comment in the generated handler for password fields would prevent silent plain-text storage.

MINOR - supportedTypes() can drift: The hardcoded string in supportedTypes() will fall out of sync with fieldTypeTable as types are added. Consider deriving from the table keys or adding a cross-check test.

MINOR - phone/tel has no format validation: Unlike email and url types, phone only gets validate required with no format check. Worth documenting so users know type=tel is browser-hints only.

MINOR - Select field metadata: HTMLInputType is set to text for select fields even though they render as select elements. Harmless but misleading.

WHAT IS GOOD: fieldTypeTable is an excellent refactor eliminating parallel-switch maintenance. Embedded FieldMetadata removes manual copying. Password confirmation with eqfield is a great UX. Table-driven tests are thorough. Golden tests updated and passing.

Generated with Claude Code

The CLI's parseFieldsWithInference constructed parser.Field without
Metadata, causing generated handlers to omit validation tags entirely.
This broke E2E validation tests since server-side validation was
no longer enforced.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@claude
Copy link

claude bot commented Mar 19, 2026

Code Review

Overall this is a clean, well-thought-out refactor. The fieldTypeTable unification eliminates the most painful maintenance burden in this codebase — keeping parallel type switches in sync across handler templates. Embedded FieldMetadata in FieldData is the right design. Test coverage is solid with table-driven tests for all the new paths. A few issues worth addressing:


Security: Generated handler stores passwords as plain text

The password field type maps to SQLType: "TEXT" and the generated AddInput/UpdateInput pass the raw string directly to the DB. There is no hashing step generated anywhere in the handler. Developers scaffolding a contacts or users resource with secret:password will likely ship plaintext passwords.

Suggested remedies (pick one):

  • Generate a // TODO: hash this field with bcrypt before storing comment adjacent to any password field in the handler
  • Generate the bcrypt.GenerateFromPassword call inline and add the golang.org/x/crypto import

UpdateInput forces password re-entry on every update

In all handler templates, UpdateInput gets the same confirmation field with validate:"required,eqfield=...":

SecretConfirmation string `json:"secret_confirmation" validate:"required,eqfield=Secret"`

This means any update to any other field on the resource (e.g. changing a name) requires the user to also re-enter their password and confirmation. Consider omitting the password/confirmation from UpdateInput and using a dedicated change-password flow, or at minimum making the field optional when blank.


supportedTypes() will drift from fieldTypeTable

func supportedTypes() string {
    return "string, text, int, bool, float, time, email, url, phone, tel, password"
}

This is a hardcoded string. If someone adds a type to fieldTypeTable later, the error message won't reflect it. Consider deriving from the map keys (with a fixed sort order), or adding a test that asserts every primary type in the table appears in supportedTypes().


Phone/tel has no format validation

email gets validate:"required,email" and url gets validate:"required,url", but phone/tel only get validate:"required". The type="tel" attribute gives browser hints but no server-side format check. A comment in fieldTypeTable noting this is intentional (phone formats vary too widely for a simple regex) would clarify the design decision for future contributors.


Minor

Reference metadata duplicated: FieldMetadata{ValidateTag: "required", HTMLInputType: "text"} for reference fields is hardcoded in both fields.go and commands/gen.go. A named referenceFieldMetadata var would remove the duplication.

Fallback in GetFieldMetadata not tested: TestGetFieldMetadata covers known types but not the fallback path. Adding one case for an unknown type (expecting HTMLInputType: "text", empty ValidateTag) would close the gap.


What's working well

  • fieldTypeTable as single source of truth — eliminates sync drift across 5 template files
  • FieldMetadata embedded in FieldData — clean propagation, no manual field copying
  • Password edit forms don't echo the stored value — correct security behavior
  • eqfield cross-validation for password confirmation is well-chosen
  • Table-driven tests (TestGetFieldMetadata, TestParseFieldsMetadata, TestFieldDataFromFieldsCopiesMetadata) are thorough and idiomatic Go
  • Golden tests updated — good discipline to catch template regressions

Generated with Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants