feat(profile): surface remaining hideable fields in the edit form#64
Merged
Conversation
Completes the per-field visibility story end-to-end. The backend (PR #59) has supported these five fields all along; the edit form just had no inputs for them. Users can now actually enter — and choose audience for — every hideable field. New inputs in profile/index.astro: - Years of experience: <input type="number" min="0" max="60">, slotted as a half-width row matching the Location/Timezone grid rhythm - Languages I speak: chip input mirroring Skills/Interests, optional (zero entries elides the field from the wire and the stored doc) - Links: <fieldset> with three <input type="url" pattern="https://.*"> inputs (GitHub, LinkedIn, personal website). The pattern is a soft browser hint; the server's sanitizedUrl() is the hard gate that rejects non-https — that's the XSS protection that matters. Five matching visibility selectors added to the existing #field-visibility-section (now 10 selectors total — one per hideable field). The runtime whitelist guard added in #60 already keeps the data-field bindings honest; no further defence needed. Helper refactor (motivated by the new languages chip input): - bindTagInput / renderTags / addTag dropped their `type: 'skill' | 'interest'` literal-union parameter in favour of taking the Set directly. The close-button now mutates whatever Set was passed in, so the chip behaviour is genuinely generic and a fourth/fifth chip input would slot in with no helper changes. Pure refactor: skills + interests behave identically to before. Backend extension (api/src/profile-save.ts): - Pulls in normalizeStringList for preferredLanguages (same caps as skills/interests — 50 chars per item, 30 items max). Empty list stored as undefined so the doc doesn't carry noise. - New boundedInteger(value, min, max) helper in lib/validation: accepts numbers OR numeric strings (the form ships a string from <input type="number">), returns undefined for NaN/Infinity/out-of- range. CRITICAL: uses Number() not parseInt — parseInt('5x', 10) returns 5, which would let "5x" become a stored 5 years of experience. Number('5x') returns NaN, which the isFinite gate rejects. Test pinned for that exact case. Frontend service (src/services/api.ts): - ProfileInput extended with optional yearsOfExperience and preferredLanguages, matching the now-supported wire shape. Tests (+58): - lib/validation.test.ts: 18 cases for boundedInteger covering in-range integers, fractional floor, below/above bounds, NaN/ Infinity, numeric string parsing, empty/whitespace strings, the "5x" parseInt-gotcha (specifically pinned), non-number/non-string rejection, custom bounds - profile-save.test.ts: yearsOfExperience plumbing (4 cases: valid integer, string parsing, empty → undefined, garbage → undefined); preferredLanguages plumbing (4 cases: valid array, per-item trim + empty drop, missing/empty → undefined, non-array → undefined) - profile-edit.test.ts: HIDEABLE_FIELDS_WITH_UI extended from 5 to 10 (every existing parametric test now runs once per new field + its visibility selector); + 8 new assertions for the new input shapes (number input, bounds, chip section, bindTagInput reuse, URL inputs × 3 with type="url" + pattern, pre-population on load, payload inclusion) All 483 tests pass; lint, format, audit (high gate + moderate gate), Astro + API builds clean. https://claude.ai/code/session_018WA3SvALW1EmxJKVmRUGrR
There was a problem hiding this comment.
Pull request overview
This PR completes the profile edit form coverage for all backend-supported hideable fields by adding UI inputs for spoken languages, years of experience, and three link URLs, plus wiring them into per-field visibility controls and save payloads. It also extends the backend validation/saving pipeline to normalize preferredLanguages and to validate yearsOfExperience via a new boundedInteger helper.
Changes:
- Frontend: add edit-form inputs for
preferredLanguages,yearsOfExperience, andgithubUrl/linkedinUrl/websiteUrl, plus corresponding per-field visibility selectors and save/load wiring. - Backend: add
boundedInteger()validation helper and persistyearsOfExperience+ normalizedpreferredLanguagesinprofile-save. - Tests: expand frontend source-assertion tests and backend unit tests to pin the new fields’ behaviors and validation edge cases.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/services/api.ts | Extends ProfileInput (and related profile types) to include preferredLanguages and yearsOfExperience. |
| src/profile-edit.test.ts | Adds source-level tests asserting the presence/wiring of new form inputs and visibility selectors. |
| src/pages/profile/index.astro | Implements new form controls, generic chip-input refactor, and save/load wiring for new fields + visibility selectors. |
| api/src/profile-save.ts | Normalizes and persists preferredLanguages; validates and persists yearsOfExperience using boundedInteger. |
| api/src/profile-save.test.ts | Adds end-to-end handler tests for yearsOfExperience and preferredLanguages plumbing. |
| api/src/lib/validation.ts | Introduces boundedInteger helper for safe integer coercion/bounds checking. |
| api/src/lib/validation.test.ts | Adds focused unit coverage for boundedInteger parsing/coercion/bounds behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+995
to
+1002
| // 0 (which would lie about the user's experience). The server | ||
| // validates the upper bound too; this client-side coerce is a | ||
| // UX nicety, not a security gate. | ||
| const yearsRaw = yearsEl?.value?.trim() ?? ''; | ||
| const yearsParsed = yearsRaw === '' ? undefined : Number.parseInt(yearsRaw, 10); | ||
| const yearsOfExperience = | ||
| typeof yearsParsed === 'number' && Number.isFinite(yearsParsed) && yearsParsed >= 0 | ||
| ? yearsParsed |
Comment on lines
+89
to
+94
| * - we want to reject NaN, Infinity, fractional, and out-of-bound values | ||
| * - "empty / invalid" must become `undefined` (not 0) so the stored | ||
| * document doesn't lie about the user's data | ||
| * | ||
| * Strings are parsed with parseInt(value, 10); pre-existing numbers are | ||
| * floored. The range gate is inclusive on both ends. |
Two Copilot review findings on PR #64: 1. Client/server parsing disagreement (real bug). The server-side boundedInteger was updated in this PR's first commit to use Number() (rejecting "5x" as NaN, defeating parseInt's lenient leading-digit parse). The client coerce was left on parseInt — meaning Number('5e2') → 500 on the server while parseInt('5e2', 10) → 5 on the client. Users typing values the server would reject got no UI feedback (save "succeeded" but the value silently became undefined). Fix: client now uses Number() + Math.floor + the same [0, 60] inclusive bound + isFinite gate as the server. Symmetry pinned by two new regex tests so the next refactor can't drift them apart again. 2. Stale doc comment on boundedInteger. The comment claimed "rejects fractional, parses with parseInt" but the implementation floors fractional values (test pinned) and uses Number() (also test pinned). Updated the doc to match: - Fractional values are FLOORED, not rejected (5.7 → 5) - Uses Number() not parseInt; explains the "5x" → NaN trade-off and notes the scientific-notation case ("5e2" → 500) is intentional, caught by the [min, max] bound No behaviour change beyond the client/server symmetry — server logic is unchanged. Tests: 485 pass (+2 new pinning the client semantics). https://claude.ai/code/session_018WA3SvALW1EmxJKVmRUGrR
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR delivers
Completes the per-field visibility story end-to-end. The backend (PR #59) has supported five additional hideable fields all along; the edit form just had no inputs for them. Users can now actually enter — and choose audience for — every hideable field.
New form inputs:
<input type="number" min="0" max="60">, half-width matching the Location/Timezone grid rhythm<fieldset>with three<input type="url" pattern="https://.*">inputs (GitHub, LinkedIn, personal website). The pattern is a soft browser hint; the server'ssanitizedUrl()is the hard gate that rejects non-https — that's the XSS protection.Five matching visibility selectors added to the existing
#field-visibility-section(now 10 selectors total). The runtime whitelist guard from #60 keeps thedata-fieldbindings honest.Helper refactor (motivated by the languages chip input):
bindTagInput/renderTags/addTagdropped theirtype: 'skill' | 'interest'literal-union parameter in favour of taking the Set directly. The close-button now mutates whatever Set was passed in, so the chip behaviour is genuinely generic and a fourth/fifth chip input would slot in with no helper changes. Pure refactor — skills + interests behave identically to before.Backend extension — new
boundedInteger(value, min, max)helper foryearsOfExperience. Critical detail: usesNumber()notparseInt.parseInt('5x', 10)returns5, which would let"5x"become a stored 5 years of experience.Number('5x')returnsNaN, which theisFinitegate rejects. Test pinned for that exact case.preferredLanguagesreuses the existingnormalizeStringListwith the same caps as skills/interests.Verified by
api/src/lib/validation.test.ts::boundedInteger / accepts in-range integer %j(4 cases)api/src/lib/validation.test.ts::boundedInteger / floors fractional numbers (5.7 → 5)api/src/lib/validation.test.ts::boundedInteger / rejects %j (%s) with undefined(4 out-of-range cases)api/src/lib/validation.test.ts::boundedInteger / rejects NaN/Infinity(3 cases)api/src/lib/validation.test.ts::boundedInteger / parses numeric strings— form input ships stringsapi/src/lib/validation.test.ts::boundedInteger / returns undefined for non-numeric string %j(5 cases including the"5x"parseInt-gotcha)api/src/lib/validation.test.ts::boundedInteger / rejects non-number/non-string input(5 type cases)api/src/lib/validation.test.ts::boundedInteger / respects custom min/max boundsapi/src/profile-save.test.ts::yearsOfExperience plumbing / persists a valid integer years valueapi/src/profile-save.test.ts::yearsOfExperience plumbing / parses numeric strings— pins the form-input contract end-to-endapi/src/profile-save.test.ts::yearsOfExperience plumbing / stores undefined for an empty string— "prefer not to say" preservedapi/src/profile-save.test.ts::yearsOfExperience plumbing / drops out-of-range / non-numeric input %j to undefined (no false zero)(7 cases) — prevents lying-by-coercionapi/src/profile-save.test.ts::preferredLanguages plumbing / persists a valid string arrayapi/src/profile-save.test.ts::preferredLanguages plumbing / trims per-item whitespace and drops emptiesapi/src/profile-save.test.ts::preferredLanguages plumbing / stores undefined (not []) when input is missing or yields zero entriesapi/src/profile-save.test.ts::preferredLanguages plumbing / treats non-array input as zero entries (defensive)src/profile-edit.test.ts::declares a numeric input for years of experiencesrc/profile-edit.test.ts::bounds the years input between 0 and 60src/profile-edit.test.ts::declares a chip-input section for spoken languagessrc/profile-edit.test.ts::reuses the generic bindTagInput helper for languages— anti-duplication invariantsrc/profile-edit.test.ts::declares a type="url" input for #githubUrl/#linkedinUrl/#websiteUrl with https-only pattern(3 cases)src/profile-edit.test.ts::pre-populates each new field from the loaded profile— prevents save-wipes-stored-data class of bugsrc/profile-edit.test.ts::includes the new fields in the saveProfile payloadConstitution compliance
sanitizedUrlenforces https-only on the three URL fields (same XSS-blocking semantic as before; pattern attribute is a UX hint, not a security control);boundedIntegerdefensively returns undefined for"5x"etc. (no lying-by-coercion); URLpatternandmin/maxare documented as UX hints with server-side validation as the security boundary<fieldset>+<legend>semantic introduced in feat(profile): a11y sweep — fieldset/legend on every grouped section #62<label for="…">or lives inside a<fieldset>with a<legend>; native form semantics support keyboard + screen readerPlatform constraints
Operator action items
Test plan
npm run audit(frontend, high gate): clean; API audit: cleanPrivate→ second user views/find/<you>→ those fields are absenthttps://claude.ai/code/session_018WA3SvALW1EmxJKVmRUGrR
Generated by Claude Code