This is an automated email from the ASF dual-hosted git repository.
diegopucci pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 66f1e1f714 refactor(bulk_select): Fix bulk select tagging issues for
users (#31631)
66f1e1f714 is described below
commit 66f1e1f714ee611cecf60b0e0aa2fbde149a3a4f
Author: Levis Mbote <[email protected]>
AuthorDate: Mon Jan 13 19:30:29 2025 +0300
refactor(bulk_select): Fix bulk select tagging issues for users (#31631)
---
.../src/features/tags/BulkTagModal.test.tsx | 114 +++++++++++++++++++++
.../src/features/tags/BulkTagModal.tsx | 2 +-
superset/commands/tag/create.py | 37 ++++---
3 files changed, 136 insertions(+), 17 deletions(-)
diff --git a/superset-frontend/src/features/tags/BulkTagModal.test.tsx
b/superset-frontend/src/features/tags/BulkTagModal.test.tsx
new file mode 100644
index 0000000000..d590dd551e
--- /dev/null
+++ b/superset-frontend/src/features/tags/BulkTagModal.test.tsx
@@ -0,0 +1,114 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information. The ASF licenses
this file
+ * to you under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License. You may
obtain
+ * a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
distributed
+ * under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR
CONDITIONS
+ * OF ANY KIND, either express or implied. See the License for the specific
language
+ * governing permissions and limitations under the License.
+ */
+
+import {
+ render,
+ screen,
+ fireEvent,
+ waitFor,
+} from 'spec/helpers/testing-library';
+import fetchMock from 'fetch-mock';
+import BulkTagModal from './BulkTagModal';
+
+const mockedProps = {
+ onHide: jest.fn(),
+ refreshData: jest.fn(),
+ addSuccessToast: jest.fn(),
+ addDangerToast: jest.fn(),
+ show: true,
+ selected: [
+ { original: { id: 1, name: 'Dashboard 1' } },
+ { original: { id: 2, name: 'Dashboard 2' } },
+ ],
+ resourceName: 'dashboard',
+};
+
+describe('BulkTagModal', () => {
+ afterEach(() => {
+ fetchMock.reset();
+ jest.clearAllMocks();
+ });
+
+ test('should render', () => {
+ const { container } = render(<BulkTagModal {...mockedProps} />);
+ expect(container).toBeInTheDocument();
+ });
+
+ test('renders the correct title and message', () => {
+ render(<BulkTagModal {...mockedProps} />);
+ expect(
+ screen.getByText(/you are adding tags to 2 dashboards/i),
+ ).toBeInTheDocument();
+ expect(screen.getByText('Bulk tag')).toBeInTheDocument();
+ });
+
+ test('renders tags input field', async () => {
+ render(<BulkTagModal {...mockedProps} />);
+ const tagsInput = await screen.findByRole('combobox', { name: /tags/i });
+ expect(tagsInput).toBeInTheDocument();
+ });
+
+ test('calls onHide when the Cancel button is clicked', () => {
+ render(<BulkTagModal {...mockedProps} />);
+ const cancelButton = screen.getByText('Cancel');
+ fireEvent.click(cancelButton);
+ expect(mockedProps.onHide).toHaveBeenCalled();
+ });
+
+ test('submits the selected tags and shows success toast', async () => {
+ fetchMock.post('glob:*/api/v1/tag/bulk_create', {
+ result: {
+ objects_tagged: [1, 2],
+ objects_skipped: [],
+ },
+ });
+
+ render(<BulkTagModal {...mockedProps} />);
+
+ const tagsInput = await screen.findByRole('combobox', { name: /tags/i });
+ fireEvent.change(tagsInput, { target: { value: 'Test Tag' } });
+ fireEvent.keyDown(tagsInput, { key: 'Enter', code: 'Enter' });
+
+ fireEvent.click(screen.getByText('Save'));
+
+ await waitFor(() => {
+ expect(mockedProps.addSuccessToast).toHaveBeenCalledWith(
+ 'Tagged 2 dashboards',
+ );
+ });
+
+ expect(mockedProps.refreshData).toHaveBeenCalled();
+ expect(mockedProps.onHide).toHaveBeenCalled();
+ });
+
+ test('handles API errors gracefully', async () => {
+ fetchMock.post('glob:*/api/v1/tag/bulk_create', 500);
+
+ render(<BulkTagModal {...mockedProps} />);
+
+ const tagsInput = await screen.findByRole('combobox', { name: /tags/i });
+ fireEvent.change(tagsInput, { target: { value: 'Test Tag' } });
+ fireEvent.keyDown(tagsInput, { key: 'Enter', code: 'Enter' });
+
+ fireEvent.click(screen.getByText('Save'));
+
+ await waitFor(() => {
+ expect(mockedProps.addDangerToast).toHaveBeenCalledWith(
+ 'Failed to tag items',
+ );
+ });
+ });
+});
diff --git a/superset-frontend/src/features/tags/BulkTagModal.tsx
b/superset-frontend/src/features/tags/BulkTagModal.tsx
index 32ccd4b0dd..3195461285 100644
--- a/superset-frontend/src/features/tags/BulkTagModal.tsx
+++ b/superset-frontend/src/features/tags/BulkTagModal.tsx
@@ -59,7 +59,7 @@ const BulkTagModal: FC<BulkTagModalProps> = ({
endpoint: `/api/v1/tag/bulk_create`,
jsonPayload: {
tags: tags.map(tag => ({
- name: tag.value,
+ name: tag.label,
objects_to_tag: selected.map(item => [
resourceName,
+item.original.id,
diff --git a/superset/commands/tag/create.py b/superset/commands/tag/create.py
index 775250dc81..b3788eb870 100644
--- a/superset/commands/tag/create.py
+++ b/superset/commands/tag/create.py
@@ -88,27 +88,32 @@ class CreateCustomTagWithRelationshipsCommand(CreateMixin,
BaseCommand):
def validate(self) -> None:
exceptions = []
objects_to_tag = set(self._properties.get("objects_to_tag", []))
+
for obj_type, obj_id in objects_to_tag:
object_type = to_object_type(obj_type)
# Validate object type
- for obj_type, obj_id in objects_to_tag:
- object_type = to_object_type(obj_type)
-
- if not object_type:
- exceptions.append(
- TagInvalidError(f"invalid object type {object_type}")
- )
- try:
- if model := to_object_model(object_type, obj_id): # type:
ignore
- security_manager.raise_for_ownership(model)
- except SupersetSecurityException:
- # skip the object if the user doesn't have access
- self._skipped_tagged_objects.add((obj_type, obj_id))
+ if not object_type:
+ exceptions.append(TagInvalidError(f"invalid object type
{object_type}"))
+ continue
- self._properties["objects_to_tag"] = (
- set(objects_to_tag) - self._skipped_tagged_objects
- )
+ try:
+ if model := to_object_model(object_type, obj_id):
+ try:
+ security_manager.raise_for_ownership(model)
+ except SupersetSecurityException:
+ if (
+ not model.created_by
+ or model.created_by !=
security_manager.current_user
+ ):
+ # skip the object if the user doesn't have access
+ self._skipped_tagged_objects.add((obj_type,
obj_id))
+ except Exception as e:
+ exceptions.append(TagInvalidError(str(e)))
+
+ self._properties["objects_to_tag"] = (
+ set(objects_to_tag) - self._skipped_tagged_objects
+ )
if exceptions:
raise TagInvalidError(exceptions=exceptions)
