Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 22 additions & 5 deletions lib/AppInfo/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@

namespace OCA\Stackiq\AppInfo;

use OCA\Decidesk\Event\DecisionConcludedEvent;
use OCA\OpenRegister\Contract\ObjectServiceInterface;
use OCA\OpenRegister\Event\ObjectCreatedEvent;
use OCA\OpenRegister\Event\ObjectUpdatedEvent;
Expand Down Expand Up @@ -803,13 +802,31 @@ private function registerEventListeners(IRegistrationContext $context): void {
// Sync user profile updates into the contactpersoon mirror.
$context->registerEventListener(UserProfileUpdatedEvent::class, UserProfileUpdatedEventListener::class);

// Project a concluded decidesk contract-approval Decision onto the
// catalog contract. Only fires when decidesk is installed (it owns the
// DecisionConcludedEvent class); the listener filters by sourceApp and
// Project a concluded contract-approval Decision from the decision app
// onto the catalog contract. The listener filters by sourceApp and
// IDOR-checks the decision id before projecting (the In onderhandeling
// -> Actief transition is reached only here). Replaces the former HTTP
// outcome-callback + daily reconcile poll.
$context->registerEventListener(DecisionConcludedEvent::class, DecisionConcludedListener::class);
//
// BOTH SPELLINGS, by FQN STRING rather than `::class`. That app renamed
// its PSR-4 root from OCA\Decidesk to OCA\Decidiq with no compatibility
// alias, and `::class` on an imported name resolves at COMPILE TIME — so
// this registered a class nothing dispatches any more, the listener
// never fired, and every approved contract stayed in `In onderhandeling`.
// An event with no listener and a listener on no event look identical
// from here: nothing throws, nothing is logged.
//
// Registering a name that does not resolve is harmless, because dispatch
// matches on the concrete event class, but the guard is kept so this
// stays symmetric with ContractApprovalService::isDelegationConfigured()
// on the outbound side.
foreach (ContractApprovalService::DECISION_CONCLUDED_EVENTS as $concludedEvent) {
if (class_exists($concludedEvent) === false) {
continue;
}

$context->registerEventListener($concludedEvent, DecisionConcludedListener::class);
}

}//end registerEventListeners()

Expand Down
17 changes: 15 additions & 2 deletions lib/EventListener/DecisionConcludedListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@

namespace OCA\Stackiq\EventListener;

