From 84f4509dbe2fe80d482509549fbfcc7033eb7c2a Mon Sep 17 00:00:00 2001
From: dpsutton <dan@dpsutton.com>
Date: Tue, 3 Aug 2021 19:19:24 -0500
Subject: [PATCH] correct moderation review hydration (#17311)
* remove moderation reviews hydration
* Restore hydration of moderation_reviews and handle correctly
nested hydration is (i think) positional. So if you get something
handed to you you must return something in that position. Prevoiusly
was removing nils and then returning a smaller collection of hydrated
items (cards here). But this meant input might look like this
[card1 nil card2] and return [card1 card2] and in the nested hierarchy
things didn't get matched up correctly.
In the real world application it might look like this:
```clojure
{:ordered-cards
[{:card-id 1}
{:card-id nil
:viz-settings {info-for text-card}}
{:card-id 2}]}
```
And the nil card comes into this function and we return them in a
strange manner things get wonky
---
src/metabase/moderation.clj | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/src/metabase/moderation.clj b/src/metabase/moderation.clj
index d972a29a98b..018c88a1a63 100644
--- a/src/metabase/moderation.clj
+++ b/src/metabase/moderation.clj
@@ -26,16 +26,23 @@
{:batched-hydrate :moderation_reviews}
[items]
;; no need to do work on empty items. Also, can have nil here due to text cards. I think this is a bug in toucan. To
- ;; get here we are `(hydrate dashboard [:ordered_cards [:card :moderation_reviews] :series] ...)` But ordered_cards dont have to have cards. but the hydration will pass the nil card id into
- (when-some [items (seq (keep identity items))]
- (let [all-reviews (group-by (juxt :moderated_item_type :moderated_item_id)
- (db/select 'ModerationReview
- :moderated_item_type "card"
- :moderated_item_id [:in (map :id items)]
- {:order-by [[:id :desc]]}))]
+ ;; get here we are `(hydrate dashboard [:ordered_cards [:card :moderation_reviews] :series] ...)` But ordered_cards
+ ;; dont have to have cards. but the hydration will pass the nil card id into here. NOTE: it is important that each
+ ;; item that comes into this comes out. The nested hydration is positional, not by an id so everything that comes in
+ ;; must go out in the same order
+ (when (seq items)
+ (let [item-ids (not-empty (keep :id items))
+ all-reviews (when item-ids
+ (group-by (juxt :moderated_item_type :moderated_item_id)
+ (db/select 'ModerationReview
+ :moderated_item_type "card"
+ :moderated_item_id [:in item-ids]
+ {:order-by [[:id :desc]]})))]
(for [item items]
- (let [k ((juxt (comp keyword object->type) u/the-id) item)]
- (assoc item :moderation_reviews (get all-reviews k ())))))))
+ (if (nil? item)
+ nil
+ (let [k ((juxt (comp keyword object->type) u/the-id) item)]
+ (assoc item :moderation_reviews (get all-reviews k ()))))))))
(defn moderated-item
"The moderated item for a given request or review"
--
GitLab