From e677eb7ad237144bf7af5108d769dc3cd2adacf3 Mon Sep 17 00:00:00 2001
From: John Swanson <john.swanson@metabase.com>
Date: Fri, 14 Jun 2024 05:25:03 -0700
Subject: [PATCH] Always provide `can_restore` (#44124)
Previously we were just adding this when the item was archived. Instead,
always send a value - if the item is not archived, it can't be restored,
so it will be `false` in this case.
Making this change will make fixing a frontend bug easier, because the
frontend will be able to just trust the `can_restore` value coming in
from the backend.
---
src/metabase/models/collection.clj | 40 ++++++++++++++-------------
test/metabase/api/collection_test.clj | 3 ++
test/metabase/api/dashboard_test.clj | 2 +-
3 files changed, 25 insertions(+), 20 deletions(-)
diff --git a/src/metabase/models/collection.clj b/src/metabase/models/collection.clj
index 659bbbd4477..d62018e6996 100644
--- a/src/metabase/models/collection.clj
+++ b/src/metabase/models/collection.clj
@@ -1599,25 +1599,27 @@
(for [{collection :collection
:as item*} (t2/hydrate items :collection)
:let [item (dissoc item* :collection)]]
- (cond-> item
- (:archived item)
- (assoc :can_restore (and
- ;; the item is directly in the trash (it was archived independently, not as
- ;; part of a collection)
- (:archived_directly item)
-
- ;; EITHER:
- (or
- ;; the item was archived from the root collection
- (nil? (:collection_id item))
- ;; or the collection we'll restore to actually exists.
- (some? collection))
-
- ;; the collection we'll restore to is not archived
- (not (:archived collection))
-
- ;; we have perms on the collection
- (mi/can-write? (or collection root-collection)))))))
+ (assoc item :can_restore (boolean
+ (and
+ ;; the item is archived
+ (:archived item)
+
+ ;; the item is directly in the trash (it was archived independently, not as
+ ;; part of a collection)
+ (:archived_directly item)
+
+ ;; EITHER:
+ (or
+ ;; the item was archived from the root collection
+ (nil? (:collection_id item))
+ ;; or the collection we'll restore to actually exists.
+ (some? collection))
+
+ ;; the collection we'll restore to is not archived
+ (not (:archived collection))
+
+ ;; we have perms on the collection
+ (mi/can-write? (or collection root-collection)))))))
(mi/define-batched-hydration-method can-restore
:can_restore
diff --git a/test/metabase/api/collection_test.clj b/test/metabase/api/collection_test.clj
index 4a050107617..3c2c066186f 100644
--- a/test/metabase/api/collection_test.clj
+++ b/test/metabase/api/collection_test.clj
@@ -76,6 +76,7 @@
:authority_level nil
:is_personal false
:id "root"
+ :can_restore false
:can_delete false}
(assoc (into {:is_personal false} collection) :can_write true :can_delete false)]
(filter #(#{(:id collection) "root"} (:id %))
@@ -640,6 +641,7 @@
[{:collection_id (:id collection)
:can_write true
:can_delete false
+ :can_restore false
:id card-id
:archived false
:location nil
@@ -1423,6 +1425,7 @@
(is (= {:name "Our analytics"
:id "root"
:can_write true
+ :can_restore false
:effective_location nil
:effective_ancestors []
:authority_level nil
diff --git a/test/metabase/api/dashboard_test.clj b/test/metabase/api/dashboard_test.clj
index 9a80b1cb629..dfc41eb2075 100644
--- a/test/metabase/api/dashboard_test.clj
+++ b/test/metabase/api/dashboard_test.clj
@@ -4489,7 +4489,7 @@
(t2.with-temp/with-temp [:model/Collection {coll-id :id} {:name "A"}
:model/Dashboard {dash-id :id} {:name "My Dashboard"
:collection_id coll-id}]
- (is (nil? (can-restore? dash-id :crowberto)))))))
+ (is (false? (can-restore? dash-id :crowberto)))))))
(deftest dependent-metadata-test
(mt/with-temp
--
GitLab