use OCA\Decidesk\Event\DecisionConcludedEvent;
use OCA\Decidesk\Event\DecisionConcludedEvent as DecideskDecisionConcludedEvent;
use OCA\Decidiq\Event\DecisionConcludedEvent as DecidiqDecisionConcludedEvent;
use OCA\Stackiq\Service\ContractApprovalService;
use OCP\EventDispatcher\Event;
use OCP\EventDispatcher\IEventListener;
Expand Down Expand Up @@ -71,7 +72,19 @@ public function __construct(
* @spec openspec/specs/contract-decision-delegation/spec.md
*/
public function handle(Event $event): void {
if (($event instanceof DecisionConcludedEvent) === false) {
// BOTH SPELLINGS. The decision app renamed its PSR-4 root from
// OCA\Decidesk to OCA\Decidiq with no compatibility alias, so an
// `instanceof` against one name silently rejects the other app's real
// event and this method returns as if the event were somebody else's.
// The two classes publish an identical getter surface (verified against
// decidiq development d72839c), so everything below is unchanged.
//
// `instanceof` against a class that is not installed is simply false —
// it neither autoloads nor errors — which is why naming both here costs
// nothing on an instance that runs only one of them.
if (($event instanceof DecidiqDecisionConcludedEvent) === false
&& ($event instanceof DecideskDecisionConcludedEvent) === false
) {
return;
}

Expand Down
27 changes: 27 additions & 0 deletions lib/Service/ContractApprovalService.php
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,33 @@ class ContractApprovalService {
'\\OCA\\Decidesk\\Event\\DecisionRequestedEvent',
];

/**
* The fully-qualified conclusion-event class spellings, NEWEST FIRST — what
* {@see \OCA\Stackiq\AppInfo\Application} attaches the inbound listener to.
*
* THE INBOUND HALF OF THE SAME PROBLEM AS THE CONSTANT ABOVE, and it stayed
* broken after that one was fixed. Application.php imported
* `OCA\Decidesk\Event\DecisionConcludedEvent` and registered `::class`,
* which resolves at COMPILE TIME to a string nothing dispatches any more, so
* the listener attached to a name that never fires. Nothing errors: an
* event with no listener and a listener on no event look identical from
* here, and the contract simply never leaves `In onderhandeling`.
*
* Measured 2026-09-09 against decidiq development d72839c:
* OCA\Decidiq\Event\DecisionConcludedEvent EXISTS with the full getter
* surface this app reads (getSourceApp, getSubjectId, getExternalReference,
* getDecisionId, getStatus); OCA\Decidesk\Event\DecisionConcludedEvent is
* MISSING. The old spelling is kept regardless — an instance pinned to a
* release from before that rename still dispatches it, and dropping it here
* re-breaks the integration in the other direction.
*
* @var array<int, string>
*/
public const DECISION_CONCLUDED_EVENTS = [
'\\OCA\\Decidiq\\Event\\DecisionConcludedEvent',
'\\OCA\\Decidesk\\Event\\DecisionConcludedEvent',
];

/**
* This consumer app id, stamped on the request event as `sourceApp` and
* used by the conclusion listener to filter inbound events.
Expand Down
4 changes: 4 additions & 0 deletions phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ parameters:
# class` errors that a bare ignore pattern cannot fix. Analysis-only;
# never loaded at runtime or by PHPUnit.
- tests/analysis-stubs/decidesk-events.stub.php
# The SAME contract under the namespace that app renamed to. Both
# spellings are in the field, and without this one the analyser proves
# the newer half of the inbound guard dead.
- tests/analysis-stubs/decidiq-events.stub.php

ignoreErrors:
# OrganizationSyncService's `if ($contactObject !== null)` at the top of
Expand Down
1 change: 1 addition & 0 deletions psalm.xml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
what also resolves the isHandled()/getDecisionId() calls on the
dispatched event. Analysis-only; never loaded at runtime. -->
<file name="tests/analysis-stubs/decidesk-events.stub.php" preloadClasses="true" />
<file name="tests/analysis-stubs/decidiq-events.stub.php" preloadClasses="true" />
<!-- OpenRegister RegisterResolverService. Same cross-app situation; this
stub already existed for PHPUnit mock generation and mirrors the real
signature in openregister/lib/Service/RegisterResolverService.php. -->
Expand Down
114 changes: 114 additions & 0 deletions tests/Unit/EventListener/DecisionConcludedListenerNamespaceTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
<?php

/**
* Tests that the contract-approval conclusion listener accepts the decision
* app's event under BOTH namespaces it has shipped.
*
* @category Test
* @package OCA\Stackiq\Tests\Unit\EventListener
* @author Conduction b.v. <info@conduction.nl>
* @copyright 2026 Conduction B.V.
* @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12
* @link https://github.com/ConductionNL/stackiq
*
* SPDX-FileCopyrightText: 2026 Conduction B.V. <info@conduction.nl>
* SPDX-License-Identifier: EUPL-1.2
*/

declare(strict_types=1);

namespace OCA\Stackiq\Tests\Unit\EventListener;

use OCA\Stackiq\EventListener\DecisionConcludedListener;
use OCA\Stackiq\Service\ContractApprovalService;
use PHPUnit\Framework\TestCase;
use Psr\Log\LoggerInterface;

require_once __DIR__ . '/decision-conclusion-event-doubles.php';

/**
* A cross-app event class name is a runtime lookup this app can only follow.
*
* The decision app renamed its PSR-4 root from `OCA\Decidesk` to `OCA\Decidiq`
* with no compatibility alias. `handle()` tested `instanceof` against the old
* spelling only, so the real event arrived and was rejected as somebody else's:
* the contract stayed in `In onderhandeling`, nothing threw, and nothing was
* logged. An event with no listener looks exactly like a listener on no event.
*
* Both spellings are asserted, because pinning either one alone reproduces the
* outage on the half of the fleet running the other.
*/
class DecisionConcludedListenerNamespaceTest extends TestCase {
/**
* The listener projects an outcome carried by either spelling of the event.
*
* @return void
*/
public function testTheOutcomeIsProjectedUnderEitherFleetNamespace(): void {
$spellings = [
'OCA\Decidiq\Event\DecisionConcludedEvent',
'OCA\Decidesk\Event\DecisionConcludedEvent',
];

foreach ($spellings as $fqcn) {
$projected = [];

$approvalService = $this->createMock(ContractApprovalService::class);
$approvalService->method('resolveContractForOutcome')->willReturn('contract-1');
$approvalService->method('projectOutcome')->willReturnCallback(
static function (string $contractUuid, string $outcomeStatus) use (&$projected): void {
$projected[] = [$contractUuid, $outcomeStatus];
}
);

$listener = new DecisionConcludedListener($approvalService, $this->createMock(LoggerInterface::class));
$listener->handle(new $fqcn('decision-1', 'approved', ContractApprovalService::SOURCE_APP, 'subject-1', 'ext-1'));

$this->assertSame(
[['contract-1', 'approved']],
$projected,
'the listener ignored a real conclusion event dispatched as ' . $fqcn
);
}
}//end testTheOutcomeIsProjectedUnderEitherFleetNamespace()

/**
* An event raised by a different consumer app is still ignored.
*
* Widening the accepted class names must not widen the sourceApp filter:
* that filter is what stops this app projecting another consumer's decision
* onto its own contracts.
*
* @return void
*/
public function testAnotherConsumersDecisionIsStillIgnored(): void {
$approvalService = $this->createMock(ContractApprovalService::class);
$approvalService->expects($this->never())->method('resolveContractForOutcome');

$listener = new DecisionConcludedListener($approvalService, $this->createMock(LoggerInterface::class));
$listener->handle(
new \OCA\Decidiq\Event\DecisionConcludedEvent('decision-1', 'approved', 'dossiq', 'subject-1', 'ext-1')
);

}//end testAnotherConsumersDecisionIsStillIgnored()

/**
* The registration list carries both spellings, newest first.
*
* Written out rather than read from the constant under test: iterating the
* same list the assertion checks is the shape that cannot fail.
*
* @return void
*/
public function testTheRegistrationListCarriesBothSpellingsNewestFirst(): void {
$this->assertSame(
[
'\OCA\Decidiq\Event\DecisionConcludedEvent',
'\OCA\Decidesk\Event\DecisionConcludedEvent',
],
ContractApprovalService::DECISION_CONCLUDED_EVENTS,
'order is the contract: the current namespace first, the pre-rename one retained'
);

}//end testTheRegistrationListCarriesBothSpellingsNewestFirst()
}//end class
Loading
Loading