Skip to content

feat(usage): show when an organisation is over its plan limit #8432

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

Open
talissoncosta wants to merge 6 commits into
base: main
Choose a base branch
from
+799 −276
Open

feat(usage): show when an organisation is over its plan limit #8432

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fa6d6bb
fix(usage): let a restricted organisation reach the usage page
talissoncosta Sep 3, 2026
ad90ce4
feat(usage): show the over-limit and restricted states
talissoncosta Sep 3, 2026
38f706c
docs(frontend): say what the import and component rules enforce
talissoncosta Sep 3, 2026
adff61a
refactor(usage): give the page's copy a module of its own
talissoncosta Sep 3, 2026
8f700fa
fix(usage): promise nothing to a restriction we cannot explain
talissoncosta Sep 3, 2026
adeb660
refactor(usage): gather the fixed sentences into one block
talissoncosta Sep 3, 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
4 changes: 2 additions & 2 deletions frontend/CLAUDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@

## Rules
1. **API Integration**: Use `npx ssg` CLI + check `../api` backend
2. **Imports**: Use `common/`, `components/`, `project/` (NO relative imports)
2. **Imports**: Use the `common/`, `components/`, `project/` aliases when the relative path would go up (`../`); use a relative path for same-folder or descendant imports. This is enforced by `@dword-design/import-alias/prefer-alias`, and `eslint --fix` will rewrite an alias back to relative inside its own alias root (e.g. `components/base/forms/X` becomes `./base/forms/X` in a file under `web/components/`).
3. **State**: Redux Toolkit + RTK Query, store in `common/store.ts`
4. **Feature Flags**: When user says "create a feature flag", you MUST: (1) Create it in Flagsmith using MCP tools (`mcp__flagsmith__create_feature`), (2) Implement code with `useFlags` hook. See `.claude/context/feature-flags/` for details
5. **Linting**: ALWAYS run `npx eslint --fix <file>` on any files you modify
6. **Type Enums**: Extract inline union types to named types (e.g., `type Status = 'A' | 'B'` instead of inline)
7. **NO FETCH**: NEVER use `fetch()` directly - ALWAYS use RTK Query mutations/queries (inject endpoints into services in `common/services/`), see api-integration context
8. **Component structure**: Each new component lives in its own folder with an `index.ts` barrel - `ComponentName/ComponentName.tsx`, co-located `ComponentName.scss`, any sub-components, and an `index.ts` that re-exports the default (and public types). Import via the folder (`components/.../ComponentName`), never the inner file. Keep files focused (~100 lines as a target); split by concern, not to hit a number. Data tables/constant maps are exempt.
8. **Component structure**: A component with nothing to keep beside it is a single `ComponentName.tsx`. It gets a folder once it has a co-located `ComponentName.scss`, sub-components, tests or hooks: `ComponentName/ComponentName.tsx` plus an `index.ts` re-exporting the default (and public types). Import via `components/.../ComponentName` either way, never the inner file, so promoting a file to a folder changes no imports. Keep files focused (~100 lines as a target); split by concern, not to hit a number. Data tables/constant maps are exempt.
Comment thread
coderabbitai[bot] marked this conversation as resolved.

