Skip to content

Commit f3f4abc

Browse files
rubenvdlindeclaude
andauthored
feat(tasks): find the newest sequence for a template, across every anchor (#3360)
The mapper could answer "what happened to THIS object" three ways and "has this approval ever run" not at all. Every existing finder constrains anchor_object_uuid, so an aggregate over a template had no anchor to pass. buildiq is the caller that needed it. Its automation dry-run panel used to read the newest ApprovalStep on a chain; when #3302 retired that surface there was no replacement for the aggregate, so the panel degraded to reporting nothing at all (buildiq#651). This restores it. One row off the existing template_id index, ordered like its siblings and limited in the QUERY rather than trimmed in PHP. Tested with a negative control, not just a green run: adding an anchor predicate and dropping the limit makes the new test fail with "Failed asserting that an array does not contain 'anchor_object_uuid'". A finder that quietly constrained the anchor would answer a different question than the caller asked, which is the one thing this method must not do. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 925c204 commit f3f4abc

2 files changed

Lines changed: 63 additions & 0 deletions

File tree

lib/Db/TaskSequenceMapper.php

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,43 @@ public function findRunning(string $anchorObjectUuid, string $templateId): ?Task
9898
return ($rows[0] ?? null);
9999
}//end findRunning()
100100

101+
/**
102+
* The newest sequence opened from a template, across EVERY anchor.
103+
*
104+
* The other finders here answer "what happened to THIS object". This one
105+
* answers "has this approval ever run", which is what a designer surface
106+
* needs to report a template's last outcome without knowing an object to
107+
* ask about.
108+
*
109+
* buildiq's automation dry-run panel is the caller: it used to read the
110+
* newest ApprovalStep on the chain, and when #3302 retired that surface the
111+
* per-anchor finders could not replace it — an aggregate over the template
112+
* has no anchor to pass. Without this the panel degraded to reporting
113+
* nothing at all (buildiq#651).
114+
*
115+
* Indexed by `template_id` and ordered like its siblings, so it is one row
116+
* off the same index rather than a scan.
117+
*
118+
* @param string $templateId The compiled template id.
119+
*
120+
* @return TaskSequence|null The newest sequence for the template, or null when none has run.
121+
*
122+
* @spec openspec/changes/flow-approval-consolidation/specs/flow-approval-consolidation/spec.md#requirement-an-approval-is-an-ordered-task-sequence-with-o
123+
*/
124+
public function findNewestForTemplate(string $templateId): ?TaskSequence {
125+
$qb = $this->db->getQueryBuilder();
126+
$qb->select('*')
127+
->from($this->getTableName())
128+
->where($qb->expr()->eq('template_id', $qb->createNamedParameter($templateId)))
129+
->orderBy('opened_at', 'DESC')
130+
->addOrderBy('id', 'DESC')
131+
->setMaxResults(1);
132+
133+
$rows = $this->findEntities(query: $qb);
134+
135+
return ($rows[0] ?? null);
136+
}//end findNewestForTemplate()
137+
101138
/**
102139
* Every sequence for an anchor and template, newest first.
103140
*

tests/Unit/Db/TaskSequenceMapperQueriesTest.php

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,32 @@ public function testFindNewestForAnchorOrdersByOpenTimeDescending(): void {
7676
self::assertNotSame([], $orderings, 'history must be explicitly ordered, never id-lucky');
7777
}//end testFindNewestForAnchorOrdersByOpenTimeDescending()
7878

79+
public function testFindNewestForTemplateDoesNotFilterOnAnAnchor(): void {
80+
$mapper = new TaskSequenceMapper(db: $this->connectionWith(rows: [$this->row('seq-9', 'rejected')]));
81+
82+
$newest = $mapper->findNewestForTemplate(templateId: 'tpl-1');
83+
84+
self::assertSame('seq-9', $newest->getUuid());
85+
86+
// The whole point of this finder: it aggregates ACROSS anchors, so an
87+
// anchor predicate would silently answer a different question than the
88+
// caller asked. Its siblings all constrain the anchor; this one must not.
89+
$predicates = array_filter($this->calls, static fn (array $call): bool => $call[0] === 'expr.eq');
90+
$columns = array_map(static fn (array $call): mixed => $call[1], $predicates);
91+
self::assertContains('template_id', $columns);
92+
self::assertNotContains('anchor_object_uuid', $columns, 'a template-wide finder must not constrain the anchor');
93+
94+
// One row off the index, not a scan the caller trims.
95+
$limits = array_filter($this->calls, static fn (array $call): bool => $call[0] === 'setMaxResults');
96+
self::assertNotSame([], $limits, 'the newest row must be taken by the query, not in PHP');
97+
}//end testFindNewestForTemplateDoesNotFilterOnAnAnchor()
98+
99+
public function testFindNewestForTemplateWithNoRunIsNull(): void {
100+
$mapper = new TaskSequenceMapper(db: $this->connectionWith(rows: []));
101+
102+
self::assertNull($mapper->findNewestForTemplate(templateId: 'tpl-1'));
103+
}//end testFindNewestForTemplateWithNoRunIsNull()
104+
79105
public function testATerminalStatusReadsAsTerminal(): void {
80106
$sequence = new TaskSequence();
81107
$sequence->setStatus(TaskSequence::STATUS_REJECTED);

0 commit comments

Comments
 (0)