fix(foreignKey): in str primary keys slashes are now working

src/agent_toolkit/forestadmin/agent_toolkit/services/serializers/json_api_serializer.py

@@ -1,6 +1,7 @@
 from ast import literal_eval
 from datetime import date, datetime, time
 from typing import Any, Callable, Dict, List, Optional, Tuple, Union, cast
+from urllib.parse import quote
 from uuid import uuid4
 
 from forestadmin.agent_toolkit.forest_logger import ForestLogger
@@ -84,16 +85,18 @@ def _serialize_one(
     ) -> DumpedResult:
         projection = projection if projection is not None else self.projection
         pk_value = self._get_id(collection, data)
+
+        encoded_pk_value = quote(pk_value, safe="") if isinstance(pk_value, str) else pk_value
         ret = {
             "data": {
                 "id": pk_value,
                 "attributes": {},
-                "links": {"self": f"/forest/{collection.name}/{pk_value}"},
+                "links": {"self": f"/forest/{collection.name}/{encoded_pk_value}"},
                 "relationships": {},
                 "type": collection.name,
             },
             "included": [],
-            "links": {"self": f"/forest/{collection.name}/{pk_value}"},
+            "links": {"self": f"/forest/{collection.name}/{encoded_pk_value}"},
         }
 
         first_level_projection = [*projection.relations.keys(), *projection.columns]
@@ -109,7 +112,7 @@ def _serialize_one(
                     key,
                     data,
                     cast(RelationAlias, collection.schema["fields"][key]),
-                    f"/forest/{collection.name}/{pk_value}",
+                    f"/forest/{collection.name}/{encoded_pk_value}",
                 )
                 ret["data"]["relationships"][key] = relation
                 if included is not None and not self._is_in_included(ret["included"], included):
@@ -207,10 +210,13 @@ def _serialize_to_one_relationships(
                 continue
             included_attributes[key] = self._serialize_value(value, foreign_collection.schema["fields"][key])
 
+        id_ = self._get_id(foreign_collection, data)
+        encoded_id = quote(id_, safe="") if isinstance(id_, str) else id_
+
         included = {
-            "id": self._get_id(foreign_collection, data),
+            "id": id_,
             "links": {
-                "self": f"/forest/{foreign_collection.name}/{self._get_id(foreign_collection, data)}",
+                "self": f"/forest/{foreign_collection.name}/{encoded_id}",
             },
             "type": foreign_collection.name,
         }

src/agent_toolkit/tests/resources/collections/test_crud.py

@@ -270,6 +270,29 @@ def _create_collections(cls):
             },
         )
 
+        # str as pk for url encoding
+
+        cls.collection_str_pk = cls._create_collection(
+            "StrPK",
+            {
+                "pk": {
+                    "column_type": PrimitiveType.STRING,
+                    "is_primary_key": True,
+                    "type": FieldType.COLUMN,
+                    "is_read_only": False,
+                    "default_value": None,
+                    "enum_values": None,
+                    "filter_operators": set([Operator.EQUAL, Operator.IN]),
+                    "is_sortable": True,
+                    "validations": [],
+                },
+                "name": {
+                    "column_type": "String",
+                    "type": "Column",
+                },
+            },
+        )
+
     @classmethod
     def setUpClass(cls) -> None:
         cls.loop = asyncio.new_event_loop()
@@ -293,6 +316,7 @@ def setUpClass(cls) -> None:
             "tag": cls.collection_tag,
             # for uuid
             "author": cls.collection_author,
+            "StrPK": cls.collection_str_pk,
         }
         cls.datasource_composite.add_datasource(cls.datasource)
 
@@ -1349,6 +1373,27 @@ def test_list_errors_jsonapi_error(self):
             "status": 500,
         }
 
+        request = RequestCollection(
+            RequestMethod.GET,
+            self.collection_order,
+            query={
+                "collection_name": "order",
+                "timezone": "Europe/Paris",
+                "fields[order]": "id,cost",
+            },
+            headers={},
+            client_ip="127.0.0.1",
+        )
+        crud_resource = CrudResource(
+            self.datasource_composite,
+            self.datasource,
+            self.permission_service,
+            self.ip_white_list_service,
+            self.options,
+        )
+        with patch.object(self.collection_order, "list", new_callable=AsyncMock, return_value=mock_orders):
+            response = self.loop.run_until_complete(crud_resource.list(request))
+
     # count
     def test_count(self):
         request = RequestCollection(

src/agent_toolkit/tests/services/serializers/test_jsonapi.py

@@ -456,6 +456,49 @@ def setUpClass(cls) -> None:
             }
         )
 
+        cls.collection_str_pk = Collection("StrPK", cls.datasource)  # type:ignore
+        cls.collection_str_pk.add_fields(
+            {
+                "pk": Column(
+                    column_type=PrimitiveType.STRING,
+                    is_primary_key=True,
+                    type=FieldType.COLUMN,
+                    is_read_only=False,
+                    default_value=None,
+                    enum_values=None,
+                    filter_operators=set([Operator.EQUAL, Operator.IN]),
+                    is_sortable=True,
+                    validations=[],
+                ),
+                "name": Column(column_type="String", type="Column"),
+                "relation_pk": Column(column_type="String", type="Column"),
+                "relation": ManyToOne(
+                    foreign_collection="StrPKRelation",
+                    foreign_key="relation_pk",
+                    foreign_key_target="pk",
+                    type=FieldType.MANY_TO_ONE,
+                ),
+            }
+        )
+
+        cls.collection_str_pk_relation = Collection("StrPKRelation", cls.datasource)  # type:ignore
+        cls.collection_str_pk_relation.add_fields(
+            {
+                "pk": Column(
+                    column_type=PrimitiveType.STRING,
+                    is_primary_key=True,
+                    type=FieldType.COLUMN,
+                    is_read_only=False,
+                    default_value=None,
+                    enum_values=None,
+                    filter_operators=set([Operator.EQUAL, Operator.IN]),
+                    is_sortable=True,
+                    validations=[],
+                ),
+                "name": Column(column_type="String", type="Column"),
+            }
+        )
+
         cls.datasource.add_collection(cls.collection_order)
         cls.datasource.add_collection(cls.collection_order_products)
         cls.datasource.add_collection(cls.collection_product)
