Skip to content

fix: keep cost attribution labels grouped after saving a check - #1796

Open
g3john wants to merge 1 commit into
mainfrom
fix/cost-attribution-labels-regroup-after-save
Open

fix: keep cost attribution labels grouped after saving a check#1796
g3john wants to merge 1 commit into
mainfrom
fix/cost-attribution-labels-regroup-after-save

Conversation

@g3john

@g3john g3john commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Addresses: #1791

Problem

When a check has both cost attribution labels and custom labels, saving the check moves the cost attribution labels into the Custom labels section. The cost attribution rows are left blank, as though the values had been cleared.

Nothing is actually lost. The check is saved correctly and the labels return to their proper sections when you navigate away and come back, but it looks like the save has just discarded your cost attribution values.

Solution

Sorting the labels into the two sections was a one-shot, only re-running when the tenant's cost attribution names changed. It now also runs whenever the cost attribution rows are out of sync with the names they should contain, which is the state a form reset leaves behind. It keys off the cost attribution rows rather than scanning the custom labels for cost-attribution-looking names

Before

Screen.Recording.2026-08-10.at.3.47.49.PM.mov

After

Screen.Recording.2026-08-10.at.3.47.19.PM.mov

Saving a check refetches it and resets the form with every label in a
single flat list, which the one-shot split effect never re-grouped
because the tenant's cost attribution label names had not changed.

Reconcile whenever the cost attribution rows fall out of sync with the
name list, which is the state a reset leaves behind.

Co-authored-by: Cursor <cursoragent@cursor.com>
@g3john
g3john requested a review from a team as a code owner August 10, 2026 19:36
@g3john
g3john requested review from VikaCep and ckbedwell August 10, 2026 19:37
@cursor

cursor Bot commented Aug 10, 2026

Copy link
Copy Markdown

Current version of PR was reviewed by /review-bugbot on Aug 10, 15:14 EDT. It flagged 0 findings.

Bugbot on commit feb09f8 is skipped.

@github-actions github-actions Bot added the fix A fix applied to the application. label Aug 10, 2026
@github-actions

Copy link
Copy Markdown

Script size changes

Name +/- Main This PR Outcome
[702.js] = 6,717.24 kB 6,717.24 kB
[87.js] +0.02% 3,636.14 kB 3,636.98 kB
[469.js] = 506.85 kB 506.85 kB
[datasource/module.js] = 119.68 kB 119.68 kB
[9.js] = 99.10 kB 99.10 kB
[692.js] = 67.57 kB 67.57 kB
[398.js] = 48.57 kB 48.57 kB
[module.js] = 35.04 kB 35.04 kB
[294.js] = 22.06 kB 22.06 kB
[466.js] = 5.15 kB 5.15 kB
[131.js] = 5.00 kB 5.00 kB
[845.js] = 3.25 kB 3.25 kB
[295.js] = 2.79 kB 2.79 kB

Totals

Name +/- Main This PR Outcome
[Scripts] = 11,268.44 kB 11,269.27 kB
[Non-script Assets] = 1,469.61 kB 1,469.61 kB
[All] = 12,738.04 kB 12,738.88 kB

Generated by 🚫 dangerJS against feb09f8

Comment on lines +37 to +65
const RESET_BUTTON_TEXT = 'Simulate check refetch';

// Saving a check refetches it, which re-seeds `labels` with every label on the check and clears
// `calLabels`. This harness reproduces that reset so the regrouping can be asserted.
function renderWithRefetchReset(calNames: string[], labels: Label[]) {
function Harness() {
const formMethods = useForm({ defaultValues: { labels, calLabels: [] }, mode: 'onChange' });

return (
<FormProvider {...formMethods}>
<form>
<GenericLabelContent description="Test description" calNames={calNames} />
<button type="button" onClick={() => formMethods.reset({ labels, calLabels: [] })}>
{RESET_BUTTON_TEXT}
</button>
</form>
</FormProvider>
);
}

render(
<OpenFeatureTestProvider domain={SM_OPEN_FEATURE_DOMAIN} flagValueMap={getTestFlagValues()}>
<Harness />
</OpenFeatureTestProvider>
);

return userEvent.setup();
}

@g3john g3john Aug 10, 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 sure if its worth doing this just to have a test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix A fix applied to the application.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant