metabase / toucan

A classy high-level Clojure library for defining application models and retrieving them from a DB
Eclipse Public License 1.0
570 stars 49 forks source link

apply-type-fns does not apply the function if the value is false #70

Closed jonathanjansen closed 2 years ago

jonathanjansen commented 4 years ago

Hi @camsaul

I am attempting to run a type function, when the column value is false the function does not run, as per below.


(toucan.models/add-type! :demo
                         :in (fn [val]
                               (println "Demo type called")
                               (let [n-val (if val "True Transformed" "False Transformed")]
                                 n-val))
                         :out boolean)

(extend (class Venue)
  toucan.models/IModel
  (merge toucan.models/IModelDefaults {:default-fields (constantly #{:id :name :category})
                                :types          (constantly {:category :keyword
                                                             :demo :demo
                                                             }
                                                            )
                                :properties     (constantly {:timestamped? true})}))

(clojure.test/is (=
 #toucan.test_models.venue.VenueInstance{:demo "True Transformed"}
 (#'toucan.models/apply-type-fns #toucan.test_models.venue.VenueInstance{:demo "A"} :in)))
;=> true
(clojure.test/is (=
 #toucan.test_models.venue.VenueInstance{:demo "False Transformed"}
 (#'toucan.models/apply-type-fns #toucan.test_models.venue.VenueInstance{:demo nil} :in)))
;=> false
(clojure.test/is (=
 #toucan.test_models.venue.VenueInstance{:demo "True Transformed"}
 (#'toucan.models/apply-type-fns #toucan.test_models.venue.VenueInstance{:demo true} :in)))
;=> true
(clojure.test/is (=
 #toucan.test_models.venue.VenueInstance{:demo "False Transformed"}
 (#'toucan.models/apply-type-fns #toucan.test_models.venue.VenueInstance{:demo false} :in)))
;=> false
`
camsaul commented 4 years ago

@jonathanjansen, that sounds like a bug for sure. There's probably somewhere in apply-type-fns where it's checking whether the value is truthy before applying the function, rather than checking whether it's non-nil. Should be a one-liner once it's tracked down. PRs welcome!

jonathanjansen commented 4 years ago

@camsaul

Thank you for the reply

Its the when-let src/toucan/models.clj line 303

(defn- apply-type-fns
  "Apply the appropriate `type-fns` for OBJ."
  [obj direction]
  (into obj (for [[col type] (types obj)]
              (when-let [v (get obj col)]
                {col ((get-in @type-fns [type direction]) v)}))))

I will do a pull request over the week.

camsaul commented 2 years ago

Fixed by #90