Skip to content

Conversation

@tuupke
Copy link
Contributor

@tuupke tuupke commented Nov 28, 2025

The route requires the judgehostid as route parameter, not the hostname.

Currently clicking the notification results in the screen shown below. Arguably this should be a 4xx error, but then it should be fixed within all route specifications by appending <\d+> to the parameter name. e.g.:

#[Route(path: '/{judgehostid}', methods: ['GET'], name: 'jury_judgehost')]
to 
#[Route(path: '/{judgehostid<\d+>}', methods: ['GET'], name: 'jury_judgehost')]
image

The route requires the judgehostid as route parameter, not the hostname.
@tuupke
Copy link
Contributor Author

tuupke commented Nov 28, 2025

@nickygerritsen is changing things in #3024 to use external ids (strings) everywhere but here. So will add that change here too.

This fixes issues with non-integer judgehostids throwing a 500 instead of a 404.
@tuupke tuupke enabled auto-merge November 28, 2025 09:53
@tuupke tuupke added this pull request to the merge queue Nov 28, 2025
Merged via the queue into DOMjudge:main with commit 31f7e81 Nov 28, 2025
37 checks passed
@tuupke tuupke deleted the Fix/judgehost-down-notification branch November 28, 2025 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants