fix: 1337 middleware checkpermission hebergement user#1204
Open
olivier-rabot wants to merge 2 commits intomainfrom
Open
fix: 1337 middleware checkpermission hebergement user#1204olivier-rabot wants to merge 2 commits intomainfrom
olivier-rabot wants to merge 2 commits intomainfrom
Conversation
olivier-rabot
temporarily deployed
to
review-auto
GitHub Actions
Inactive
revu-bot
reviewed
Feb 27, 2026
Collaborator
revu-bot
left a comment
revu-bot
left a comment
There was a problem hiding this comment.
PR Review: fix/1337 middleware checkpermission hebergement user
This PR fixes two issues:
- SQL typo bug (
pp.current→pph.current) ingetIsHebergementAutoriseForUserIdquery — this was the root cause of the crash. - Missing error handling in the
checkPermissionHebergementUsermiddleware — the unhandled promise rejection from the SQL error was crashing the request without a proper response.
Both fixes are correct and necessary. One additional concern is noted below regarding a residual SQL inconsistency in the getListe query that was not addressed in this PR.
| File | Lines | Severity | Issue |
|---|---|---|---|
Hebergement.js |
~194 | CRITICAL | Residual pp alias in getListe query — same class of bug as the one fixed |
checkPermissionHebergementUser.js |
~35 | MINOR | return is unnecessary before the last next() call in the catch block |
revu-bot
reviewed
Feb 27, 2026
olivier-rabot
temporarily deployed
to
build-review-auto
GitHub Actions
Inactive
olivier-rabot
temporarily deployed
to
build-review-auto
GitHub Actions
Inactive
olivier-rabot
temporarily deployed
to
build-review-auto
GitHub Actions
Inactive
olivier-rabot
temporarily deployed
to
build-review-auto
GitHub Actions
Inactive
olivier-rabot
temporarily deployed
to
build-review-auto
GitHub Actions
Inactive
revu-bot
reviewed
Feb 27, 2026
Collaborator
revu-bot
left a comment
revu-bot
left a comment
There was a problem hiding this comment.
PR Review: fix/1337 middleware checkpermission hebergement user
This PR fixes a SQL bug (pp alias used instead of pph) in getIsHebergementAutoriseForUserId that caused a runtime error, and wraps the middleware call in a try/catch to handle such errors gracefully. The fix is correct and targeted.
| File | Lines | Severity | Issue |
|---|---|---|---|
Hebergement.js |
228 | CRITICAL | Redundant/duplicate personne_physique JOIN in getListe query still present after cleanup |
Hebergement.js |
191–197 | IMPORTANT | getIsHebergementAutoriseForUserId query may return false positives due to LEFT JOIN semantics |
checkPermissionHebergementUser.js |
34–40 | MINOR | Error detail (err) is logged but not forwarded — acceptable, but consider structured logging |
Overall the root-cause fix is correct. One residual concern in the getListe query and a semantic issue with the permission check query are worth addressing.
|
🎉 Deployment for commit 17bc940 : Ingresses
Docker images
|
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
2 participants
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.
Ticket(s) lié(s)
https://jira-mcas.atlassian.net/jira/software/c/projects/VAO/boards/336?selectedIssue=VAO-1337
Description
Erreur lors de la desactivation/activation (?) d'un hebergement. Problème d'alias dans la requete SQL. Aussi, suppression d'une double jointure dans une requete sql du meme service (requete getListe)
Screenshot / liens loom
Check-list
console.log<script lang="ts">Testing instructions