## Key Files
- Store: `common/store.ts`
Expand Down
177 changes: 129 additions & 48 deletions frontend/documentation/components/UsageDashboard.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,27 @@
import { FC, useState } from 'react'
import type { Meta, StoryObj } from 'storybook'
import { UsageDashboard } from 'components/pages/usage'
import UsagePageLayout from 'components/pages/usage/components/UsagePageLayout'
import OverLimitBanner from 'components/pages/usage/components/OverLimitBanner'
import SectionHeading from 'components/pages/usage/components/SectionHeading'
import UsageBreakdown, {
useUsageBreakdown,
} from 'components/pages/usage/components/UsageBreakdown'
import UsageMeter from 'components/pages/usage/components/UsageMeter'
import UsageOverTime from 'components/pages/usage/components/UsageOverTime'
import {
allowanceWindow,
contributionNote,
overLimitNote,
planSectionCopy,
} from 'components/pages/usage/copy'
import { overLimitOf } from 'components/pages/usage/overLimit'
import {
allowanceWindow,
isBilledOnAPeriod,
isBillingPeriodSelected,
isChargedForOverages,
periodLabel,
periodsFor,
PeriodSelection,
planSectionCopy,
resolvePeriod,
showsContribution,
showsPlanCeiling,
Expand All @@ -22,6 +31,9 @@ import { BillingPeriod, PeriodOption } from 'common/types/requests'
import { PlanLimit } from 'components/shared/UsageBar/utils'
import { Subscription } from 'common/types/responses'
import { toUsageResponse, USAGE_SCENARIOS } from './fixtures/usage'
// The harness fakes the project select, so it never renders UsageFilters and
// would otherwise miss the width its stylesheet sets.
import 'components/pages/usage/components/UsageFilters/UsageFilters.scss'

const PROJECTS = [
'All Projects',
Expand Down Expand Up @@ -69,6 +81,7 @@ type HarnessProps = {
empty?: boolean
isLoading?: boolean
isError?: boolean
isRestricted?: boolean
}

/**
Expand All @@ -79,6 +92,7 @@ const UsagePage: FC<HarnessProps> = ({
empty,
isError,
isLoading,
isRestricted,
limit,
scale = 1,
subscription,
Expand All @@ -99,10 +113,20 @@ const UsagePage: FC<HarnessProps> = ({
scenarioFor(billingPeriod, !!empty, isFreePlan),
share * scale,
)
const allowanceTotal = toUsageResponse(
const allowance = toUsageResponse(
scenarioFor(allowanceWindow(basis), !!empty, isFreePlan),
scale,
).totals.total
)
const allowanceTotal = allowance.totals.total
const exceeded = overLimitOf(allowanceTotal, limit, allowance)

const contribution = showsContribution(
basis,
billingPeriod,
filtered ? 1 : undefined,
)
? contributionNote(project, scoped.totals.total, allowanceTotal)
: undefined

// The note needs the organisation over the period on screen, not over the
// allowance window, or a project can read as more than all of it.
Expand All @@ -114,53 +138,79 @@ const UsagePage: FC<HarnessProps> = ({
)}`

return (
<UsageDashboard
breakdown={
<UsageBreakdown
{...breakdown}
onChangeDimension={setDimension}
scope={scope}
/>
}
data={scoped}
filters={
<Row className='gap-2'>
<div style={{ minWidth: 210 }}>
<Select
aria-label='Period'
onChange={(option: PeriodOption) => setChosenPeriod(option.value)}
options={periods}
value={periods.find((option) => option.value === billingPeriod)}
/>
</div>
<div style={{ minWidth: 210 }}>
<Select
aria-label='Project'
onChange={(option: { value: string }) => setProject(option.value)}
options={PROJECTS.map((name) => ({ label: name, value: name }))}
value={{ label: project, value: project }}
/>
</div>
</Row>
}
hasBillingPeriod={isBillingPeriodSelected(billingPeriod)}
<UsagePageLayout
isError={isError}
isLoading={isLoading}
limit={limit}
meterNote={
showsContribution(basis, billingPeriod, filtered ? 1 : undefined)
? contributionNote(project, scoped.totals.total, allowanceTotal)
: undefined
alert={
(exceeded || isRestricted) && (
<OverLimitBanner
over={exceeded}
basis={basis}
canUpgrade
isRestricted={isRestricted}
mayBeCharged={
isBilledOnAPeriod(basis) && isChargedForOverages(subscription)
}
/>
)
}
// Nothing to refetch here; passed so FailedToLoad renders its button.
onRetry={() => {}}
periodLabel={periodLabel(periods, billingPeriod)}
planCopy={planSectionCopy(basis, limit)}
showPlanCeiling={showsPlanCeiling(
billingPeriod,
filtered ? 1 : undefined,
)}
total={allowanceTotal}
/>
>
<SectionHeading {...planSectionCopy(basis, limit)} />

<UsageMeter
total={allowanceTotal}
limit={limit}
note={exceeded ? overLimitNote(exceeded) : contribution}
/>

<SectionHeading
title='Explore usage'
hint='Narrow the chart and the breakdown by period or project.'
action={
<Row className='gap-2'>
<div className='usage-filters__field'>
Comment thread
talissoncosta marked this conversation as resolved.
<Select
aria-label='Period'
onChange={(option: PeriodOption) =>
setChosenPeriod(option.value)
}
options={periods}
value={periods.find((option) => option.value === billingPeriod)}
/>
</div>
<div className='usage-filters__field'>
<Select
aria-label='Project'
onChange={(option: { value: string }) =>
setProject(option.value)
}
options={PROJECTS.map((name) => ({ label: name, value: name }))}
value={{ label: project, value: project }}
/>
</div>
</Row>
}
/>

<UsageOverTime
data={scoped}
limit={
showsPlanCeiling(billingPeriod, filtered ? 1 : undefined)
? limit
: undefined
}
isBillingPeriod={isBillingPeriodSelected(billingPeriod)}
periodLabel={periodLabel(periods, billingPeriod)}
/>

<UsageBreakdown
{...breakdown}
onChangeDimension={setDimension}
scope={scope}
/>
</UsagePageLayout>
)
}

Expand All @@ -183,10 +233,30 @@ export const PaidApproachingTheLimit: Story = {
args: { limit: 1400000, subscription: billed },
}

// Billed on a term, so the banner mentions charges.
export const PaidOverTheLimit: Story = {
args: { limit: 900000, subscription: billed },
}

// Only free plans are ever restricted, and this is where they are sent.
export const FreeAndRestricted: Story = {
args: {
isRestricted: true,
limit: 50000,
subscription: subscriptionOf({ plan: 'free' }),
},
}

// The block outlives the overage, and support can set it by hand, so with
// nothing in evidence the banner promises nothing.
export const RestrictedWithNoOverageInEvidence: Story = {
args: {
isRestricted: true,
limit: 5000000,
subscription: subscriptionOf({ plan: 'free' }),
},
}

export const FreeOnARollingWindow: Story = {
args: { limit: 50000, subscription: subscriptionOf({ plan: 'free' }) },
}
Expand All @@ -203,6 +273,17 @@ export const EnterpriseWithoutABillingPeriod: Story = {
},
}

// Invoiced outside Chargebee, so no charge line.
export const EnterpriseOverTheLimit: Story = {
args: {
limit: 1000000,
subscription: subscriptionOf({
payment_method: 'XERO',
plan: 'enterprise',
}),
},
}

// On Chargebee, but no period has arrived. Reads differently from invoiced,
// because this one may resolve itself.
export const ChargebeeWithoutAPeriodYet: Story = {
Expand Down
2 changes: 2 additions & 0 deletions frontend/jest.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ module.exports = {
'^common/(.*)$': '<rootDir>/common/$1',
'^components/(.*)$': '<rootDir>/web/components/$1',
'^project/(.*)$': '<rootDir>/web/project/$1',
// webpack resolves this one too; without it jest cannot follow the app.
'^web/(.*)$': '<rootDir>/web/$1',
},
preset: 'ts-jest',
roots: ['<rootDir>'],
Expand Down
28 changes: 28 additions & 0 deletions frontend/web/__tests__/routePaths.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { isAllowedWhileBlocked } from 'web/routePaths'

// App renders <Blocked /> wherever this is false, so a wrong answer either
// locks a blocked organisation out, or lets it back in.
describe('isAllowedWhileBlocked', () => {
it.each`
pathname | allowed
${'/organisation/7528/usage'} | ${true}
${'/organisations'} | ${true}
${'/organisation/7528/projects'} | ${false}
${'/organisation/7528/settings'} | ${false}
${'/organisation-settings'} | ${false}
${'/project/1/environment/abc/features'} | ${false}
${'/account'} | ${false}
`(
'$pathname is reachable while blocked: $allowed',
({ allowed, pathname }) => {
expect(isAllowedWhileBlocked(pathname)).toBe(allowed)
},
)

// Allowed by pattern, not by prefix.
it('does not open anything nested under the usage page', () => {
expect(isAllowedWhileBlocked('/organisation/7528/usage/breakdown')).toBe(
false,
)
})
})
7 changes: 4 additions & 3 deletions frontend/web/components/App.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ import Announcement from './Announcement'
import { getBuildVersion } from 'common/services/useBuildVersion'
import AccountProvider from 'common/providers/AccountProvider'
import Nav from './navigation/Nav'
import { isAllowedWhileBlocked } from 'web/routePaths'
import 'project/darkMode'

const App = class extends Component {
static propTypes = {
children: propTypes.element.isRequired,
Expand Down Expand Up @@ -271,9 +273,8 @@ const App = class extends Component {
const environmentId = this.getEnvironmentId(this.props)

if (
AccountStore.getOrganisation() &&
AccountStore.getOrganisation().block_access_to_admin &&
pathname !== '/organisations'
AccountStore.getOrganisation()?.block_access_to_admin &&
!isAllowedWhileBlocked(pathname)

@themis-blindfold themis-blindfold Bot Sep 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Major · ⚡ Quick win

Cover the restricted-route exception.

Observed: the only added tests are pure overLimit/utility tests; none exercises isAllowedWhileBlocked or the App guard. Predicted: a route change could again lock restricted organisations out of usage, or allow another blocked route, with CI still green. Add a routing or component test that asserts the usage URL renders for a blocked organisation and a neighbouring protected URL renders Blocked.

@talissoncosta talissoncosta Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not adding it here: isAllowedWhileBlocked reads web/routes, which imports App, so a test importing it pulls in the whole app. That cycle already broke the boot in this PR once. Fixing it properly, moving the route map into a component-free module, is a follow-up and the test comes with it.

@themis-blindfold themis-blindfold Bot Sep 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This remains open. The changed guard imports routes from a module that imports App back (frontend/web/components/App.js:32, frontend/web/routes.js:4); extracting the route paths in this PR removes that cycle and allows the required blocked-usage versus protected-route regression test. A follow-up cannot cover this PR's behaviour change.

@talissoncosta talissoncosta Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tracked in #8456, which moves the route table into a module that imports no components and brings the test with it. Kept separate because it touches the file every page depends on. Not blocking this one.

@themis-blindfold themis-blindfold Bot Sep 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not fixed. This PR still changes the blocked-route guard at frontend/web/components/App.js:283 using the circular route import at frontend/web/components/App.js:33; its added tests do not exercise either the usage exception or a neighbouring blocked route. #8456 cannot provide regression coverage for this PR's behaviour change. Extract the paths and add the test here, or revert the exception.

@themis-blindfold themis-blindfold Bot Sep 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not fixed: routePaths.test.ts:19 calls only the helper. It never renders the changed blocked-organisation guard at App.js:275, so a regression in that guard can still pass. Add the blocked usage versus protected-route render test.

@talissoncosta talissoncosta Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't render it here: testEnvironment is node and the repo has no RTL or jsdom, nothing renders a component in a test today. The helper is covered by eight cases; what's left uncovered is one && in App. Standing up component tests is its own piece of work.

@themis-blindfold themis-blindfold Bot Sep 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not fixed. routePaths.test.ts:18 only calls the helper; it never executes the blocked-organisation branch in App.js:275. A node unit environment does not require adding a component-test stack: this repo already runs browser tests via Playwright (package.json:15). Add a restricted-organisation browser test for the usage route and a protected route.

@talissoncosta talissoncosta Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right and I was wrong to say it could not be rendered: the repo runs Playwright with a real e2e suite. No test there sets organisation-level flags yet, so it needs new setup rather than a new stack. Tracking it rather than doing it in this PR.

@themis-blindfold themis-blindfold Bot Sep 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not fixed. routePaths.test.ts:18 only exercises the helper; the blocked-organisation render branch at App.js:275 remains untested. Add the restricted-organisation browser coverage here or revert the exception.

) {
return <Blocked />
}
Expand Down
5 changes: 5 additions & 0 deletions frontend/web/components/BlockedOrgInfo.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ export default function BlockedOrgInfo() {
<div className='text-nowrap'>
<div>Organisation name: {AccountStore.getOrganisation().name}</div>
<div>Organisation ID: {AccountStore.getOrganisation().id}</div>
<div>
<a href={`/organisation/${AccountStore.getOrganisation().id}/usage`}>
See your usage
</a>
</div>
<div>
<a href='/organisations'>Switch to a different organisation</a>
</div>
Expand Down
Loading
Loading