From 60cb22c438e3a2545090c9ae76cf2b744e9fb12e Mon Sep 17 00:00:00 2001
From: Alexander Polyankin <alexander.polyankin@metabase.com>
Date: Wed, 27 Jul 2022 18:13:16 +0300
Subject: [PATCH] Do not offer to save questions to view-only collections
(#24307)
---
frontend/src/metabase/containers/SaveQuestionModal.jsx | 3 ++-
frontend/src/metabase/entities/groups.js | 6 +++---
frontend/src/metabase/entities/persisted-models.js | 4 ++--
frontend/src/metabase/entities/schemas.js | 6 +++---
frontend/src/metabase/entities/tables.js | 8 ++++----
frontend/src/metabase/entities/timelines.js | 6 +++---
.../22727-readonly-collection-offered-on-save.cy.spec.js | 2 +-
7 files changed, 18 insertions(+), 17 deletions(-)
diff --git a/frontend/src/metabase/containers/SaveQuestionModal.jsx b/frontend/src/metabase/containers/SaveQuestionModal.jsx
index b5dbc97b728..0d1eefceb14 100644
--- a/frontend/src/metabase/containers/SaveQuestionModal.jsx
+++ b/frontend/src/metabase/containers/SaveQuestionModal.jsx
@@ -79,6 +79,7 @@ export default class SaveQuestionModal extends Component {
this.props;
const isStructured = Q_DEPRECATED.isStructured(card.dataset_query);
+ const isReadonly = originalCard != null && !originalCard.can_write;
const initialValues = {
name:
@@ -87,7 +88,7 @@ export default class SaveQuestionModal extends Component {
: "",
description: card.description || "",
collection_id:
- card.collection_id === undefined
+ card.collection_id === undefined || isReadonly
? initialCollectionId
: card.collection_id,
saveType:
diff --git a/frontend/src/metabase/entities/groups.js b/frontend/src/metabase/entities/groups.js
index 0b73937f51e..dfbd576c0a0 100644
--- a/frontend/src/metabase/entities/groups.js
+++ b/frontend/src/metabase/entities/groups.js
@@ -13,8 +13,8 @@ const Groups = createEntity({
fields: [{ name: "name" }],
},
- reducer: (state = {}, { type, payload }) => {
- if (type === CREATE_MEMBERSHIP) {
+ reducer: (state = {}, { type, payload, error }) => {
+ if (type === CREATE_MEMBERSHIP && !error) {
const { membership, group_id } = payload;
const members = state[group_id]?.members;
if (members) {
@@ -25,7 +25,7 @@ const Groups = createEntity({
}
}
- if (type === DELETE_MEMBERSHIP) {
+ if (type === DELETE_MEMBERSHIP && !error) {
const { membershipId, groupId } = payload;
const members = state[groupId]?.members;
if (members) {
diff --git a/frontend/src/metabase/entities/persisted-models.js b/frontend/src/metabase/entities/persisted-models.js
index 462aa5dc2ca..f2267f339c9 100644
--- a/frontend/src/metabase/entities/persisted-models.js
+++ b/frontend/src/metabase/entities/persisted-models.js
@@ -36,8 +36,8 @@ const PersistedModels = createEntity({
getByModelId: getPersistedModelInfoByModelId,
},
- reducer: (state = {}, { type, payload }) => {
- if (type === REFRESH_CACHE) {
+ reducer: (state = {}, { type, payload, error }) => {
+ if (type === REFRESH_CACHE && !error) {
return {
...state,
[payload.id]: {
diff --git a/frontend/src/metabase/entities/schemas.js b/frontend/src/metabase/entities/schemas.js
index 50fde6e7e06..26319561196 100644
--- a/frontend/src/metabase/entities/schemas.js
+++ b/frontend/src/metabase/entities/schemas.js
@@ -50,8 +50,8 @@ export default createEntity({
},
},
- reducer: (state = {}, { type, payload }) => {
- if (type === Questions.actionTypes.CREATE) {
+ reducer: (state = {}, { type, payload, error }) => {
+ if (type === Questions.actionTypes.CREATE && !error) {
const { question, status, data } = payload;
if (question) {
const schema = getCollectionVirtualSchemaId(question.collection);
@@ -71,7 +71,7 @@ export default createEntity({
}
}
- if (type === Questions.actionTypes.UPDATE) {
+ if (type === Questions.actionTypes.UPDATE && !error) {
const { question } = payload;
const schemaId = getCollectionVirtualSchemaId(question.collection);
diff --git a/frontend/src/metabase/entities/tables.js b/frontend/src/metabase/entities/tables.js
index 3bed285adc3..faf04c12986 100644
--- a/frontend/src/metabase/entities/tables.js
+++ b/frontend/src/metabase/entities/tables.js
@@ -144,7 +144,7 @@ const Tables = createEntity({
},
reducer: (state = {}, { type, payload, error }) => {
- if (type === Questions.actionTypes.CREATE) {
+ if (type === Questions.actionTypes.CREATE && !error) {
const card = payload.question;
const virtualQuestionTable = convertSavedQuestionToVirtualTable(card);
@@ -158,7 +158,7 @@ const Tables = createEntity({
};
}
- if (type === Questions.actionTypes.UPDATE) {
+ if (type === Questions.actionTypes.UPDATE && !error) {
const card = payload.question;
const virtualQuestionId = getQuestionVirtualTableId(card);
@@ -202,7 +202,7 @@ const Tables = createEntity({
}
}
- if (type === Metrics.actionTypes.CREATE) {
+ if (type === Metrics.actionTypes.CREATE && !error) {
const { table_id: tableId, id: metricId } = payload.metric;
const table = state[tableId];
if (table) {
@@ -213,7 +213,7 @@ const Tables = createEntity({
}
}
- if (type === Segments.actionTypes.UPDATE) {
+ if (type === Segments.actionTypes.UPDATE && !error) {
const { table_id: tableId, archived, id: segmentId } = payload.segment;
const table = state[tableId];
if (archived && table && table.segments) {
diff --git a/frontend/src/metabase/entities/timelines.js b/frontend/src/metabase/entities/timelines.js
index 6235e457eef..12c82472201 100644
--- a/frontend/src/metabase/entities/timelines.js
+++ b/frontend/src/metabase/entities/timelines.js
@@ -57,7 +57,7 @@ const Timelines = createEntity({
},
reducer: (state = {}, action) => {
- if (action.type === TimelineEvents.actionTypes.CREATE) {
+ if (action.type === TimelineEvents.actionTypes.CREATE && !action.error) {
const event = TimelineEvents.HACK_getObjectFromAction(action);
return updateIn(state, [event.timeline_id, "events"], (eventIds = []) => {
@@ -65,7 +65,7 @@ const Timelines = createEntity({
});
}
- if (action.type === TimelineEvents.actionTypes.UPDATE) {
+ if (action.type === TimelineEvents.actionTypes.UPDATE && !action.error) {
const event = TimelineEvents.HACK_getObjectFromAction(action);
return _.mapObject(state, timeline => {
@@ -84,7 +84,7 @@ const Timelines = createEntity({
});
}
- if (action.type === TimelineEvents.actionTypes.DELETE) {
+ if (action.type === TimelineEvents.actionTypes.DELETE && !action.error) {
const eventId = action.payload.result;
return _.mapObject(state, timeline => {
diff --git a/frontend/test/metabase/scenarios/permissions/reproductions/22727-readonly-collection-offered-on-save.cy.spec.js b/frontend/test/metabase/scenarios/permissions/reproductions/22727-readonly-collection-offered-on-save.cy.spec.js
index 0b99cb5401a..9b08f905eaa 100644
--- a/frontend/test/metabase/scenarios/permissions/reproductions/22727-readonly-collection-offered-on-save.cy.spec.js
+++ b/frontend/test/metabase/scenarios/permissions/reproductions/22727-readonly-collection-offered-on-save.cy.spec.js
@@ -3,7 +3,7 @@ import { USER_GROUPS } from "__support__/e2e/cypress_data";
const { ALL_USERS_GROUP } = USER_GROUPS;
-describe.skip("issue 22727", () => {
+describe("issue 22727", () => {
beforeEach(() => {
cy.intercept("POST", "/api/dataset").as("dataset");
--
GitLab