[biobank] Fix fatal errors on shipment submission#10882
Conversation
Guard CenterID construction in the Shipment and Log entity constructors. When a shipment or log was submitted without a center (e.g. no containers selected, so no derived centerId), the raw null was passed straight into the CenterID constructor, which requires a non-null string, throwing a TypeError before the handler's validation could run. Construction is now guarded with isset so a null center is stored as null and caught by the existing validator, which returns a clean field-level error to the user. Also remove the #[\Override] attribute from Shipments::isAccessibleBy, which has no matching parent method and caused a fatal error under PHP 8.3's override enforcement.
|
@HenriRabalais . Here's a 1 minute video. Your changes work so with the integration tests passed you'll get my LGTM Kooha-2026-07-13-21-24-00.mp4 |
|
There are other issues :like temperature can be a string value. Should it be contrained to an integer value and set to Celcius? |
|
Also: why does |
|
The other issue ": I entered something in the container field and the error message showed "at least 1 container must be selected" which is not intuitive behaviour, given that I entered something. I also don't know what I am supposed to enter in terms of container. Should it not be a list. I know these issues are off topic. Should I make new issues for each? lmk |
| * | ||
| * @return bool | ||
| */ | ||
| #[\Override] |
There was a problem hiding this comment.
This should not be removed. #[\Override] means that it overrides or implements an interface and isAccessibleBy comes from the AccessibleResource interface.
Fixed in #10801. @HenriRabalais you might want to link |
4 participants
Brief summary of changes
Fixes two fatal errors in the biobank shipments module that occurred when submitting a shipment.
The main bug: when a shipment was submitted without a center (for example when no containers were selected, so no
centerIdcould be derived on the frontend) the rawnullwas passed straight into theCenterIDconstructor.CenterIDextendsValidatableIdentifier, whose constructor requires a non-null string, so this threw aTypeErrorand returned a 500 before the shipment handler's validation could run. Because construction happened before validation, the existing_validate()logic was never reached.The
CenterIDconstruction in both theShipmentandLogentity constructors is now guarded withisset, so a null center is stored asnulland caught by the existing validator, which returns the intended field-level error to the user rather than a fatal.DateTimeconstruction inLogwas guarded the same way, since it had the same latent issue with null date/time values.Separately, the
#[\Override]attribute was removed fromShipments::isAccessibleBy(). The method has no matching parent method, so under PHP 8.3's override enforcement it threw its own fatal error on every request to the module.Resolves #10761 — the non-intuitive save behaviour in that issue was this failure path: submitting without containers threw the fatal instead of showing a graceful error. With this fix the submission returns the proper field validation errors.
Testing instructions