Skip to content

feat(plans): surface legacy no-account plans as Unassigned (closes #973) #994

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
cristim merged 2 commits into from
Jun 6, 2026
Merged

feat(plans): surface legacy no-account plans as Unassigned (closes #973) #994

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
575e1d1
feat(plans): surface legacy no-account plans as Unassigned (closes #973)
cristim Jun 5, 2026
dbe378f
test(plans): real DB test for Unassigned bucket query (refs #973)
cristim Jun 5, 2026
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
80 changes: 80 additions & 0 deletions frontend/src/__tests__/plans.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,86 @@ describe('Plans Module', () => {
const list = document.getElementById('plans-list');
expect(list?.innerHTML).toContain('Null Services Plan');
});

// Regression guard for issue #973: a plan with unassigned=true must
// appear in the "Unassigned" section, and an assigned plan must NOT
// appear there. Before the fix the backend silently dropped zero-account
// plans from account-filtered responses, so no "Unassigned" section was
// ever rendered.
test('unassigned plan renders under Unassigned section, assigned plan does not (issue #973)', async () => {
(api.getPlans as jest.Mock).mockResolvedValue({
plans: [
{
id: 'assigned-plan',
name: 'Assigned Plan',
enabled: true,
auto_purchase: false,
unassigned: false,
services: {
'ec2': { provider: 'aws', service: 'ec2', enabled: true, term: 1, payment: 'no-upfront', coverage: 80 }
},
ramp_schedule: { type: 'immediate', percent_per_step: 100, step_interval_days: 0, current_step: 0, total_steps: 1 }
},
{
id: 'legacy-plan',
name: 'Legacy Unscoped Plan',
enabled: true,
auto_purchase: false,
unassigned: true,
services: {
'rds': { provider: 'aws', service: 'rds', enabled: true, term: 3, payment: 'no-upfront', coverage: 70 }
},
ramp_schedule: { type: 'immediate', percent_per_step: 100, step_interval_days: 0, current_step: 0, total_steps: 1 }
}
]
});
(api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] });

await loadPlans();

const list = document.getElementById('plans-list');
const html = list?.innerHTML ?? '';

// Both plans are rendered.
expect(html).toContain('Assigned Plan');
expect(html).toContain('Legacy Unscoped Plan');

// The "Unassigned" section header is present.
expect(html).toContain('unassigned-plans-header');
expect(html).toContain('Unassigned');

// The unassigned section header must appear AFTER the assigned plan card
// (assigned plans come first, unassigned section is appended after).
const assignedPos = html.indexOf('Assigned Plan');
const unassignedHeaderPos = html.indexOf('unassigned-plans-header');
const legacyPos = html.indexOf('Legacy Unscoped Plan');
expect(assignedPos).toBeLessThan(unassignedHeaderPos);
expect(unassignedHeaderPos).toBeLessThan(legacyPos);
});

test('no Unassigned section when all plans are assigned', async () => {
(api.getPlans as jest.Mock).mockResolvedValue({
plans: [
{
id: 'plan-a',
name: 'Normal Plan',
enabled: true,
auto_purchase: false,
unassigned: false,
services: {
'ec2': { provider: 'aws', service: 'ec2', enabled: true, term: 1, payment: 'no-upfront', coverage: 80 }
},
ramp_schedule: { type: 'immediate', percent_per_step: 100, step_interval_days: 0, current_step: 0, total_steps: 1 }
}
]
});
(api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] });

await loadPlans();

const list = document.getElementById('plans-list');
expect(list?.innerHTML).not.toContain('unassigned-plans-header');
});
});

describe('loadPlannedPurchases', () => {
Expand Down
4 changes: 4 additions & 0 deletions frontend/src/api/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,10 @@ export interface Plan {
updated_at: string;
next_execution_date?: string;
last_execution_date?: string;
// unassigned is true for legacy plans that have zero plan_accounts rows.
// Such plans are surfaced under an "Unassigned" section in the Plans UI
// so operators can find and re-scope them (issue #973).
unassigned?: boolean;
}

export interface CreatePlanRequest {
Expand Down
192 changes: 118 additions & 74 deletions frontend/src/plans.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -789,6 +789,10 @@ interface BackendPlan {
total_steps: number;
};
next_execution_date?: string;
// unassigned is true for legacy plans that have zero plan_accounts rows
// (issue #973). The backend sets this flag when an account filter is
// active so the frontend can bucket them under an "Unassigned" section.
unassigned?: boolean;
}

// Pretty label for a service slug used inside the plan card.
Expand Down Expand Up @@ -878,6 +882,85 @@ async function loadPlanAccountNames(planId: string, cardEl: Element): Promise<vo
}
}

