Repository: gouline/dbt-metabase (language: python)

Issue:
Handle hidden tables during model sync
- Don't fail model sync on hidden tables (still log warning though)
- Skip column sync for tables remaining hidden or switching to hidden in the current execution
- Attempt to sync columns for tables switching from hidden with a log suggesting a retry to allow Metabase to sync the stale table on errors
- Fixes #264


Discussion / hints:

Fix patch:
diff --git a/sandbox/metabase/metabase.db.mv.db b/sandbox/metabase/metabase.db.mv.db
index 9f6850fb..d642d7c5 100644
Binary files a/sandbox/metabase/metabase.db.mv.db and b/sandbox/metabase/metabase.db.mv.db differ
diff --git a/sandbox/models/staging/schema.yml b/sandbox/models/staging/schema.yml
index 7ed0a8ab..0502f984 100644
--- a/sandbox/models/staging/schema.yml
+++ b/sandbox/models/staging/schema.yml
@@ -2,6 +2,8 @@ version: 2
 
 models:
   - name: stg_customers
+    meta:
+      metabase.visibility_type: hidden
     columns:
       - name: customer_id
         data_tests:


Test patch:
diff --git a/tests/_mocks.py b/tests/_mocks.py
index 49c31811..84333d60 100644
--- a/tests/_mocks.py
+++ b/tests/_mocks.py
@@ -7,7 +7,7 @@
 from dotenv import dotenv_values
 
 from dbtmetabase.core import DbtMetabase
-from dbtmetabase.manifest import Manifest, Model
+from dbtmetabase.manifest import Column, Manifest, Model
 from dbtmetabase.metabase import Metabase
 
 FIXTURES_PATH = Path("tests") / "fixtures"
@@ -50,28 +50,30 @@ def _api(
         params: Optional[Dict[str, Any]] = None,
         **kwargs,
     ) -> Union[Mapping, Sequence]:
+        result = {}
         path_toks = f"{path.lstrip('/')}.json".split("/")
         json_path = Path.joinpath(FIXTURES_PATH, *path_toks)
 
         if self.record:
-            resp = super()._api(method, path, params, **kwargs)
-
-            if method == "get":
-                json_path.parent.mkdir(parents=True, exist_ok=True)
-                with open(json_path, "w", encoding="utf-8") as f:
-                    json.dump(resp, f, indent=4)
-
-            return resp
+            is_auth = path == "/api/session"
+            if method == "get" or is_auth:
+                result = super()._api(method, path, params, **kwargs)
+
+                if not is_auth:
+                    json_path.parent.mkdir(parents=True, exist_ok=True)
+                    with open(json_path, "w", encoding="utf-8") as f:
+                        json.dump(result, f, indent=4)
         else:
             if method == "get":
                 if json_path.exists():
                     with open(json_path, encoding="utf-8") as f:
-                        return json.load(f)
+                        result = json.load(f)
                 else:
                     response = requests.Response()
                     response.status_code = 404
                     raise requests.exceptions.HTTPError(response=response)
-            return {}
+
+        return result
 
 
 class MockManifest(Manifest):
@@ -82,6 +84,24 @@ def read_models(self) -> Sequence[Model]:
             self._models = super().read_models()
         return self._models
 
