From d2ca54511e9fe2d81e5f98a84ca068897556a4ea Mon Sep 17 00:00:00 2001
From: Sloan Sparger <sloansparger@users.noreply.github.com>
Date: Wed, 25 Sep 2024 11:23:11 -0500
Subject: [PATCH] [Permissions] PR Feedback for 47630 (#48116)
---
e2e/test/scenarios/permissions/view-data.cy.spec.js | 2 +-
.../admin/permissions/selectors/confirmations.tsx | 13 ++++++-------
.../selectors/data-permissions/fields.ts | 9 +++++----
.../selectors/data-permissions/tables.ts | 7 ++++---
4 files changed, 16 insertions(+), 15 deletions(-)
diff --git a/e2e/test/scenarios/permissions/view-data.cy.spec.js b/e2e/test/scenarios/permissions/view-data.cy.spec.js
index 5b52028664b..db20787026f 100644
--- a/e2e/test/scenarios/permissions/view-data.cy.spec.js
+++ b/e2e/test/scenarios/permissions/view-data.cy.spec.js
@@ -68,7 +68,7 @@ describeEE("scenarios > admin > permissions > view data > blocked", () => {
cy.findByRole("tooltip")
.findByText(
- /Users in groups with Blocked on a table can't view native queries on this database/,
+ /Groups with a database, schema, or table set to Blocked can't view native queries on this database/,
)
.should("exist");
diff --git a/frontend/src/metabase/admin/permissions/selectors/confirmations.tsx b/frontend/src/metabase/admin/permissions/selectors/confirmations.tsx
index 44bf8539d70..668f39900a7 100644
--- a/frontend/src/metabase/admin/permissions/selectors/confirmations.tsx
+++ b/frontend/src/metabase/admin/permissions/selectors/confirmations.tsx
@@ -90,7 +90,7 @@ export function getPermissionWarning(
return null;
}
-export function getTableBlockWarning(
+export function getBlockWarning(
dbValue: DataPermissionValue,
schemaValue: DataPermissionValue,
tableValue?: DataPermissionValue,
@@ -99,12 +99,11 @@ export function getTableBlockWarning(
return;
}
- if (schemaValue === DataPermissionValue.BLOCKED) {
- return t`Users in groups with Blocked on a schema can't view native queries on this database.`;
- }
-
- if (tableValue === DataPermissionValue.BLOCKED) {
- return t`Users in groups with Blocked on a table can't view native queries on this database.`;
+ if (
+ schemaValue === DataPermissionValue.BLOCKED ||
+ tableValue === DataPermissionValue.BLOCKED
+ ) {
+ return t`Groups with a database, schema, or table set to Blocked can't view native queries on this database.`;
}
}
diff --git a/frontend/src/metabase/admin/permissions/selectors/data-permissions/fields.ts b/frontend/src/metabase/admin/permissions/selectors/data-permissions/fields.ts
index d3f535aee1c..ce583048a80 100644
--- a/frontend/src/metabase/admin/permissions/selectors/data-permissions/fields.ts
+++ b/frontend/src/metabase/admin/permissions/selectors/data-permissions/fields.ts
@@ -26,10 +26,10 @@ import {
DataPermissionValue,
} from "../../types";
import {
+ getBlockWarning,
getPermissionWarning,
getPermissionWarningModal,
getRevokingAccessToAllTablesWarningModal,
- getTableBlockWarning,
getWillRevokeNativeAccessWarningModal,
} from "../confirmations";
@@ -64,14 +64,14 @@ const buildAccessPermission = (
);
const dbValue = getSchemasPermission(
- originalPermissions,
+ permissions,
groupId,
entityId,
DataPermission.VIEW_DATA,
);
const schemaValue = getTablesPermission(
- originalPermissions,
+ permissions,
groupId,
entityId,
DataPermission.VIEW_DATA,
@@ -85,8 +85,9 @@ const buildAccessPermission = (
groupId,
);
- const blockWarning = getTableBlockWarning(dbValue, schemaValue, value);
+ const blockWarning = getBlockWarning(dbValue, schemaValue, value);
+ // permissionWarning should always trump a blockWarning
const warning = permissionWarning || blockWarning;
const confirmations = (newValue: DataPermissionValue) => [
diff --git a/frontend/src/metabase/admin/permissions/selectors/data-permissions/tables.ts b/frontend/src/metabase/admin/permissions/selectors/data-permissions/tables.ts
index 42160130387..e6b279da932 100644
--- a/frontend/src/metabase/admin/permissions/selectors/data-permissions/tables.ts
+++ b/frontend/src/metabase/admin/permissions/selectors/data-permissions/tables.ts
@@ -23,9 +23,9 @@ import {
DataPermissionValue,
} from "../../types";
import {
+ getBlockWarning,
getPermissionWarning,
getPermissionWarningModal,
- getTableBlockWarning,
getViewDataPermissionsTooRestrictiveWarningModal,
getWillRevokeNativeAccessWarningModal,
} from "../confirmations";
@@ -60,7 +60,7 @@ const buildAccessPermission = (
);
const dbValue = getSchemasPermission(
- originalPermissions,
+ permissions,
groupId,
entityId,
DataPermission.VIEW_DATA,
@@ -74,8 +74,9 @@ const buildAccessPermission = (
groupId,
);
- const blockWarning = getTableBlockWarning(dbValue, value);
+ const blockWarning = getBlockWarning(dbValue, value);
+ // permissionWarning should always trump a blockWarning
const warning = permissionWarning || blockWarning;
const confirmations = (newValue: DataPermissionValue) => [
--
GitLab