GET /api/database/:id/schemas should not return 'empty' schemas (#18123)

src/metabase/api/database.clj

@@ -696,7 +696,11 @@
   "Returns a list of all the schemas found for the database `id`"
   [id]
   (api/read-check Database id)
-  (->> (db/select-field :schema Table :db_id id, :active true, {:order-by [[:%lower.schema :asc]]})
+  (->> (db/select-field :schema Table
+         :db_id id :active true
+         ;; a non-nil value means Table is hidden -- see [[metabase.models.table/visibility-types]]
+         :visibility_type nil
+         {:order-by [[:%lower.schema :asc]]})
        (filter (partial can-read-schema? id))
        ;; for `nil` schemas return the empty string
        (map #(if (nil? %) "" %))
@@ -723,6 +727,7 @@
                          :db_id           db-id
                          :schema          schema
                          :active          true
+                         ;; a non-nil value means Table is hidden -- see [[metabase.models.table/visibility-types]]
                          :visibility_type nil
                          {:order-by [[:display_name :asc]]})))
 

src/metabase/models/table.clj

@@ -16,12 +16,12 @@
 
 ;;; ----------------------------------------------- Constants + Entity -----------------------------------------------
 
-(def ^:const visibility-types
+(def visibility-types
   "Valid values for `Table.visibility_type` (field may also be `nil`).
    (Basically any non-nil value is a reason for hiding the table.)"
   #{:hidden :technical :cruft})
 
-(def ^:const field-orderings
+(def field-orderings
   "Valid values for `Table.field_order`.
   `:database`     - use the same order as in the table definition in the DB;
   `:alphabetical` - order alphabetically by name;

test/metabase/api/database_test.clj

@@ -837,6 +837,18 @@
         (is (= ["" " "]
                (mt/user-http-request :lucky :get 200 (format "database/%d/schemas" db-id))))))))
 
+(deftest get-schemas-should-not-return-schemas-with-no-visible-tables
+  (testing "GET /api/database/:id/schemas should not return schemas with no VISIBLE TABLES"
+    (mt/with-temp* [Database [{db-id :id}]
+                    Table    [_ {:db_id db-id, :schema "schema_1", :name "table_1"}]
+                    ;; table is not visible. Any non-nil value of `visibility_type` means Table shouldn't be visible
+                    Table    [_ {:db_id db-id, :schema "schema_2", :name "table_2a", :visibility_type "hidden"}]
+                    Table    [_ {:db_id db-id, :schema "schema_2", :name "table_2b", :visibility_type "cruft"}]
+                    ;; table is not active
+                    Table    [_ {:db_id db-id, :schema "schema_3", :name "table_3", :active false}]]
+      (is (= #{"schema_1"}
+             (set (mt/user-http-request :crowberto :get 200 (format "database/%d/schemas" db-id))))))))
+
 (deftest get-schema-tables-test
   (testing "GET /api/database/:id/schema/:schema\n"
     (testing "Permissions: Can we fetch the Tables in a schema?"