Compare commits

...

1 Commits

Author SHA1 Message Date
Evan
7bfe066a6b fix(explore): allow chart owners to overwrite when owners are objects
The Save chart modal gates "Save (Overwrite)" on canOverwriteSlice(),
which checked slice.owners.includes(user.userId). The Slice type declares
owners as { id: number }[], and the Explore bootstrap can hydrate owners in
object form, so includes() against a numeric userId never matched. A chart
owner who wasn't also an admin was therefore forced into "Save as...",
which is the path that produces the duplicate/stale-chart behavior reported
in the issue.

Normalize owner identity to the numeric id before comparing so owners are
recognized whether they arrive as plain ids or as objects.

Fixes #38911

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-23 20:47:40 -07:00
2 changed files with 41 additions and 6 deletions

View File

@@ -335,6 +335,27 @@ test('enables overwrite option for admin non-owner', () => {
expect(getByRole('radio', { name: 'Save (Overwrite)' })).toBeEnabled();
});
test('enables overwrite option for owner when owners are objects', () => {
// The Slice type declares `owners: { id: number }[]`, and the explore
// bootstrap can deliver owners in object form. A chart owner must still be
// able to overwrite their own chart in that case.
const { getByRole } = setup(
{},
mockStore({
...initialState,
explore: {
...initialState.explore,
slice: {
...initialState.explore.slice,
owners: [{ id: 1 }],
},
},
user: { userId: 1 },
}),
);
expect(getByRole('radio', { name: 'Save (Overwrite)' })).toBeEnabled();
});
test('updates slice name and selected dashboard', async () => {
const dashboardId = mockEvent.value;
const saveDataset = jest.fn().mockResolvedValue(undefined);

View File

@@ -132,15 +132,29 @@ class SaveModal extends Component<SaveModalProps, SaveModalState> {
return typeof dashboard?.value === 'string';
}
canOverwriteSlice(): boolean {
return (
(this.props.can_overwrite ||
isUserAdmin(this.props.user) ||
this.props.slice?.owners?.includes(this.props.user.userId)) &&
!this.props.slice?.is_managed_externally
isCurrentUserOwner(): boolean {
const userId = this.props.user?.userId;
if (userId === undefined) {
return false;
}
// Owners can arrive either as plain ids (number) or as objects depending on
// where the slice was hydrated from (bootstrap vs. SLICE_UPDATED reducer),
// so normalize to the numeric id before comparing.
return Boolean(
this.props.slice?.owners?.some((owner: number | { id?: number }) =>
typeof owner === 'number' ? owner === userId : owner?.id === userId,
),
);
}
canOverwriteSlice(): boolean {
const canEdit =
this.props.can_overwrite ||
isUserAdmin(this.props.user) ||
this.isCurrentUserOwner();
return canEdit && !this.props.slice?.is_managed_externally;
}
async componentDidMount() {
let { dashboardId } = this.props;
if (!dashboardId) {