@@ -464,6 +507,8 @@ def setUpClass(cls) -> None:
         cls.datasource.add_collection(cls.collection_picture)
         cls.datasource.add_collection(cls.collection_comment)
         cls.datasource.add_collection(cls.collection_all_types)
+        cls.datasource.add_collection(cls.collection_str_pk)
+        cls.datasource.add_collection(cls.collection_str_pk_relation)
 
 
 class TestJsonApiDeserializer(TestJsonApi):
@@ -1253,3 +1298,58 @@ def test_should_ignore_polymorphic_many_to_one_if_type_is_unknown(self):
                 ],
             },
         )
+
+    def test_string_primary_keys_should_be_url_encoded(self):
+        serializer = JsonApiSerializer(self.datasource, Projection("pk", "name"))
+        record = {"pk": "hello/world", "name": "hello world"}
+        dumped = serializer.serialize(record, self.collection_str_pk)
+        self.assertEqual(
+            dumped,
+            {
+                "data": {
+                    "type": "StrPK",
+                    "id": "hello/world",
+                    "attributes": {"pk": "hello/world", "name": "hello world"},
+                    "links": {"self": "/forest/StrPK/hello%2Fworld"},
+                },
+                "links": {"self": "/forest/StrPK/hello%2Fworld"},
+            },
+        )
+
+    def test_string_foreign_keys_should_be_url_encoded_so_foreign_pk(self):
+        serializer = JsonApiSerializer(
+            self.datasource, Projection("pk", "name", "relation_pk", "relation:pk", "relation:name")
+        )
+        record = {
+            "pk": "hello/world",
+            "name": "hello world",
+            "relation_pk": "hello/other/people",
+            "relation": {"pk": "hello/other/people", "name": "hello other people"},
+        }
+        dumped = serializer.serialize(record, self.collection_str_pk)
+        self.assertEqual(
+            dumped,
+            {
+                "data": {
+                    "type": "StrPK",
+                    "id": "hello/world",
+                    "attributes": {"pk": "hello/world", "name": "hello world", "relation_pk": "hello/other/people"},
+                    "links": {"self": "/forest/StrPK/hello%2Fworld"},
+                    "relationships": {
+                        "relation": {
+                            "data": {"id": "hello/other/people", "type": "StrPKRelation"},
+                            "links": {"related": {"href": "/forest/StrPK/hello%2Fworld/relationships/relation"}},
+                        }
+                    },
+                },
+                "links": {"self": "/forest/StrPK/hello%2Fworld"},
+                "included": [
+                    {
+                        "id": "hello/other/people",
+                        "links": {"self": "/forest/StrPKRelation/hello%2Fother%2Fpeople"},
+                        "type": "StrPKRelation",
+                        "attributes": {"pk": "hello/other/people", "name": "hello other people"},
+                    }
+                ],
+            },
+        )

src/django_agent/forestadmin/django_agent/urls.py

@@ -1,5 +1,5 @@
 from django.conf import settings
-from django.urls import path
+from django.urls import path, re_path
 
 from .views import actions, authentication, capabilities, charts, crud, crud_related, index, native_query, stats
 
@@ -52,24 +52,24 @@
     # stats
     path(f"{prefix}forest/stats/<str:collection_name>", stats.stats, name="stats"),
     # crud related
-    path(
-        f"{prefix}forest/<str:collection_name>/<str:pks>/relationships/<str:relation_name>/count",
+    re_path(
+        f"{prefix}forest/(?P<collection_name>[^/]+)/(?P<pks>.*)/relationships/(?P<relation_name>[^/]+)/count",
         crud_related.count,
         name="crud_related_count",
     ),
-    path(
-        f"{prefix}forest/<str:collection_name>/<str:pks>/relationships/<str:relation_name>.csv",
+    re_path(
+        f"{prefix}forest/(?P<collection_name>[^/]+)/(?P<pks>.*)/relationships/(?P<relation_name>[^/]+).csv",
         crud_related.csv,
         name="crud_related_csv",
     ),
-    path(
-        f"{prefix}forest/<str:collection_name>/<str:pks>/relationships/<str:relation_name>",
+    re_path(
+        f"{prefix}forest/(?P<collection_name>[^/]+)/(?P<pks>.*)/relationships/(?P<relation_name>[^/]+)",
         crud_related.list_,
         name="crud_related_list",
     ),
     # crud
     path(f"{prefix}forest/<str:collection_name>.csv", crud.csv, name="crud_csv"),
     path(f"{prefix}forest/<str:collection_name>/count", crud.count, name="crud_count"),
-    path(f"{prefix}forest/<str:collection_name>/<str:pks>", crud.detail, name="crud_detail"),
+    re_path(f"{prefix}forest/(?P<collection_name>[^/]+)/(?P<pks>.+)", crud.detail, name="crud_detail"),
     path(f"{prefix}forest/<str:collection_name>", crud.list_, name="crud_list"),
 ]