AuditCheck: P_I_AgeTooHigh - age too high - #1732
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
📜 Recent review details⏰ Context from checks skipped due to timeout. (6)
🧰 Additional context used🧠 Learnings (2)📚 Learning: 2026-03-31T18:20:32.322ZApplied to files:
📚 Learning: 2026-04-21T14:52:21.885ZApplied to files:
🔇 Additional comments (5)
WalkthroughAdded configurable maximum and review-threshold ages to project configuration and widget validation. Added backend audit checks and localized messages for ages above the maximum and very old ages. Added parameterized tests covering threshold boundaries, typical ages, and missing values. Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: one or more packages not found in the registry. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
e1db6c6 to
efd6cdd
Compare
| "HM_I_geographyNotInSurveyTerritory": "Home geography is outside the survey territory", | ||
|
|
||
| "P_M_Age": "Person age is missing", | ||
| "P_I_AgeTooHigh": "Person age is too high (should be at most 125)", |
There was a problem hiding this comment.
It seems that maxPersonAge is a variable, with 125 being the default.
Therefore, shouldn't the error message be dynamic and display maxPersonAge, instead of a hardcoded '125'?
There was a problem hiding this comment.
Voir commentaire plus bas
| errorCode: 'P_W_VeryOldAge', | ||
| version: 1, | ||
| level: 'warning', | ||
| message: 'Person age is very high (please verify, between 100 and 125)', |
There was a problem hiding this comment.
Same comment here, 100 and 125 should be minReviewPersonAge and maxPersonAge instead.
Also, what's the difference between the message defined here, and in audits.json? (I see they're slightly different)
There was a problem hiding this comment.
Bonne observation. La logique utilise projectConfig.maxPersonAge / minReviewPersonAge, mais l’UI admin affiche t('audits:' + errorCode) depuis locales/*/audits.json — pas audit.message, et pas d’interpolation aujourd’hui. C’est la même chose que HH_I_Size (pas d'interpolation, valeurs en raw). Pour cette PR, je propose de garder 125/100 dans les locales alignés sur les defaults de project.config.ts. L’interpolation i18n de projectConfig dans AuditDisplay serait un autre issue séparé si on veut généraliser ça.
There was a problem hiding this comment.
The localized stings have been updated to remove any hardcoded values. Until #1741 is fixed, we should remove any interpolated data in the error localized strings.
b8dc98b to
e78db69
Compare
e78db69 to
319e83d
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/evolution-common/src/config/__tests__/project.config.test.ts`:
- Around line 18-19: Add isolated non-default threshold coverage: in
packages/evolution-common/src/config/__tests__/project.config.test.ts lines
18-19, set and assert non-default maxPersonAge and minReviewPersonAge, restoring
shared configuration afterward; in
packages/evolution-backend/src/services/audits/auditChecks/checks/__tests__/person/P_I_AgeTooHigh.test.ts
lines 17-43, verify the error boundary uses the configured maximum; and in
packages/evolution-backend/src/services/audits/auditChecks/checks/__tests__/person/P_W_VeryOldAge.test.ts
lines 17-45, verify the warning range uses the configured minimum and maximum.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 75adaf7a-826b-425b-8b16-068a1ee7f5fd
📒 Files selected for processing (8)
locales/en/audits.jsonlocales/fr/audits.jsonpackages/evolution-backend/src/services/audits/auditChecks/checks/PersonAuditChecks.tspackages/evolution-backend/src/services/audits/auditChecks/checks/__tests__/person/P_I_AgeTooHigh.test.tspackages/evolution-backend/src/services/audits/auditChecks/checks/__tests__/person/P_W_VeryOldAge.test.tspackages/evolution-common/src/config/__tests__/project.config.test.tspackages/evolution-common/src/config/project.config.tspackages/evolution-common/src/services/widgets/validations/validations.ts
📜 Review details
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-03-31T18:20:32.322Z
Learnt from: kaligrafy
Repo: chairemobilite/evolution PR: 1462
File: packages/evolution-common/src/services/baseObjects/Completable.ts:1-6
Timestamp: 2026-03-31T18:20:32.322Z
Learning: In this repository (chairemobilite/evolution), do not flag or require a copyright year in file headers. For new files, the header format `Copyright Polytechnique Montreal and contributors` (without a year) is acceptable. Only verify the header presence/consistency, not the presence of the year.
Applied to files:
packages/evolution-backend/src/services/audits/auditChecks/checks/__tests__/person/P_W_VeryOldAge.test.tspackages/evolution-common/src/config/__tests__/project.config.test.tspackages/evolution-common/src/services/widgets/validations/validations.tspackages/evolution-common/src/config/project.config.tspackages/evolution-backend/src/services/audits/auditChecks/checks/__tests__/person/P_I_AgeTooHigh.test.tspackages/evolution-backend/src/services/audits/auditChecks/checks/PersonAuditChecks.ts
📚 Learning: 2026-04-21T14:52:21.885Z
Learnt from: kaligrafy
Repo: chairemobilite/evolution PR: 1510
File: packages/evolution-backend/src/services/audits/auditChecks/README.md:148-149
Timestamp: 2026-04-21T14:52:21.885Z
Learning: When adding a new backend audit check error code in the Evolution project, you must also add the corresponding translations in locales/en/audits.json and locales/fr/audits.json. This is required because the UI looks up error codes via t('audits:${errorCode}') and the CI test AuditLocales.test.ts enforces that all audit error codes have locale entries; without the locale keys, the build should fail.
Applied to files:
locales/en/audits.jsonlocales/fr/audits.json
🔇 Additional comments (5)
packages/evolution-common/src/config/project.config.ts (1)
86-95: LGTM!Also applies to: 258-259
packages/evolution-common/src/services/widgets/validations/validations.ts (1)
223-223: LGTM!Also applies to: 251-251
packages/evolution-backend/src/services/audits/auditChecks/checks/PersonAuditChecks.ts (1)
2-8: LGTM!Also applies to: 35-88
locales/en/audits.json (1)
31-32: LGTM!locales/fr/audits.json (1)
31-32: LGTM!
| * Age from which a person age triggers a reviewer warning audit (inclusive). | ||
| * Applies up to {@link maxPersonAge}. Defaults to 100. | ||
| */ | ||
| minReviewPersonAge: number; |
There was a problem hiding this comment.
Il va falloir qu'on fasse du ménage dans les configurations d'âge, on commence à en avoir beaucoup de différents! Pour la prochaine version d'Evolution...
Aussi, je ne suis pas certaine de ce nom, 1- ça concerne les audits uniquement, 2- ça ajouter simplement un warning, donc addAuditWarningAge? Et le default pourrait être undefined si on ne veut pas de warning. Ou à tout le moins mettre ce champ optionnel
There was a problem hiding this comment.
default = undefined
| const { person } = context; | ||
| const age = person.age; | ||
|
|
||
| if (age !== undefined && age !== null && age > projectConfig.maxPersonAge) { |
There was a problem hiding this comment.
typeof age === 'number' à la place?
AuditCheck: P_W_VeryOldAge - very old age Add audit checks for person age: - P_I_AgeTooHigh: error when age > `maxPersonAge`, default 125 - P_W_VeryOldAge: warning when age between `addAuditWarningOldAge` and `maxPersonAge`, default disable (addAuditWarningOldAge = undefined) Update project config: - Add `maxPersonAge` and `addAuditWarningOldAge` Update age real time validation according to new project config
319e83d to
adfae60
Compare
3 participants
AuditCheck: P_W_VeryOldAge - very old age
Add audit checks for person age:
maxPersonAge, default 125veryOldPersonAgeandmaxPersonAge, default 100–125Update project config:
maxPersonAgeandveryOldPersonAgeminReviewPersonAgeto matchveryOldPersonAgeUpdate age real time validation according to new project config
Summary by CodeRabbit
maxPersonAge) and an optional “very old age” warning threshold.maxPersonAgeinstead of a fixed limit.