+    def find_model(self, model_name: str) -> Optional[Model]:
+        filtered = [m for m in self._models if m.name == model_name]
+        if filtered:
+            return filtered[0]
+        return None
+
+    def find_column(
+        self,
+        model_name: str,
+        column_name: str,
+    ) -> Optional[Column]:
+        model = self.find_model(model_name=model_name)
+        if model:
+            filtered = [c for c in model.columns if c.name == column_name]
+            if filtered:
+                return filtered[0]
+        return None
+
 
 class MockDbtMetabase(DbtMetabase):
     def __init__(
diff --git a/tests/fixtures/api/card/27.json b/tests/fixtures/api/card/27.json
index 1828511f..8d739af6 100644
--- a/tests/fixtures/api/card/27.json
+++ b/tests/fixtures/api/card/27.json
@@ -2,7 +2,7 @@
     "cache_invalidated_at": null,
     "description": "Orders and customers",
     "archived": false,
-    "view_count": 361,
+    "view_count": 389,
     "collection_position": null,
     "table_id": 10,
     "can_run_adhoc_query": true,
@@ -561,7 +561,7 @@
     "creator": {
         "email": "dbtmetabase@example.com",
         "first_name": "dbtmetabase",
-        "last_login": "2024-06-20T06:03:33.519227Z",
+        "last_login": "2024-07-04T05:55:25.914119Z",
         "is_qbnewb": false,
         "is_superuser": true,
         "id": 1,
diff --git a/tests/fixtures/api/card/28.json b/tests/fixtures/api/card/28.json
index d7bfbedb..589ee640 100644
--- a/tests/fixtures/api/card/28.json
+++ b/tests/fixtures/api/card/28.json
@@ -2,7 +2,7 @@
     "cache_invalidated_at": null,
     "description": null,
     "archived": false,
-    "view_count": 180,
+    "view_count": 194,
     "collection_position": null,
     "table_id": 10,
     "can_run_adhoc_query": true,
@@ -549,7 +549,7 @@
     "creator": {
         "email": "dbtmetabase@example.com",
         "first_name": "dbtmetabase",
-        "last_login": "2024-06-20T06:03:33.519227Z",
+        "last_login": "2024-07-04T05:55:25.914119Z",
         "is_qbnewb": false,
         "is_superuser": true,
         "id": 1,
diff --git a/tests/fixtures/api/card/29.json b/tests/fixtures/api/card/29.json
index 837addff..d1b4c208 100644
--- a/tests/fixtures/api/card/29.json
+++ b/tests/fixtures/api/card/29.json
@@ -2,7 +2,7 @@
     "cache_invalidated_at": null,
     "description": null,
     "archived": false,
-    "view_count": 179,
+    "view_count": 193,
     "collection_position": null,
     "table_id": null,
     "can_run_adhoc_query": true,
@@ -41,7 +41,7 @@
     "creator": {
         "email": "dbtmetabase@example.com",
         "first_name": "dbtmetabase",
-        "last_login": "2024-06-20T06:03:33.519227Z",
+        "last_login": "2024-07-04T05:55:25.914119Z",
         "is_qbnewb": false,
         "is_superuser": true,
         "id": 1,
diff --git a/tests/fixtures/api/card/30.json b/tests/fixtures/api/card/30.json
index 20d72db9..f5cfbb88 100644
--- a/tests/fixtures/api/card/30.json
+++ b/tests/fixtures/api/card/30.json
@@ -2,7 +2,7 @@
     "cache_invalidated_at": null,
     "description": "Dummy 1",
     "archived": false,
-    "view_count": 21,
+    "view_count": 28,
     "collection_position": null,
     "table_id": null,
     "can_run_adhoc_query": true,
@@ -41,7 +41,7 @@
     "creator": {
         "email": "dbtmetabase@example.com",
         "first_name": "dbtmetabase",
-        "last_login": "2024-06-20T06:03:33.519227Z",
+        "last_login": "2024-07-04T05:55:25.914119Z",
         "is_qbnewb": false,
         "is_superuser": true,
         "id": 1,
diff --git a/tests/fixtures/api/card/31.json b/tests/fixtures/api/card/31.json
index 62870a94..870fddbf 100644
--- a/tests/fixtures/api/card/31.json
+++ b/tests/fixtures/api/card/31.json
@@ -2,7 +2,7 @@
     "cache_invalidated_at": null,
     "description": "Dummy 2",
     "archived": false,
-    "view_count": 21,
+    "view_count": 28,
     "collection_position": null,
     "table_id": null,
     "can_run_adhoc_query": true,
@@ -41,7 +41,7 @@
     "creator": {
         "email": "dbtmetabase@example.com",
         "first_name": "dbtmetabase",
-        "last_login": "2024-06-20T06:03:33.519227Z",
+        "last_login": "2024-07-04T05:55:25.914119Z",
         "is_qbnewb": false,
         "is_superuser": true,
         "id": 1,
diff --git a/tests/fixtures/api/card/32.json b/tests/fixtures/api/card/32.json
index 2a2aabd5..333c8d3f 100644
--- a/tests/fixtures/api/card/32.json
+++ b/tests/fixtures/api/card/32.json
@@ -2,7 +2,7 @@
     "cache_invalidated_at": null,
     "description": "CTE SQL",
     "archived": false,
-    "view_count": 12,
+    "view_count": 19,
     "collection_position": null,
     "table_id": null,
     "can_run_adhoc_query": true,
@@ -276,7 +276,7 @@
     "creator": {
         "email": "dbtmetabase@example.com",
         "first_name": "dbtmetabase",
-        "last_login": "2024-06-20T06:03:33.519227Z",
+        "last_login": "2024-07-04T05:55:25.914119Z",
         "is_qbnewb": false,
         "is_superuser": true,
         "id": 1,
diff --git a/tests/fixtures/api/dashboard/2.json b/tests/fixtures/api/dashboard/2.json
index 3a2f87c4..474dc006 100644
--- a/tests/fixtures/api/dashboard/2.json
+++ b/tests/fixtures/api/dashboard/2.json
@@ -1,7 +1,7 @@
 {
     "description": "Dashboard is a dashboard is a dashboard",
     "archived": false,
-    "view_count": 96,
+    "view_count": 103,
     "collection_position": null,
     "dashcards": [
         {
@@ -14,7 +14,7 @@
                 "cache_invalidated_at": null,
                 "description": "Orders and customers",
                 "archived": false,
-                "view_count": 360,
+                "view_count": 388,
                 "collection_position": null,
                 "table_id": 10,
                 "can_run_adhoc_query": true,
@@ -654,7 +654,7 @@
                 "cache_invalidated_at": null,
                 "description": null,
                 "archived": false,
-                "view_count": 180,
+                "view_count": 194,
                 "collection_position": null,
                 "table_id": 10,
                 "can_run_adhoc_query": true,
@@ -1268,7 +1268,7 @@
                 "cache_invalidated_at": null,
                 "description": null,
                 "archived": false,
-                "view_count": 179,
+                "view_count": 193,
                 "collection_position": null,
                 "table_id": null,
                 "can_run_adhoc_query": true,
diff --git a/tests/fixtures/api/database.json b/tests/fixtures/api/database.json
index d66d6b87..da09833d 100644
--- a/tests/fixtures/api/database.json
+++ b/tests/fixtures/api/database.json
@@ -51,7 +51,7 @@
         "caveats": null,
         "creator_id": 1,
         "is_full_sync": true,
-        "updated_at": "2024-06-20T05:33:42.131951Z",
+        "updated_at": "2024-07-04T05:54:51.198536Z",
         "native_permissions": "write",
         "cache_ttl": null,
         "details": {
diff --git a/tests/fixtures/api/database/2/metadata.json b/tests/fixtures/api/database/2/metadata.json
index c22daedd..64cbe3ff 100644
--- a/tests/fixtures/api/database/2/metadata.json
+++ b/tests/fixtures/api/database/2/metadata.json
@@ -408,7 +408,7 @@
             ],
             "caveats": null,
             "segments": [],
-            "updated_at": "2024-06-20T06:03:34.567961Z",
+            "updated_at": "2024-07-04T05:54:52.259601Z",
             "active": true,
             "id": 12,
             "db_id": 2,
@@ -937,7 +937,7 @@
             ],
             "caveats": "Some facts are derived from payments",
             "segments": [],
-            "updated_at": "2024-06-20T06:03:34.567961Z",
+            "updated_at": "2024-07-04T05:54:52.259601Z",
             "active": true,
             "id": 10,
             "db_id": 2,
@@ -1099,7 +1099,7 @@
             ],
             "caveats": null,
             "segments": [],
-            "updated_at": "2024-06-20T06:03:34.567961Z",
+            "updated_at": "2024-07-04T05:54:52.259601Z",
             "active": true,
             "id": 13,
             "db_id": 2,
@@ -1309,7 +1309,7 @@
             ],
             "caveats": null,
             "segments": [],
-            "updated_at": "2024-06-20T06:03:34.567961Z",
+            "updated_at": "2024-07-04T05:54:52.259601Z",
             "active": true,
             "id": 14,
             "db_id": 2,
@@ -1523,7 +1523,7 @@
             ],
             "caveats": null,
             "segments": [],
-            "updated_at": "2024-06-20T06:03:34.567961Z",
+            "updated_at": "2024-07-04T05:54:52.259601Z",
             "active": true,
             "id": 11,
             "db_id": 2,
@@ -1700,11 +1700,11 @@
             ],
             "caveats": null,
             "segments": [],
-            "updated_at": "2024-06-20T06:03:34.567961Z",
+            "updated_at": "2024-07-04T05:54:57.125982Z",
             "active": true,
             "id": 16,
             "db_id": 2,
-            "visibility_type": null,
+            "visibility_type": "hidden",
             "field_order": "custom",
             "is_upload": false,
             "initial_sync_status": "complete",
@@ -1925,7 +1925,7 @@
             ],
             "caveats": null,
             "segments": [],
-            "updated_at": "2024-06-20T06:03:34.567961Z",
+            "updated_at": "2024-07-04T05:54:52.259601Z",
             "active": true,
             "id": 15,
             "db_id": 2,
@@ -2154,7 +2154,7 @@
             ],
             "caveats": null,
             "segments": [],
-            "updated_at": "2024-06-20T06:03:34.567961Z",
+            "updated_at": "2024-07-04T05:54:52.259601Z",
             "active": true,
             "id": 9,
             "db_id": 2,
@@ -2171,7 +2171,7 @@
     ],
     "creator_id": 1,
     "is_full_sync": true,
-    "updated_at": "2024-06-20T05:33:42.131951Z",
+    "updated_at": "2024-07-04T05:54:51.198536Z",
     "cache_ttl": null,
     "details": {
         "ssl": false,
diff --git a/tests/fixtures/api/table.json b/tests/fixtures/api/table.json
index 430cfd92..274160fa 100644
--- a/tests/fixtures/api/table.json
+++ b/tests/fixtures/api/table.json
@@ -57,7 +57,7 @@
             "caveats": null,
             "creator_id": 1,
             "is_full_sync": true,
-            "updated_at": "2024-06-20T05:33:42.131951Z",
+            "updated_at": "2024-07-04T05:54:51.198536Z",
             "cache_ttl": null,
             "details": {
                 "ssl": false,
@@ -92,7 +92,7 @@
         "show_in_getting_started": false,
         "name": "customers",
         "caveats": null,
-        "updated_at": "2024-06-20T06:02:33.283724Z",
+        "updated_at": "2024-07-04T05:54:52.259601Z",
         "active": true,
         "id": 12,
         "db_id": 2,
@@ -163,7 +163,7 @@
             "caveats": null,
             "creator_id": 1,
             "is_full_sync": true,
-            "updated_at": "2024-06-20T05:33:42.131951Z",
+            "updated_at": "2024-07-04T05:54:51.198536Z",
             "cache_ttl": null,
             "details": {
                 "ssl": false,
@@ -198,7 +198,7 @@
         "show_in_getting_started": false,
         "name": "orders",
         "caveats": "Some facts are derived from payments",
-        "updated_at": "2024-06-20T06:02:33.283724Z",
+        "updated_at": "2024-07-04T05:54:52.259601Z",
         "active": true,
         "id": 10,
         "db_id": 2,
@@ -269,7 +269,7 @@
             "caveats": null,
             "creator_id": 1,
             "is_full_sync": true,
-            "updated_at": "2024-06-20T05:33:42.131951Z",
+            "updated_at": "2024-07-04T05:54:51.198536Z",
             "cache_ttl": null,
             "details": {
                 "ssl": false,
@@ -304,7 +304,7 @@
         "show_in_getting_started": false,
         "name": "raw_customers",
         "caveats": null,
-        "updated_at": "2024-06-20T06:02:33.283724Z",
+        "updated_at": "2024-07-04T05:54:52.259601Z",
         "active": true,
         "id": 13,
         "db_id": 2,
@@ -375,7 +375,7 @@
             "caveats": null,
             "creator_id": 1,
             "is_full_sync": true,
-            "updated_at": "2024-06-20T05:33:42.131951Z",
+            "updated_at": "2024-07-04T05:54:51.198536Z",
             "cache_ttl": null,
             "details": {
                 "ssl": false,
@@ -410,7 +410,7 @@
         "show_in_getting_started": false,
         "name": "raw_orders",
         "caveats": null,
-        "updated_at": "2024-06-20T06:02:33.283724Z",
+        "updated_at": "2024-07-04T05:54:52.259601Z",
         "active": true,
         "id": 14,
         "db_id": 2,
@@ -481,7 +481,7 @@
             "caveats": null,
             "creator_id": 1,
             "is_full_sync": true,
-            "updated_at": "2024-06-20T05:33:42.131951Z",
+            "updated_at": "2024-07-04T05:54:51.198536Z",
             "cache_ttl": null,
             "details": {
                 "ssl": false,
@@ -516,7 +516,7 @@
         "show_in_getting_started": false,
         "name": "raw_payments",
         "caveats": null,
-        "updated_at": "2024-06-20T06:02:33.283724Z",
+        "updated_at": "2024-07-04T05:54:52.259601Z",
         "active": true,
         "id": 11,
         "db_id": 2,
@@ -587,7 +587,7 @@
             "caveats": null,
             "creator_id": 1,
             "is_full_sync": true,
-            "updated_at": "2024-06-20T05:33:42.131951Z",
+            "updated_at": "2024-07-04T05:54:51.198536Z",
             "cache_ttl": null,
             "details": {
                 "ssl": false,
@@ -622,11 +622,11 @@
         "show_in_getting_started": false,
         "name": "stg_customers",
         "caveats": null,
-        "updated_at": "2024-06-20T06:02:33.283724Z",
+        "updated_at": "2024-07-04T05:54:57.125982Z",
         "active": true,
         "id": 16,
         "db_id": 2,
-        "visibility_type": null,
+        "visibility_type": "hidden",
         "field_order": "custom",
         "is_upload": false,
         "initial_sync_status": "complete",
@@ -693,7 +693,7 @@
             "caveats": null,
             "creator_id": 1,
             "is_full_sync": true,
-            "updated_at": "2024-06-20T05:33:42.131951Z",
+            "updated_at": "2024-07-04T05:54:51.198536Z",
             "cache_ttl": null,
             "details": {
                 "ssl": false,
@@ -728,7 +728,7 @@
         "show_in_getting_started": false,
         "name": "stg_orders",
         "caveats": null,
-        "updated_at": "2024-06-20T06:02:33.283724Z",
+        "updated_at": "2024-07-04T05:54:52.259601Z",
         "active": true,
         "id": 15,
         "db_id": 2,
@@ -799,7 +799,7 @@
             "caveats": null,
             "creator_id": 1,
             "is_full_sync": true,
-            "updated_at": "2024-06-20T05:33:42.131951Z",
+            "updated_at": "2024-07-04T05:54:51.198536Z",
             "cache_ttl": null,
             "details": {
                 "ssl": false,
@@ -834,7 +834,7 @@
         "show_in_getting_started": false,
         "name": "stg_payments",
         "caveats": null,
-        "updated_at": "2024-06-20T06:02:33.283724Z",
+        "updated_at": "2024-07-04T05:54:52.259601Z",
         "active": true,
         "id": 9,
         "db_id": 2,
diff --git a/tests/fixtures/api/user/1.json b/tests/fixtures/api/user/1.json
index f49fde95..c98f6186 100644
--- a/tests/fixtures/api/user/1.json
+++ b/tests/fixtures/api/user/1.json
@@ -2,7 +2,7 @@
     "email": "dbtmetabase@example.com",
     "first_name": "dbtmetabase",
     "locale": null,
-    "last_login": "2024-06-20T06:03:33.519227Z",
+    "last_login": "2024-07-04T05:55:25.914119Z",
     "is_active": true,
     "user_group_memberships": [
         {
@@ -13,7 +13,7 @@
         }
     ],
     "is_qbnewb": false,
-    "updated_at": "2024-06-20T06:03:33.519227Z",
+    "updated_at": "2024-07-04T05:55:25.914119Z",
     "is_superuser": true,
     "login_attributes": null,
     "id": 1,
diff --git a/tests/test_manifest.py b/tests/test_manifest.py
index 160bbaf2..a731d712 100644
--- a/tests/test_manifest.py
+++ b/tests/test_manifest.py
@@ -1,21 +1,22 @@
 import unittest
 from operator import attrgetter
-from typing import Optional, Sequence
+from typing import Sequence
 
 from dbtmetabase.manifest import Column, Group, Manifest, Model
 
-from ._mocks import FIXTURES_PATH
+from ._mocks import FIXTURES_PATH, MockManifest
 
 
 class TestManifest(unittest.TestCase):
     def test_v11_disabled(self):
-        models = Manifest(FIXTURES_PATH / "manifest-v11-disabled.json").read_models()
+        manifest = MockManifest(FIXTURES_PATH / "manifest-v11-disabled.json")
+        manifest.read_models()
 
-        orders_mod = self._find_model(models, "orders")
+        orders_mod = manifest.find_model("orders")
         self.assertIsNone(orders_mod)
 
-        customer_id_col = self._find_column(models, "customers", "customer_id")
-        assert customer_id_col
+        customer_id_col = manifest.find_column("customers", "customer_id")
+        self.assertIsNotNone(customer_id_col)
         self.assertIsNone(customer_id_col.fk_target_table)
         self.assertIsNone(customer_id_col.fk_target_field)
 
@@ -387,23 +388,3 @@ def _assertModelsEqual(
             )
 
         self.assertEqual(first, second)
-
-    @staticmethod
-    def _find_model(models: Sequence[Model], model_name: str) -> Optional[Model]:
-        filtered = [m for m in models if m.name == model_name]
-        if filtered:
-            return filtered[0]
-        return None
-
-    @staticmethod
-    def _find_column(
-        models: Sequence[Model],
-        model_name: str,
-        column_name: str,
-    ) -> Optional[Column]:
-        model = TestManifest._find_model(models=models, model_name=model_name)
-        if model:
-            filtered = [c for c in model.columns if c.name == column_name]
-            if filtered:
-                return filtered[0]
-        return None
diff --git a/tests/test_models.py b/tests/test_models.py
index 340d046f..e488a070 100644
--- a/tests/test_models.py
+++ b/tests/test_models.py
@@ -13,7 +13,24 @@ def test_export(self):
         self.c.export_models(
             metabase_database="dbtmetabase",
             skip_sources=True,
-            sync_timeout=0,
+            sync_timeout=1,
+            order_fields=True,
+        )
+
+    def test_export_hidden_table(self):
+        # pylint: disable=protected-access
+        self.c._manifest.read_models()
+        model = self.c._manifest.find_model("stg_customers")
+        model.visibility_type = "hidden"
+
+        column = model.columns[0]
+        column.name = "new_column_since_stale"
+        model.columns.append(column)
+
+        self.c.export_models(
+            metabase_database="dbtmetabase",
+            skip_sources=True,
+            sync_timeout=1,
             order_fields=True,
         )