// renderPlanCard generates the HTML for a single plan card.
// When the plan is unassigned (no plan_accounts rows) account-scoped
// actions that require an account (Add Purchases, Edit) are suppressed
// — only History and Delete remain — and a read-only "Unassigned" badge
// is shown so operators can identify and re-scope the plan.
function renderPlanCard(plan: BackendPlan, canManagePlan: boolean, canDeletePlan: boolean): string {
const info = extractPlanInfo(plan);
const status = getStatusBadge(plan.enabled, plan.auto_purchase);
const rampSchedule = plan.ramp_schedule || { type: 'immediate', current_step: 0, total_steps: 1 };
const overdue = isPlanOverdue(plan);
// Hide the stale next_execution_date for disabled plans — keeping it
// visible implies the plan will still run on that date, which it won't.
const showNextDate = Boolean(plan.next_execution_date) && plan.enabled;
const overdueBadge = overdue && plan.enabled
? '<span class="status-badge badge-danger" title="Next purchase date is in the past">Overdue</span>'
: '';
// Read-only mode: unassigned plans cannot be purchased against until an
// operator re-assigns them to at least one account.
const isUnassigned = Boolean(plan.unassigned);

return `
<div class="plan-card">
<div class="plan-header">
<h3>${escapeHtml(plan.name)}</h3>
<div class="plan-status">
<span class="status-badge ${status.class}">${status.label}</span>
${overdueBadge}
${canManagePlan && !isUnassigned ? `
<label class="toggle-label">
<input type="checkbox" data-action="toggle-plan" data-id="${plan.id}" ${plan.enabled ? 'checked' : ''}>
<span class="slider"></span>
</label>
` : ''}
</div>
</div>
<div class="plan-body">
<div class="plan-details">
<div class="plan-detail">
<span class="plan-detail-label">Provider</span>
<span class="plan-detail-value"><span class="provider-badge ${info.provider}">${info.provider.toUpperCase()}</span></span>
</div>
<div class="plan-detail">
<span class="plan-detail-label">Service</span>
<span class="plan-detail-value">${escapeHtml(info.service)}</span>
</div>
<div class="plan-detail">
<span class="plan-detail-label">Term</span>
<span class="plan-detail-value">${formatTerm(info.term)}</span>
</div>
<div class="plan-detail">
<span class="plan-detail-label">Coverage</span>
<span class="plan-detail-value">${info.coverage}%</span>
</div>
<div class="plan-detail">
<span class="plan-detail-label">Ramp Schedule</span>
<span class="plan-detail-value">${formatBackendRampSchedule(rampSchedule)}</span>
</div>
<div class="plan-detail">
<span class="plan-detail-label">Progress</span>
<span class="plan-detail-value">${rampSchedule.current_step || 0}/${rampSchedule.total_steps || 1} steps</span>
</div>
${showNextDate ? `
<div class="plan-detail">
<span class="plan-detail-label">Next Purchase</span>
<span class="plan-detail-value">${formatDate(plan.next_execution_date || '')}</span>
</div>
` : ''}
</div>
<div class="plan-actions">
${canManagePlan && !isUnassigned ? `<button data-action="add-purchases" data-id="${plan.id}" data-name="${escapeHtml(plan.name)}" class="primary">Add Purchases</button>` : ''}
${canManagePlan && !isUnassigned ? `<button data-action="edit-plan" data-id="${plan.id}">Edit</button>` : ''}
<button data-action="view-history" data-id="${plan.id}" class="secondary">History</button>
${canDeletePlan ? `<button data-action="delete-plan" data-id="${plan.id}" class="danger">Delete</button>` : ''}
</div>
</div>
</div>
`;
}

