Repository navigation
feat(portfolio): shared validation rules for portfolios and usernames - #55
Conversation
Define the student-portfolio rules once in shared/portfolio/rules.json: field limits, https-only link and host patterns, email pattern, and the username format with reserved names and blocked words. Add fixtures.json with 144 username and portfolio cases that both validators must agree on, and a README that documents the limits and how to change a rule. Refs #42
Validate untrusted portfolio data and usernames against the shared rules, reporting each problem as a path and a stable code. Drafts only need to be well-formed; publishing also requires a name and headline. Links are checked without a URL parser, and only A-Z is lower-cased, so the result matches the .NET validator exactly. Run all shared fixtures in the unit tests. Refs #42
Implement the same portfolio and username rules in the backend, read from the shared rules file copied into the build output, and run the shared fixtures in the test project. Anchor patterns with \z because .NET's $ also matches before a trailing newline, and fail loudly when a rule key is missing instead of skipping the check. Refs #42
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
procoder-ji
left a comment
procoder-ji
left a comment
There was a problem hiding this comment.
Reviewed the TypeScript and .NET validators, shared rules/fixtures, build inclusion and issue #42. Two reproducible input-validation defects need fixes: oversized blank fields bypass the declared limits, and malformed UTF-16 is accepted by TypeScript but throws in .NET. Details and minimal repros are inline.
Verification: 9 focused TypeScript tests and 8 focused .NET tests pass, and all three CI jobs are green on this commit. Additional probes expose the two missing cases. No changes were made to the PR branch.
Check the length before the blank test, so a field of only spaces can no longer exceed its limit. Previously an 1001-space about, a 301-space website or a 255-space email passed validation. Treat a lone surrogate half as invalid-characters in values, links, emails, usernames and field names. System.Text.Json decodes strings and property names lazily and throws when they are first read, so the .NET validator now reads them without throwing and reports the same problem as the TypeScript validator, which previously accepted them. Add shared fixtures for both cases. The malformed-text cases list UTF-16 code units because a lone surrogate escape is not portable JSON. Refs #42
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
procoder-ji
left a comment
procoder-ji
left a comment
There was a problem hiding this comment.
Thanks @Coding-Professional for addressing both findings and adding shared regression coverage.
Re-reviewed a284af6: oversized blank text, links and email now respect their limits; malformed UTF-16 values and property names return consistent validation problems without throwing. Valid surrogate pairs still pass.
Verified the original repros and ran 11 focused TypeScript tests and 13 focused .NET tests, all passing. All three CI jobs are green on this commit. No remaining blocking findings.
Summary
Closes #42. Part of #40. Follows ADR 0002.
One source of rules for student portfolios, enforced by two validators that run against the same fixtures, the way
shared/practice/questions.jsonis shared today.shared/portfolio/rules.json: limits, reserved names, blocked words, and the patterns for links, hosts and email.shared/portfolio/fixtures.json: 38 username, 116 portfolio and 17 malformed-text cases, each with the problems it must produce (path:code, for exampleprojects[0].link:invalid-link).src/features/portfolio/: TypeScript validator (validatePortfolio,validateUsername) and types.backend/src/ProCoderJi.Api/Features/Portfolio/: .NET validator and rules loader.rules.jsonis copied into the build output like the question bank.shared/portfolio/README.md: the limits (generated fromrules.json), the problem codes, and how to change a rule.Nothing calls the validators yet; the API (#43) and the editor (#45) will.
How it behaves
nameandheadline.https, with no credentials, port, IP address orlocalhost.links.githubmust be ongithub.comandlinks.linkedinonlinkedin.com(subdomains allowed), so a field labelled GitHub cannot point at a lookalike site.Numbers to review
nameskillsheadlineprojectsaboutprojects.technologieseducationexperienceachievementsThese are starting points taken from what fits a phone-sized card. They live only in
rules.json, so changing one is a one-line edit plus a fixture.Decisions for the reviewer
mailto:link. ADR 0002 says links allowmailto:for the email field; storing the bare address is simpler for students and the page builds the link. Say if you would rather storemailto:.themefield yet. The page issue (Portfolio: public page at /u/[username] with two themes #44) decides how a student picks a theme and can add it with a rules version bump.bank-guideis refused as well aspaytm-help. It is cheap to loosen later and costly to discover a phishing name after the fact.x<yis refused as HTML-like;x < yis fine. The Markdown import (Portfolio: import details from a Markdown file #46) will need to explain this to students writing C++ or maths.Keeping the two runtimes identical
The validators avoid URL parsers and culture-aware string functions, because JavaScript and .NET disagree about them. Two divergences were found and are covered by fixtures:
$also matches before a trailing newline, soriya\nwould pass a pattern that JavaScript rejects. The .NET validator anchors with\z.k. Both validators lower-case A-Z only.I mutated each validator at these points and at the host-suffix, reserved-name, hyphen and item-limit checks to confirm the fixtures fail when a rule breaks.
Checks
npm run lintnpm run typechecknpm test(159 pass, 2 skipped; 11 new)npm run builddotnet test backend/ProCoderJi.slnx --configuration Release(34 pass; 13 new)