mikebridge commented on code in PR #43887:
URL: https://github.com/apache/superset/pull/43887#discussion_r4043721078
##########
superset-frontend/src/pages/DatabaseList/index.tsx:
##########
@@ -798,13 +823,13 @@ function DatabaseList({
const isSemanticLayer = original.source_type === 'semantic_layer';
if (isSemanticLayer) {
- if (!canEdit && !canDelete) return null;
+ if (!canWriteLayer) return null;
const isLoadingDependents =
slDeletePreview?.status === 'loading' &&
slDeletePreview.item.uuid === original.uuid;
return (
<div className="actions">
- {canDelete && (
+ {canWriteLayer && (
Review Comment:
Addressed in d5b760fc253301706e52c47ef7fb05dc551c9770. Removed the redundant
Delete wrapper; the semantic row still returns early without canWriteLayer. The
loading/confirmation behavior is unchanged.
##########
superset-frontend/src/pages/DatabaseList/index.tsx:
##########
@@ -824,7 +849,7 @@ function DatabaseList({
onClick={() => openSemanticLayerDeleteModal(original)}
/>
)}
- {canEdit && (
+ {canWriteLayer && (
Review Comment:
Addressed in d5b760fc253301706e52c47ef7fb05dc551c9770. Removed the redundant
Edit wrapper after the canWriteLayer early return. Independent Database and
SemanticLayer permissions remain unchanged.
##########
superset-frontend/src/pages/DatabaseList/index.tsx:
##########
@@ -894,7 +919,7 @@ function DatabaseList({
},
Header: t('Actions'),
id: 'actions',
- hidden: !canEdit && !canDelete,
+ hidden: !canEdit && !canDelete && !canWriteLayer,
Review Comment:
Addressed in d5b760fc253301706e52c47ef7fb05dc551c9770. Added canExport to
Actions-column visibility. The export-only regression asserts Export is visible
while Edit/Delete remain absent.
##########
superset-frontend/src/pages/DatasetList/DatasetList.connectionPermissions.test.tsx:
##########
@@ -0,0 +1,69 @@
+/**
+ * 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
+ * regarding copyright ownership. 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 fetchMock from 'fetch-mock';
+import { screen, waitFor } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+import {
+ setupMocks,
+ renderDatasetList,
+ mockAdminUser,
+} from './DatasetList.testHelpers';
+
+beforeEach(() => {
+ setupMocks();
+ window.featureFlags = { SEMANTIC_LAYERS: true } as never;
+ fetchMock.get('glob:*/api/v1/semantic_layer/?*', { result: [], count: 0 });
+});
+
+afterEach(() => {
+ window.featureFlags = {} as never;
+ fetchMock.clearHistory().removeRoutes();
+ jest.restoreAllMocks();
+});
+
+test.each([false, true])(
+ 'dataset connection options respect independent layer read (%s)',
+ async canReadLayer => {
+ const user = {
+ ...mockAdminUser,
+ roles: {
+ Admin: [
+ ...mockAdminUser.roles.Admin,
+ ...(canReadLayer ? [['can_read', 'SemanticLayer']] : []),
+ ],
+ },
+ };
+ renderDatasetList(user);
+ await screen.findByTestId('search-filter-container');
+ const filter = screen
+ .getAllByTestId('compact-filter-pill')
+ .find(item => item.textContent?.includes('Data connection'));
+ expect(filter).toBeDefined();
+ await userEvent.click(filter!);
+ await waitFor(() =>
+ expect(
+ fetchMock.callHistory.calls('glob:*/api/v1/dataset/related/database*')
+ .length,
+ ).toBeGreaterThan(0),
+ );
+ expect(
+ fetchMock.callHistory.calls('glob:*/api/v1/semantic_layer/?*').length >
0,
Review Comment:
Addressed in d5b760fc253301706e52c47ef7fb05dc551c9770. Moved the
semantic-layer route glob into shared API_ENDPOINTS and reused the existing
related-database constant in the assertion.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
