From 249aef077ee6c06a864ccbeaf728f73dc5b2d128 Mon Sep 17 00:00:00 2001 From: smarcet Date: Wed, 16 Sep 2026 16:37:39 -0300 Subject: [PATCH 1/2] fix(presentations): catch Throwable in PresentationMaterial::getSummitId Removing a media upload nulls the owning presentation before Doctrine's PreRemove fires, so getSummitId() dereferences null and PHP throws \Error. The catch only covered \Exception, so the error escaped the lifecycle listener and aborted the flush, which is what breaks the Services CI job on main since #579. Catch \Throwable and fall back to 0, the degraded case PresentationMaterialEventDispatchTest already documents and asserts. --- .../Events/Presentations/Materials/PresentationMaterial.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Models/Foundation/Summit/Events/Presentations/Materials/PresentationMaterial.php b/app/Models/Foundation/Summit/Events/Presentations/Materials/PresentationMaterial.php index a67f1bfa0..486fb2969 100644 --- a/app/Models/Foundation/Summit/Events/Presentations/Materials/PresentationMaterial.php +++ b/app/Models/Foundation/Summit/Events/Presentations/Materials/PresentationMaterial.php @@ -81,7 +81,7 @@ public function getSummitId(): int try { return $this->presentation->getSummitId(); } - catch (\Exception $ex){ + catch (\Throwable $ex){ return 0; } } From 6962b0b2f1a8f1600f2e5a5cd2f17b572badeb1f Mon Sep 17 00:00:00 2001 From: smarcet Date: Wed, 16 Sep 2026 16:42:18 -0300 Subject: [PATCH 2/2] fix(presentations): catch Throwable in PresentationMaterial::getPresentationId too Same shape as getSummitId(): a material whose presentation is not set dereferences null and throws \Error, which the \Exception catch did not cover. Add a test asserting both accessors return 0 in that state. --- .../Materials/PresentationMaterial.php | 2 +- .../PresentationMaterialEventDispatchTest.php | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/app/Models/Foundation/Summit/Events/Presentations/Materials/PresentationMaterial.php b/app/Models/Foundation/Summit/Events/Presentations/Materials/PresentationMaterial.php index 486fb2969..50dfb8946 100644 --- a/app/Models/Foundation/Summit/Events/Presentations/Materials/PresentationMaterial.php +++ b/app/Models/Foundation/Summit/Events/Presentations/Materials/PresentationMaterial.php @@ -68,7 +68,7 @@ public function getPresentationId(){ try { return $this->presentation->getId(); } - catch (\Exception $ex){ + catch (\Throwable $ex){ return 0; } } diff --git a/tests/Unit/Services/PresentationMaterialEventDispatchTest.php b/tests/Unit/Services/PresentationMaterialEventDispatchTest.php index 0f7e790d6..a457e81b7 100644 --- a/tests/Unit/Services/PresentationMaterialEventDispatchTest.php +++ b/tests/Unit/Services/PresentationMaterialEventDispatchTest.php @@ -136,4 +136,17 @@ public function testDeleteMediaUploadDispatchesLifeCycleEventWithoutError(): voi $this->assertCount(1, $jobs, 'Expected 1 ProcessScheduleEntityLifeCycleEvent for PresentationMediaUpload delete'); $this->assertSame(0, $jobs[0]->summit_id, 'summit_id is 0 here because unsetPresentation() runs before PreRemove - accepted degraded case, not a regression'); } + + /** + * Both accessors must survive a material whose owning presentation is not set - the + * state removeMediaUpload() leaves behind before PreRemove fires. A null dereference + * throws \Error, which a catch of \Exception does not cover. + */ + public function testAccessorsReturnZeroWhenPresentationIsNotSet(): void + { + $media_upload = new PresentationMediaUpload(); + + $this->assertSame(0, $media_upload->getPresentationId()); + $this->assertSame(0, $media_upload->getSummitId()); + } }