function renderPlans(plans: LocalPlan[]): void {
const container = document.getElementById('plans-list');
if (!container) return;
Expand All @@ -893,83 +976,44 @@ function renderPlans(plans: LocalPlan[]): void {
const canManagePlan = canAccess('update', 'plans');
const canDeletePlan = canAccess('delete', 'plans');

container.innerHTML = plans.map(rawPlan => {
// Cast to BackendPlan to handle the actual API response format
const plan = rawPlan as unknown as BackendPlan;
const info = extractPlanInfo(plan);
const status = getStatusBadge(plan.enabled, plan.auto_purchase);
const rampSchedule = plan.ramp_schedule || { type: 'immediate', current_step: 0, total_steps: 1 };
const overdue = isPlanOverdue(plan);
// Hide the stale next_execution_date for disabled plans — keeping it
// visible implies the plan will still run on that date, which it won't.
const showNextDate = Boolean(plan.next_execution_date) && plan.enabled;
const overdueBadge = overdue && plan.enabled
? '<span class="status-badge badge-danger" title="Next purchase date is in the past">Overdue</span>'
: '';

return `
<div class="plan-card">
<div class="plan-header">
<h3>${escapeHtml(plan.name)}</h3>
<div class="plan-status">
<span class="status-badge ${status.class}">${status.label}</span>
${overdueBadge}
${canManagePlan ? `
<label class="toggle-label">
<input type="checkbox" data-action="toggle-plan" data-id="${plan.id}" ${plan.enabled ? 'checked' : ''}>
<span class="slider"></span>
</label>
` : ''}
</div>
</div>
<div class="plan-body">
<div class="plan-details">
<div class="plan-detail">
<span class="plan-detail-label">Provider</span>
<span class="plan-detail-value"><span class="provider-badge ${info.provider}">${info.provider.toUpperCase()}</span></span>
</div>
<div class="plan-detail">
<span class="plan-detail-label">Service</span>
<span class="plan-detail-value">${escapeHtml(info.service)}</span>
</div>
<div class="plan-detail">
<span class="plan-detail-label">Term</span>
<span class="plan-detail-value">${formatTerm(info.term)}</span>
</div>
<div class="plan-detail">
<span class="plan-detail-label">Coverage</span>
<span class="plan-detail-value">${info.coverage}%</span>
</div>
<div class="plan-detail">
<span class="plan-detail-label">Ramp Schedule</span>
<span class="plan-detail-value">${formatBackendRampSchedule(rampSchedule)}</span>
</div>
<div class="plan-detail">
<span class="plan-detail-label">Progress</span>
<span class="plan-detail-value">${rampSchedule.current_step || 0}/${rampSchedule.total_steps || 1} steps</span>
</div>
${showNextDate ? `
<div class="plan-detail">
<span class="plan-detail-label">Next Purchase</span>
<span class="plan-detail-value">${formatDate(plan.next_execution_date || '')}</span>
</div>
` : ''}
</div>
<div class="plan-actions">
${canManagePlan ? `<button data-action="add-purchases" data-id="${plan.id}" data-name="${escapeHtml(plan.name)}" class="primary">Add Purchases</button>` : ''}
${canManagePlan ? `<button data-action="edit-plan" data-id="${plan.id}">Edit</button>` : ''}
<button data-action="view-history" data-id="${plan.id}" class="secondary">History</button>
${canDeletePlan ? `<button data-action="delete-plan" data-id="${plan.id}" class="danger">Delete</button>` : ''}
</div>
</div>
// Issue #973: split plans into assigned (have plan_accounts rows) and
// unassigned (legacy plans with zero plan_accounts rows, flagged by the
// backend). Unassigned plans are rendered under a separate read-only
// section so operators can discover and re-scope them.
const assignedPlans = plans.filter(p => !(p as unknown as BackendPlan).unassigned);
const unassignedPlans = plans.filter(p => (p as unknown as BackendPlan).unassigned);

const assignedHtml = assignedPlans.map(rawPlan =>
renderPlanCard(rawPlan as unknown as BackendPlan, canManagePlan, canDeletePlan)
).join('');

let unassignedHtml = '';
if (unassignedPlans.length > 0) {
const cards = unassignedPlans.map(rawPlan =>
renderPlanCard(rawPlan as unknown as BackendPlan, canManagePlan, canDeletePlan)
).join('');
unassignedHtml = `
<div class="plans-section-header unassigned-plans-header">
<h4>Unassigned</h4>
<span class="plans-section-description">These legacy plans have no associated accounts and cannot be purchased against. Assign accounts or delete them.</span>
</div>
${cards}
`;
}).join('');
}

// Asynchronously populate account names per plan
container.querySelectorAll<HTMLElement>('.plan-card').forEach((card, idx) => {
const plan = plans[idx] as unknown as BackendPlan;
if (plan.id) void loadPlanAccountNames(plan.id, card);
container.innerHTML = assignedHtml + unassignedHtml;

// Asynchronously populate account names per assigned plan card.
// Unassigned plans intentionally skip this: they have no plan_accounts
// rows so the API call would return an empty list.
container.querySelectorAll<HTMLElement>('.plan-card').forEach((card) => {
const planId = card.querySelector<HTMLElement>('[data-id]')?.dataset['id'];
// Find the matching plan to check unassigned status.
const allPlans = [...assignedPlans, ...unassignedPlans];
const matchedPlan = allPlans.find(p => (p as unknown as BackendPlan).id === planId) as unknown as BackendPlan | undefined;
if (planId && matchedPlan && !matchedPlan.unassigned) {
void loadPlanAccountNames(planId, card);
}
});

// Add event listeners
Expand Down
63 changes: 63 additions & 0 deletions internal/api/handler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1341,6 +1341,69 @@ func TestHandler_HandleRequest_ListPlans_Error(t *testing.T) {
assert.Equal(t, 500, resp.StatusCode)
}

// TestHandler_HandleRequest_ListPlans_UnassignedFlagged is the regression
// guard for issue #973: a plan with zero plan_accounts rows (legacy
// "universal" plan) must appear in the response flagged as unassigned=true
// when an account filter is active. Before the fix such plans were silently
// dropped by the INNER JOIN on plan_accounts.
func TestHandler_HandleRequest_ListPlans_UnassignedFlagged(t *testing.T) {
ctx := context.Background()
mockStore := new(MockConfigStore)
mockAuth := new(MockAuthService)

adminSession := &Session{UserID: "admin-id", Email: "admin@example.com"}
mockAuth.On("ValidateSession", ctx, "test-token").Return(adminSession, nil)
mockAuth.grantAdmin()

// Simulate the store returning one assigned plan and one zero-account
// (unassigned) legacy plan.
assigned := config.PurchasePlan{ID: "11111111-1111-1111-1111-111111111111", Name: "Assigned Plan", Unassigned: false}
legacy := config.PurchasePlan{ID: "22222222-2222-2222-2222-222222222222", Name: "Legacy Plan", Unassigned: true}
mockStore.On("ListPurchasePlans", mock.Anything, mock.Anything).Return([]config.PurchasePlan{assigned, legacy}, nil)

handler := &Handler{config: mockStore, auth: mockAuth, apiKey: "test-key"}

req := &events.LambdaFunctionURLRequest{
Headers: map[string]string{
"X-API-Key": "test-key",
"Authorization": "Bearer test-token",
},
QueryStringParameters: map[string]string{
"account_ids": "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
},
RequestContext: events.LambdaFunctionURLRequestContext{
HTTP: events.LambdaFunctionURLRequestContextHTTPDescription{
Method: "GET",
Path: "/api/plans",
},
},
}

resp, err := handler.HandleRequest(ctx, req)
require.NoError(t, err)
assert.Equal(t, 200, resp.StatusCode)

var body PlansResponse
require.NoError(t, json.Unmarshal([]byte(resp.Body), &body))
require.Len(t, body.Plans, 2)

// Find plans by ID to avoid order dependence.
plansByID := make(map[string]config.PurchasePlan, 2)
for _, p := range body.Plans {
plansByID[p.ID] = p
}

// Assigned plan must not carry the unassigned flag.
ap, ok := plansByID["11111111-1111-1111-1111-111111111111"]
require.True(t, ok, "assigned plan missing from response")
assert.False(t, ap.Unassigned, "assigned plan must have unassigned=false")

// Legacy zero-account plan must carry the unassigned flag.
lp, ok := plansByID["22222222-2222-2222-2222-222222222222"]
require.True(t, ok, "legacy unassigned plan missing from response")
assert.True(t, lp.Unassigned, "zero-account plan must have unassigned=true")
}

// Test for updateConfig error case - invalid JSON returns 500 (not 400)
func TestHandler_HandleRequest_UpdateConfig_InvalidJSON(t *testing.T) {
ctx := context.Background()
Expand Down
Loading
Loading