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
1 change: 1 addition & 0 deletions changelog.d/fixed-shared-pr-secondary-row-lane.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Activity Monitor: a secondary issue that shares a PR with another issue (its row shows the compact "↳ #PR · under #primary" chip) now lights the PR's lane — CI / Review, Merge queue (soaking), Merged — like the primary row, instead of wearing the PR's stage word in the "PR in progress" lane (hivecommons/hive#17324).
29 changes: 26 additions & 3 deletions src/pkg/dashboard/stage_tracker_actor_icon_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,15 +150,38 @@ assert.strictEqual(strips(row).length, 1);
assert.ok(row.includes('<span class="ham-detour">Remediating · fixer</span>') && row.includes('aria-label="Next move: hive"'), row);
assert.deepStrictEqual(elsewhere(row), [], 'owner carries no elsewhere chip');

// The other issue does not redraw the PR: its own strip (040 Covered by PR) and a chip pointing at the owner row.
// The other issue does not redraw the PR (no PR column entry, no sub-row) and
// chips to the owner row — but its strip rides the PR's stage all the same:
// the lane is the PR's, not the issue's role (hivecommons/hive#17324).
row = stageTrackerRowHtml(issue(14911, [lp(14921, 14910)]), stages, ctx);
assert.strictEqual(prCol(row).length, 0, 'the PR column is empty, no second ↳ #14921 row: ' + row);
assert.strictEqual(subs(row).length, 0);
assert.strictEqual(strips(row).length, 1);
assert.ok(strip(row).includes('<span class="ham-seg-t">PR in progress</span>') && strip(row).includes('title="Covered by PR'), 'the issue draws its own 040 stage (lit at the PR column): ' + strip(row));
assert.ok(strip(row).includes('<span class="ham-detour">Remediating · fixer</span>'), 'the secondary rides the PR\'s 070 lane like the owner: ' + strip(row));
assert.ok(!strip(row).includes('title="Covered by PR'), 'not the issue\'s own 040: ' + strip(row));
assert.deepStrictEqual(elsewhere(row), [{ title: 'PR #14921 covers this issue too; its row is drawn under #14910', text: '↳ #14921 · under #14910' }]);
assert.ok(!/<span class="ham-detour">(?:Fixing|Remediating)<\/span>/.test(strip(row)), 'no second lit remediation pill');
assert.ok(tooltip(row).includes('Covering PR: #14921'), 'the row tooltip still names the covering PR');
assert.strictEqual(stageTrackerLinkedRiding(issue(14911, [lp(14921, 14910)]), stages).number, 14921);
assert.strictEqual(stageTrackerLinkedRiding(issue(14910, [lp(14921, 14910)]), stages), null, 'drawn here → not riding');
assert.strictEqual(stageTrackerLinkedRiding(issue(14911, [lp(14921, 14910, { state: 'closed' })]), stages), null, 'a closed PR drawn elsewhere drives nothing');
assert.strictEqual(stageTrackerLinkedRiding(issue(14911, [lp(14921, 14910, { stage: 50 }), lp(14922, 14910, { stage: 80 })]), stages).number, 14922, 'furthest along first');

// hivecommons/hive#17324: PR #17308 closes #17292 (primary) and #17302; it
// is soaking in the merge queue. Both rows light the Merge queue lane with
// the soak word — #17302 used to show "Soaking" in the PR in progress lane.
const soakUntil = new Date(now + 47000).toISOString();
const soaking = (under) => ({ number: 17308, title: 'fix', url: 'https://x/17308', author: 'hive-bot', author_kind: 'hive', stage: 95, stage_since: since, drawn_under: under, head: 'head',
caption: 'Soaking', merge_gate: { state: 'soaking', soak_until: soakUntil, soak_remaining_seconds: 47 }, soak_remaining_seconds: 47, stage_actor: { kind: 'hive', login: '', why: 'the lane holds it for the soak' } });
const primary = stageTrackerRowHtml(Object.assign(issue(17292, [soaking(17292)]), { stage: 40 }), stages, ctx);
const secondary = stageTrackerRowHtml(Object.assign(issue(17302, [soaking(17292)]), { stage: 30 }), stages, ctx);
const lit = (html) => (/<li class="ham-seg active[^"]*"[^>]*data-ham-stage="(\d+)"/.exec(strip(html)) || [])[1] || (/<li class="ham-seg active[^"]*"[^>]*title="([^"]*)"/.exec(strip(html)) || [])[1] || '';
assert.strictEqual(prCol(primary).length, 1, primary);
assert.strictEqual(prCol(secondary).length, 0, 'the secondary keeps its chip, not the PR title: ' + secondary);
assert.deepStrictEqual(elsewhere(secondary).map(e => e.text), ['↳ #17308 · under #17292']);
assert.ok(strip(primary).includes('Soaking'), 'primary soaks: ' + strip(primary));
assert.ok(strip(secondary).includes('Soaking'), 'secondary soaks: ' + strip(secondary));
assert.strictEqual(lit(secondary), lit(primary), 'the same chevron is lit on both rows: ' + lit(primary) + ' vs ' + lit(secondary));
assert.ok(!strip(secondary).includes('<span class="ham-seg-t">PR in progress</span>'), 'the secondary no longer lights PR in progress: ' + strip(secondary));

// Several PRs: the ones drawn here are sorted and drawn, the rest become chips.
row = stageTrackerRowHtml(issue(14912, [lp(14921, 14910), lp(14930, 14912), lp(14931, 14912, { stage: 50 })]), stages, ctx);
Expand Down
6 changes: 6 additions & 0 deletions src/pkg/dashboard/stage_tracker_motion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,7 @@ func TestStageTrackerMotionDetector(t *testing.T) {
jsFunc(t, html, "stageTrackerLinkedSupersededGone"),
jsFunc(t, html, "stageTrackerLinkedSupersededTip"),
jsFunc(t, html, "stageTrackerLinkedSorted"),
jsFunc(t, html, "stageTrackerLinkedRiding"),
jsConstLine(t, html, "STAGE_TRACKER_ATTENTION_STAGES"),
jsConstLine(t, html, "STAGE_TRACKER_HUMAN_MODS"),
jsFunc(t, html, "stageTrackerRegressionHandled"),
Expand Down Expand Up @@ -312,6 +313,11 @@ const I = (n, stage, linked) => ({ kind: 'issue', repo: 'a/b', number: n, stage,
// Column map: plain items, an issue riding its single covering PR, nested PRs keyed on their own.
assert.deepStrictEqual(stageTrackerColMap([P(1, 50), I(2, 20), I(3, 20, [{ number: 9, stage: 30 }]), I(4, 20, [{ number: 7, stage: 50 }, { number: 8, stage: 80 }])], stages),
{ 'a/b/pr#1': 4, 'a/b/issue#2': 2, 'a/b/issue#3': 3, 'a/b/pr#9': 3, 'a/b/issue#4': 2, 'a/b/pr#7': 4, 'a/b/pr#8': 6 });
// A PR closing two issues is drawn under the lower one (drawn_under); the
// other issue still rides the PR's column — the lane is the PR's, not the
// issue's own 030/040 (hivecommons/hive#17324).
assert.deepStrictEqual(stageTrackerColMap([I(10, 40, [{ number: 12, stage: 95, drawn_under: 10 }]), I(11, 30, [{ number: 12, stage: 95, drawn_under: 10 }])], stages),
{ 'a/b/issue#10': 6, 'a/b/pr#12': 6, 'a/b/issue#11': 6 }, 'the secondary issue lights the Merge queue column with the primary');
assert.deepStrictEqual(stageTrackerColMap(null, stages), {});
assert.deepStrictEqual(stageTrackerColMap([{ kind: 'pr', repo: 'a/b', number: 0 }, null], stages), {}, 'items without a number are skipped');

Expand Down
1 change: 1 addition & 0 deletions src/pkg/dashboard/stage_tracker_ui_prelude_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ func stageTrackerRowsPreludeJS(t *testing.T, html string) string {
jsFunc(t, html, "stageTrackerLinkedSupersededGone"),
jsFunc(t, html, "stageTrackerLinkedSupersededTip"),
jsFunc(t, html, "stageTrackerLinkedSorted"),
jsFunc(t, html, "stageTrackerLinkedRiding"),
stageTrackerTierJS(t, html),
jsConstLine(t, html, "STAGE_TRACKER_COVERED_BY_PR"),
jsFunc(t, html, "stageTrackerLeadStage"),
Expand Down
3 changes: 2 additions & 1 deletion src/pkg/dashboard/static-src/js/tracker-motion.js
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,8 @@
for (const it of items) {
if (!it || !it.kind || !(Number(it.number) > 0)) continue;
const linked = it.kind === 'issue' ? stageTrackerLinkedSorted(it, stages).filter(lp => lp && Number(lp.number) > 0) : [];
const shown = linked.length === 1 ? Object.assign({}, it, { via_pr: true, stage: linked[0].stage || 40, display_column: linked[0].display_column || 0, head: linked[0].head, code_activity: linked[0].code_activity }) : it;
const ride = linked.length === 1 ? linked[0] : (!linked.length && typeof stageTrackerLinkedRiding === 'function' ? stageTrackerLinkedRiding(it, stages) : null);
const shown = ride ? Object.assign({}, it, { via_pr: true, stage: ride.stage || 40, display_column: ride.display_column || 0, head: ride.head, code_activity: ride.code_activity }) : it;
out[stageTrackerItemKey(it)] = stageTrackerColIndex(shown, stages);
for (const lp of linked) out[stageTrackerItemKey({ repo: it.repo, kind: 'pr', number: lp.number })] = stageTrackerColIndex({ kind: 'pr', stage: lp.stage || 0, display_column: lp.display_column || 0, head: lp.head, code_activity: lp.code_activity }, stages);
}
Expand Down
19 changes: 17 additions & 2 deletions src/pkg/dashboard/static-src/js/tracker-row-layout.js
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,20 @@
const sinceOf = lp => Date.parse(lp.stage_since || '') || Infinity;
return linked.sort((x, y) => (stageTrackerAnchor(y.stage, stages, 'pr', y, false) - stageTrackerAnchor(x.stage, stages, 'pr', x, false)) || (stageTrackerAttentionTier(x) - stageTrackerAttentionTier(y)) || (sinceOf(x) - sinceOf(y)) || ((x.number || 0) - (y.number || 0)));
}
// Pure: the covering PR an issue row rides when every PR covering it is
// drawn under another row (its "↳ #N · under #M" chips): the furthest
// along of them (stageTrackerLinkedSorted's order), or null when one is
// drawn here or none covers it. The stage position is the PR's, not
// the issue's role — a secondary issue of a shared PR lights the same
// lane as the primary (soaking in the Merge queue, not "PR in progress"
// wearing the soak word; hivecommons/hive#17324) while its chip, not
// the PR title, names the PR.
function stageTrackerLinkedRiding(item, stages) {
if (!item || item.kind !== 'issue' || !Array.isArray(item.linked_prs) || stageTrackerLinkedSorted(item, stages).length) return null;
const elsewhere = item.linked_prs.filter(lp => lp && Number(lp.number) > 0 && !stageTrackerLinkedDrawnHere(lp, item) && !stageTrackerLinkedSupersededGone(lp, item));
const first = stageTrackerLinkedSorted(Object.assign({}, item, { linked_prs: elsewhere.map(lp => Object.assign({}, lp, { drawn_under: 0 })) }), stages)[0];
return first ? elsewhere.find(lp => Number(lp.number) === Number(first.number)) || null : null;
}

// Pure: a PR whose drawn_under issue is not a row of this page (filtered
// out or on another page) would otherwise be drawn nowhere. The first
Expand Down Expand Up @@ -336,8 +350,9 @@
// — its furthest covering PR, else the issue itself.
function stageTrackerLead(item, stages) {
const linked = item.kind === 'issue' ? stageTrackerLinkedSorted(item, stages) : [];
if (!linked.length) return { lead: item, kind: item.kind, stage: Number(item.stage) || 0, since: item.stage_since || '' };
const lp = linked[0];
// Guarded: test harnesses lift this function by name without stageTrackerLinkedRiding.
const lp = linked[0] || (typeof stageTrackerLinkedRiding === 'function' ? stageTrackerLinkedRiding(item, stages) : null);
if (!lp) return { lead: item, kind: item.kind, stage: Number(item.stage) || 0, since: item.stage_since || '' };
return { lead: lp, kind: 'pr', stage: stageTrackerLeadStage(lp), since: lp.stage_since || item.stage_since || '' };
}
// List order: furthest along the strip first — a covered issue counts as
Expand Down
24 changes: 16 additions & 8 deletions src/pkg/dashboard/static-src/js/tracker-row-render.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,14 +98,22 @@
const linked = stageTrackerLinkedSorted(item, stages);
// The primary (first-sorted) PR's base; an issue with no PR shows none.
if (linked.length) basePill = stageTrackerBasePillHtml(linked[0].base_ref, linked[0].number);
if (linked.length === 1) {
// One covering PR: a single entry whose strip the PR's stage drives.
const lp = linked[0];
const pr = stageTrackerLinkedPRItem(item, lp);
prTargetKey = stageTrackerItemKey(pr);
const who = (lp.author_kind || 'hive') + (lp.author ? ' · @' + lp.author : '');
const lineage = (lp.lineage && typeof stageTrackerLineageChipHtml === 'function' ? stageTrackerLineageChipHtml(lp, nowMs) : '') || (typeof stageTrackerAdoptedFromChipHtml === 'function' ? stageTrackerAdoptedFromChipHtml(lp, nowMs) : '');
prRefs = [STAGE_TRACKER_LINKED_GLYPH + stageTrackerPRPillsHtml(stageTrackerPRRef(lp.number, lp.url, lp.author_kind || '', lp.author, lp.title, (lp.title ? lp.title + '\n' : '') + 'Author: ' + who + ' · ' + stageTrackerLinkedTip(item, lp.number, ctx)), typeof stageTrackerLinkedChipsHtml === 'function' ? stageTrackerLinkedChipsHtml(lp, lineage) : (lineage || null))];
// One covering PR drawn here, or every covering PR drawn under
// another row (hivecommons/hive#17324): a single entry whose strip
// the PR's stage drives. Only the PR drawn here takes the PR column;
// a PR drawn elsewhere is named by the "under #M" chip in the lead
// cell and the PR column stays empty.
// Guarded: test harnesses lift this function by name without stageTrackerLinkedRiding.
const riding = linked.length === 1 ? linked[0] : (!linked.length && typeof stageTrackerLinkedRiding === 'function' ? stageTrackerLinkedRiding(item, stages) : null);
if (riding) {
const lp = riding;
if (linked.length === 1) {
const pr = stageTrackerLinkedPRItem(item, lp);
prTargetKey = stageTrackerItemKey(pr);
const who = (lp.author_kind || 'hive') + (lp.author ? ' · @' + lp.author : '');
const lineage = (lp.lineage && typeof stageTrackerLineageChipHtml === 'function' ? stageTrackerLineageChipHtml(lp, nowMs) : '') || (typeof stageTrackerAdoptedFromChipHtml === 'function' ? stageTrackerAdoptedFromChipHtml(lp, nowMs) : '');
prRefs = [STAGE_TRACKER_LINKED_GLYPH + stageTrackerPRPillsHtml(stageTrackerPRRef(lp.number, lp.url, lp.author_kind || '', lp.author, lp.title, (lp.title ? lp.title + '\n' : '') + 'Author: ' + who + ' · ' + stageTrackerLinkedTip(item, lp.number, ctx)), typeof stageTrackerLinkedChipsHtml === 'function' ? stageTrackerLinkedChipsHtml(lp, lineage) : (lineage || null))];
}
shown = Object.assign({}, item, { via_pr: true, stage: stageTrackerLeadStage(lp), stage_since: lp.stage_since || item.stage_since, modifiers: lp.modifiers || item.modifiers, deferred_modifiers: lp.deferred_modifiers || item.deferred_modifiers || [], caption: lp.caption || item.caption, reason: lp.reason || item.reason, preparation: lp.preparation || null,
ci_activity: lp.ci_activity || null, review_activity: lp.review_activity || null, code_activity: lp.code_activity || null, display_column: lp.display_column || 0,
terminal_merge: !!lp.terminal_merge, merged_by: lp.merged_by || '',
Expand Down
Loading
Loading