fix: remove yearsExperience field from PoolTeacher + add bug-bash docs#197
Merged
guangshinhaha merged 4 commits intomainfrom May 8, 2026
Merged
Conversation
Codifies a persona-based exercise that catches what unit, API integration, and Playwright suites miss: cross-module handoffs, role/context drift, perception bugs, human-flow edge cases (back/refresh/double-submit), and adversarial input. Prioritises hotspots from recent fix-commits (auto-assign, impersonation, CSV import). Re-runnable per release; appended to as new feature modules ship.
Extends the demo-cookie e2e suite to cover the persona surface the demo
session can reach:
- teacher.spec.ts /teacher/absences (demo admin is also seeded as Teacher)
- pool-views.spec.ts /demo/dashboard/preferred-pool, /rems-pool
- report.spec.ts /demo/report (sick-leave form), /demo/reporting
- multi-tenancy.spec.ts foreign slug + demo cookie -> redirect to /login,
swept across 3 slugs * 6 paths to guard
requireSchoolForSlug / middleware boundary
These are render-only by design (path A); flow tests (submit -> assign ->
notify) and POOL_TEACHER / SUPERADMIN coverage need real-auth fixtures.
Plan for that follow-up landed at docs/testing/path-b-real-auth-e2e-plan.md.
We never collected this data in practice, so the field, the form input, the REMS import column, and the auto-assign criterion that used it were all dead weight. This drops the field end-to-end: - prisma/schema.prisma: remove yearsExperience from PoolTeacher - src/lib/matchingCriteria.ts: drop the "experience" criterion (priority 5); workload moves up to priority 5. MatchCandidate / MatchContext lose the related fields. Existing MatchingConfig rows storing "experience" in their criteriaOrder are silently dropped by normaliseCriteriaOrder. - src/lib/poolMatcher.ts: stop selecting / populating / aggregating the field - src/lib/remsImport.ts: drop the column, header aliases (years, experience, yearsexperience), and the parsing case - 5 API routes: strip from request bodies, update payloads, select clauses, filter parsing (expMin / expMax) - 4 UI files: remove form input on register / profile, the column + chip + filter dropdown on REMS pool, the doc string in admin import help, and the EXPERIENCE_OPTIONS constant in badgeColors - tests: remove the "experience score" describe block; drop the "filters by experience range" test; remove yearsExperience from the helpers factory; rerank the compareRanking test to use workload as the third criterion - prisma/seed.ts: remove yearsExperience from the PoolTeacher.create call, drop the `years: number` field from the inline data type, and strip `years: N,` from each of the 30 seeded pool-teacher rows User confirmed the auto-assign matching algo isn't currently in use, so losing experience as a tie-breaker is acceptable. Run `npx prisma migrate dev --name drop_pool_teacher_years_experience` locally to generate the migration before applying. Fresh DBs and DBs that have been kept in sync via the consolidated baseline both work; existing data in the column will be lost — that's intentional since it was never populated meaningfully.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR adds testing documentation and additional Playwright render specs, but it also removes the yearsExperience field and “experience”-based functionality across the pool-teacher domain (schema, import, UI, filtering, and matching).
Changes:
- Added a manual bug-bash playbook, per-session log template, and a follow-up plan for real-auth E2E flows.
- Added new Playwright render/guard specs (teacher absences, report views, pool views, multi-tenancy isolation).
- Removed pool-teacher “years of experience” support end-to-end (Prisma schema, REMS import parsing, API filters/response fields, UI fields, and matching criterion).
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/api/rems-candidates-filters.test.ts | Removes experience-range filter coverage from REMS candidates API tests. |
| tests/api/helpers.ts | Removes yearsExperience from pool-teacher test factory inputs/data. |
| src/lib/remsImport.ts | Drops parsing/aliases for yearsExperience from REMS CSV import. |
| src/lib/poolMatcher.ts | Removes experience data from candidate rows/context used in matching. |
| src/lib/matchingCriteria.ts | Removes the experience criterion and reorders workload priority. |
| src/lib/constants/badgeColors.ts | Removes EXPERIENCE_OPTIONS used by REMS pool UI filters. |
| src/app/register/pool-teacher/page.tsx | Removes “Years of Experience” field from pool-teacher registration flow. |
| src/app/pool/profile/page.tsx | Removes “Years of Experience” field from pool-teacher profile editing. |
| src/app/api/pool-teachers/route.ts | Stops accepting/storing yearsExperience on pool-teacher create/upsert. |
| src/app/api/pool-teachers/me/route.ts | Stops accepting/storing yearsExperience on self-service pool-teacher updates. |
| src/app/api/pool-teachers/import/route.ts | Stops writing yearsExperience during REMS import confirm. |
| src/app/api/pool-teachers/[id]/route.ts | Stops accepting/storing yearsExperience on admin pool-teacher updates. |
| src/app/api/[slug]/rems-candidates/route.ts | Removes expMin/expMax filtering and omits yearsExperience from response. |
| src/app/admin/pool-teachers/PoolTeachersAdmin.tsx | Updates CSV expected-columns copy to remove yearsExperience. |
| src/app/[slug]/dashboard/rems-pool/RemsPool.tsx | Removes experience filter UI and experience display for candidates. |
| src/tests/matchingCriteria.test.ts | Removes unit tests for experience scoring and updates ranking order usage. |
| prisma/seed.ts | Removes experience values from seeded pool teachers (but retains experience-labeled grouping comments). |
| prisma/schema.prisma | Removes PoolTeacher.yearsExperience field from Prisma schema. |
| e2e/teacher.spec.ts | Adds render+snapshot coverage for teacher absences list. |
| e2e/report.spec.ts | Adds render+snapshot coverage for report/reporting views. |
| e2e/pool-views.spec.ts | Adds render+snapshot coverage for preferred pool + REMS pool views. |
| e2e/multi-tenancy.spec.ts | Adds demo-cookie isolation checks for non-demo slugs redirecting to login. |
| docs/testing/path-b-real-auth-e2e-plan.md | Adds a plan for future real-auth flow E2E testing (personas + DB assertions). |
| docs/testing/manual-bug-bash.md | Adds a detailed manual bug-bash playbook and checklist. |
| docs/testing/bug-bash-template.md | Adds a reusable bug-bash session log template. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
455
to
457
| teachingScheme TeachingScheme | ||
| yearsExperience Int? | ||
| availableFrom DateTime? | ||
| availableTo DateTime? |
Comment on lines
22
to
28
| export type CriterionId = | ||
| | "proximity" | ||
| | "subject" | ||
| | "level" | ||
| | "scheme" | ||
| | "experience" | ||
| | "workload"; | ||
|
|
| { nricLast4: "007G", name: "Wong Pei Shan", firstName: "Pei Shan", lastName: "Wong", scheme: "SRE", area: "Tampines", phone: "91001007", email: "peishan.w@example.com", status: "ACTIVE", subjects: ["Humanities", "English"], levels: ["S1", "S2", "S3"] }, | ||
| { nricLast4: "008H", name: "Aisyah binte Ahmad", firstName: "Aisyah", lastName: "Ahmad", scheme: "FAJT", area: "Woodlands", phone: "91001008", email: "aisyah.a@example.com", status: "ACTIVE", subjects: ["Science", "Art"], levels: ["P3", "P4"] }, | ||
|
|
||
| // ── Exp 4-10 years (9 teachers) ── |
|
|
||
| ## Why Path A is not enough | ||
|
|
||
| The demo cookie (`reliefcher_demo`) resolves to a single seeded user (Demo Admin, SCHOOL_ADMIN of school `demo`) — see `prisma/seed.ts:227-243`. Two consequences: |
Comment on lines
453
to
458
| telegramLinkToken String? @unique | ||
| telegramLinkExpiry DateTime? | ||
| teachingScheme TeachingScheme | ||
| yearsExperience Int? | ||
| availableFrom DateTime? | ||
| availableTo DateTime? | ||
| status PoolTeacherStatus @default(PENDING) |
criteriaOrder arrays, test assertions, and seed comments still referenced the removed "experience" criterion. Drops them to match the schema/matching-criteria removal in the prior commit. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
guangshinhaha
changed the title
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.
Summary
yearsExperiencefield end-to-end: schema, API routes, UI (registration/profile/REMS pool), matching criteria, CSV import, seed data, and all testsWhy
Years of experience was not providing meaningful signal for pool teacher matching and added unnecessary complexity to the registration and filtering flows.
Test plan
npm test)npm run buildpassesnpx prisma migrate dev --name remove_years_experienceto generate migration🤖 Generated with Claude Code