Skip to content

Commit 10dcd7d

Browse files
committed
fix(recommendations): thread showCheckboxes into SP-group child rows
The two buildVariantRowMarkup calls that render expanded Savings Plans plan-type group child rows omitted the showCheckboxes argument, so it defaulted to true. In readonly/viewer sessions (showCheckboxes === false) the SP-group child rows rendered a checkbox cell while the column header and non-SP rows did not, leaving the table misaligned and exposing an inert checkbox to a role with no purchase/plan actions. Forward showCheckboxes from the enclosing buildListMarkup scope into both nested calls so SP-group child rows match the non-SP path and the header. Add a regression test (readonly + admin) that expands an SP plan-type group and asserts the child variant rows honor the role's checkbox visibility. The readonly assertion fails before the fix and passes after.
1 parent dadc5fc commit 10dcd7d

2 files changed

Lines changed: 82 additions & 3 deletions

File tree

‎frontend/src/__tests__/recommendations-permissions.test.ts‎

Lines changed: 80 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
* per-row checkboxes, and row-click selection are all hidden/inert.
1414
* Admin and user (operator) roles are unchanged.
1515
*/
16-
import { loadRecommendations } from '../recommendations';
16+
import { loadRecommendations, resetExpandedCells } from '../recommendations';
1717
import * as api from '../api';
1818

1919
jest.mock('../api', () => ({
@@ -300,3 +300,82 @@ describe('Recommendations checkbox + row-click gating for viewer role (issue #86
300300
expect(summaryEffectiveCols).toBe(headerColCount);
301301
});
302302
});
303+
304+
// Issue #135 + #869: SP plan-type group child rows must honor showCheckboxes.
305+
// Two SP recs of different plan types in the same (provider, account, region)
306+
// scope group under one SP parent row. When expanded, each per-plan-type child
307+
// cell renders a variant row via buildVariantRowMarkup. Those nested calls must
308+
// forward showCheckboxes so readonly sessions get no checkbox cells, matching
309+
// the non-SP path and the column header (which omits the select-all checkbox).
310+
describe('Recommendations SP-group child-row checkbox gating (issue #135 + #869)', () => {
311+
// Two AWS savings-plans recs with distinct plan-type slugs but the same
312+
// scope -> two cell keys -> one SP group with 2 plan types.
313+
const spRecCompute = {
314+
...sampleRec,
315+
id: 'sp1',
316+
service: 'savings-plans-compute',
317+
resource_type: 'compute',
318+
savings: 300,
319+
};
320+
const spRecEc2 = {
321+
...sampleRec,
322+
id: 'sp2',
323+
service: 'savings-plans-ec2instance',
324+
resource_type: 'ec2instance',
325+
savings: 250,
326+
};
327+
328+
beforeEach(() => {
329+
jest.clearAllMocks();
330+
// expandedSpGroups is module-level state; reset so the SP group starts
331+
// collapsed in every test and the chevron click reliably expands it.
332+
resetExpandedCells();
333+
setupDom();
334+
(api.getRecommendations as jest.Mock).mockResolvedValue({
335+
summary: {},
336+
recommendations: [spRecCompute, spRecEc2],
337+
regions: [],
338+
});
339+
(state.getVisibleRecommendations as jest.Mock).mockReturnValue([spRecCompute, spRecEc2]);
340+
});
341+
342+
const expandSpGroup = (): HTMLTableElement => {
343+
const list = document.getElementById('recommendations-list');
344+
const table = list?.querySelector('table') as HTMLTableElement | null;
345+
expect(table).not.toBeNull();
346+
const chevron = table!.querySelector<HTMLButtonElement>('.rec-sp-group-chevron');
347+
expect(chevron).not.toBeNull();
348+
chevron!.click();
349+
return list!.querySelector('table') as HTMLTableElement;
350+
};
351+
352+
test('readonly role: expanded SP-group child variant rows have no checkbox-col', async () => {
353+
mockUser('readonly');
354+
await loadRecommendations();
355+
const table = expandSpGroup();
356+
357+
// The SP group expanded into per-plan-type child variant rows.
358+
const childRows = table.querySelectorAll('tr.rec-variant-row');
359+
expect(childRows.length).toBeGreaterThan(0);
360+
361+
// The bug this guards: child rows must not render a checkbox cell for
362+
// readonly sessions (showCheckboxes must be threaded into the nested call).
363+
for (const row of Array.from(childRows)) {
364+
expect(row.querySelectorAll('td.checkbox-col').length).toBe(0);
365+
expect(row.querySelectorAll('input[data-rec-id]').length).toBe(0);
366+
}
367+
});
368+
369+
test('admin role: expanded SP-group child variant rows retain checkbox-col', async () => {
370+
mockUser('admin');
371+
await loadRecommendations();
372+
const table = expandSpGroup();
373+
374+
const childRows = table.querySelectorAll('tr.rec-variant-row');
375+
expect(childRows.length).toBeGreaterThan(0);
376+
377+
for (const row of Array.from(childRows)) {
378+
expect(row.querySelectorAll('td.checkbox-col').length).toBe(1);
379+
}
380+
});
381+
});

‎frontend/src/recommendations.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2933,7 +2933,7 @@ function buildListMarkup(
29332933
for (const ck of childCellKeys) {
29342934
const childVariants = groups.get(ck)!;
29352935
if (childVariants.length === 1) {
2936-
rows.push(buildVariantRowMarkup(childVariants[0]!, selectedRecs, true, visibleCols));
2936+
rows.push(buildVariantRowMarkup(childVariants[0]!, selectedRecs, true, visibleCols, showCheckboxes));
29372937
continue;
29382938
}
29392939

@@ -2985,7 +2985,7 @@ function buildListMarkup(
29852985
if (isCellExpanded) {
29862986
const sortedChildVariants = sortVariantsInCell(childVariants);
29872987
for (const v of sortedChildVariants) {
2988-
rows.push(buildVariantRowMarkup(v, selectedRecs, true, visibleCols));
2988+
rows.push(buildVariantRowMarkup(v, selectedRecs, true, visibleCols, showCheckboxes));
29892989
}
29902990
}
29912991
}

0 commit comments

Comments
 (0)