django-oscar / django-oscar-api

RESTful JSON API for django-oscar
Other
369 stars 160 forks source link

ImproperlyConfigured at /api/admin/products/ when a Product has an optiongroup #324

Open JulienPalard opened 8 months ago

JulienPalard commented 8 months ago

Steps to reproduce:

From now, querying /api/admin/products/ gives:

Could not resolve URL for hyperlinked relationship using view name "attributeoptiongroup-detail". You may have failed to include the related model in your API, or incorrectly configured the `lookup_field` attribute on this field.
JulienPalard commented 8 months ago

In urls.py I see:

    path(
        "attributeoptiongroups/<int:pk>/",
        AttributeOptionGroupAdminDetail.as_view(),
        name="admin-attributeoptiongroup-detail",
    ),

so it's not the same name: attributeoptiongroup-detail vs admin-attributeoptiongroup-detail.

specialunderwear commented 8 months ago

I can not reproduced this at what url are you filing this in exactly? "Assign an option to the product from the admin by clicking on the "+" near "Product options", named opt1, choosing type: select, and assigning the option to the productgroup. Save the product."

JulienPalard commented 8 months ago

I can not reproduced this at what url are you filing this in exactly?

In Django admin, in the product edit view.

JulienPalard commented 8 months ago

II was able to reproduce it in django-oscar sandbox, by setting up django-oscar-api:

diff --git a/sandbox/settings.py b/sandbox/settings.py
index 6dd1c0858..64d578f86 100644
--- a/sandbox/settings.py
+++ b/sandbox/settings.py
@@ -259,6 +259,8 @@ INSTALLED_APPS = [
     'django.contrib.sites',
     'django.contrib.flatpages',

+    'rest_framework', 'oscarapi',
+
     'oscar.config.Shop',
     'oscar.apps.analytics.apps.AnalyticsConfig',
     'oscar.apps.checkout.apps.CheckoutConfig',
@@ -312,7 +314,7 @@ AUTHENTICATION_BACKENDS = (
     'oscar.apps.customer.auth_backends.EmailBackend',
     'django.contrib.auth.backends.ModelBackend',
 )
-
+OSCARAPI_BLOCK_ADMIN_API_ACCESS = False
 AUTH_PASSWORD_VALIDATORS = [
     {
         'NAME': 'django.contrib.auth.password_validation.MinimumLengthValidator',
diff --git a/sandbox/urls.py b/sandbox/urls.py
index 01f3b6ec0..3f14bd126 100644
--- a/sandbox/urls.py
+++ b/sandbox/urls.py
@@ -25,9 +25,11 @@ urlpatterns = [
         {'sitemaps': base_sitemaps}),
     path('sitemap-<slug:section>.xml', views.sitemap,
         {'sitemaps': base_sitemaps},
-        name='django.contrib.sitemaps.views.sitemap')
+         name='django.contrib.sitemaps.views.sitemap'),
+        path("api/", include("oscarapi.urls")),
 ]

+
 # Prefix Oscar URLs with language codes
 urlpatterns += i18n_patterns(
     path('', include(apps.get_app_config('oscar').urls[0])),

and by creating a fixture of the datail causing the issue:

[
  {
    "model": "catalogue.productclass",
    "pk": 4,
    "fields": {
      "name": "productclass1",
      "slug": "productclass1",
      "requires_shipping": true,
      "track_stock": true,
      "options": []
    }
  },
  {
    "model": "catalogue.product",
    "pk": 211,
    "fields": {
      "structure": "standalone",
      "is_public": true,
      "upc": "",
      "parent": null,
      "title": "product1",
      "slug": "product1",
      "description": "",
      "meta_title": null,
      "meta_description": "",
      "product_class": 4,
      "rating": null,
      "date_created": "2024-01-15T14:04:23.246Z",
      "date_updated": "2024-01-15T14:04:23.246Z",
      "is_discountable": true,
      "product_options": [
        3
      ]
    }
  },
  {
    "model": "catalogue.attributeoptiongroup",
    "pk": 3,
    "fields": {
      "name": "attributeoptiongroup1",
      "code": ""
    }
  },
  {
    "model": "catalogue.option",
    "pk": 3,
    "fields": {
      "name": "option1",
      "code": "option1",
      "type": "select",
      "required": false,
      "option_group": 3,
      "help_text": null,
      "order": null
    }
  }
]

Loaded the data using ./manage.py loaddata ./repro.json and hit http://127.0.0.1:8000/api/admin/products/ (after logging in as superadmin).

JulienPalard commented 8 months ago

Looks like setting:

option_group = serializers.HyperlinkedIdentityField(view_name="admin-attributeoptiongroup-detail")

in OptionSerializer does help but as I don't fully understand what are those attributes/options/optiongroup yet, I'm not confident at all.

All of this is a bit strange to me:

JulienPalard commented 8 months ago

I can also reproduce the bug by linking options to a product via a ProductClass instead of directly to the product.

So it should be reproductible via the dashboard, by creating an option of kind select in a product type, creating a product of this type, and accessing it from the API.

specialunderwear commented 8 months ago

I can not reproduced this at what url are you filing this in exactly?

In Django admin, in the product edit view.

Please paste the link from your local machine. And also a screenshot please, I can't understand how you are assigning option to products, they are only useable as attributevalues and productoption choices

specialunderwear commented 8 months ago

II was able to reproduce it in django-oscar sandbox, by setting up django-oscar-api:

diff --git a/sandbox/settings.py b/sandbox/settings.py
index 6dd1c0858..64d578f86 100644
--- a/sandbox/settings.py
+++ b/sandbox/settings.py
@@ -259,6 +259,8 @@ INSTALLED_APPS = [
     'django.contrib.sites',
     'django.contrib.flatpages',

+    'rest_framework', 'oscarapi',
+
     'oscar.config.Shop',
     'oscar.apps.analytics.apps.AnalyticsConfig',
     'oscar.apps.checkout.apps.CheckoutConfig',
@@ -312,7 +314,7 @@ AUTHENTICATION_BACKENDS = (
     'oscar.apps.customer.auth_backends.EmailBackend',
     'django.contrib.auth.backends.ModelBackend',
 )
-
+OSCARAPI_BLOCK_ADMIN_API_ACCESS = False
 AUTH_PASSWORD_VALIDATORS = [
     {
         'NAME': 'django.contrib.auth.password_validation.MinimumLengthValidator',
diff --git a/sandbox/urls.py b/sandbox/urls.py
index 01f3b6ec0..3f14bd126 100644
--- a/sandbox/urls.py
+++ b/sandbox/urls.py
@@ -25,9 +25,11 @@ urlpatterns = [
         {'sitemaps': base_sitemaps}),
     path('sitemap-<slug:section>.xml', views.sitemap,
         {'sitemaps': base_sitemaps},
-        name='django.contrib.sitemaps.views.sitemap')
+         name='django.contrib.sitemaps.views.sitemap'),
+        path("api/", include("oscarapi.urls")),
 ]

+
 # Prefix Oscar URLs with language codes
 urlpatterns += i18n_patterns(
     path('', include(apps.get_app_config('oscar').urls[0])),

and by creating a fixture of the datail causing the issue:

[
  {
    "model": "catalogue.productclass",
    "pk": 4,
    "fields": {
      "name": "productclass1",
      "slug": "productclass1",
      "requires_shipping": true,
      "track_stock": true,
      "options": []
    }
  },
  {
    "model": "catalogue.product",
    "pk": 211,
    "fields": {
      "structure": "standalone",
      "is_public": true,
      "upc": "",
      "parent": null,
      "title": "product1",
      "slug": "product1",
      "description": "",
      "meta_title": null,
      "meta_description": "",
      "product_class": 4,
      "rating": null,
      "date_created": "2024-01-15T14:04:23.246Z",
      "date_updated": "2024-01-15T14:04:23.246Z",
      "is_discountable": true,
      "product_options": [
        3
      ]
    }
  },
  {
    "model": "catalogue.attributeoptiongroup",
    "pk": 3,
    "fields": {
      "name": "attributeoptiongroup1",
      "code": ""
    }
  },
  {
    "model": "catalogue.option",
    "pk": 3,
    "fields": {
      "name": "option1",
      "code": "option1",
      "type": "select",
      "required": false,
      "option_group": 3,
      "help_text": null,
      "order": null
    }
  }
]

Loaded the data using ./manage.py loaddata ./repro.json and hit http://127.0.0.1:8000/api/admin/products/ (after logging in as superadmin).

This fixture in no way connects the option to the product. How can it cause an error?

JulienPalard commented 8 months ago

This fixture in no way connects the option to the product. How can it cause an error?

Good question, in doubt, let me try again from a clean state in a clean new venv:

$ cd django-oscar
$ rm -fr sandbox
$ git checkout -- sandbox
$ git pull --ff-only
$ pip install .
$ make sandbox
$ cd sandbox
$ pip install django-oscar-api
$ cat | git apply -
diff --git a/sandbox/settings.py b/sandbox/settings.py
index 6dd1c0858..64d578f86 100644
--- a/sandbox/settings.py
+++ b/sandbox/settings.py
@@ -259,6 +259,8 @@ INSTALLED_APPS = [
     'django.contrib.sites',
     'django.contrib.flatpages',

+    'rest_framework', 'oscarapi',
+
     'oscar.config.Shop',
     'oscar.apps.analytics.apps.AnalyticsConfig',
     'oscar.apps.checkout.apps.CheckoutConfig',
@@ -312,7 +314,7 @@ AUTHENTICATION_BACKENDS = (
     'oscar.apps.customer.auth_backends.EmailBackend',
     'django.contrib.auth.backends.ModelBackend',
 )
-
+OSCARAPI_BLOCK_ADMIN_API_ACCESS = False
 AUTH_PASSWORD_VALIDATORS = [
     {
         'NAME': 'django.contrib.auth.password_validation.MinimumLengthValidator',
diff --git a/sandbox/urls.py b/sandbox/urls.py
index 01f3b6ec0..3f14bd126 100644
--- a/sandbox/urls.py
+++ b/sandbox/urls.py
@@ -25,9 +25,11 @@ urlpatterns = [
         {'sitemaps': base_sitemaps}),
     path('sitemap-<slug:section>.xml', views.sitemap,
         {'sitemaps': base_sitemaps},
-        name='django.contrib.sitemaps.views.sitemap')
+         name='django.contrib.sitemaps.views.sitemap'),
+        path("api/", include("oscarapi.urls")),
 ]

+
 # Prefix Oscar URLs with language codes
 urlpatterns += i18n_patterns(
     path('', include(apps.get_app_config('oscar').urls[0])),

$ ./manage.py migrate
(Starting a `./manage.py runserver` in another shell)
$ curl --user superuser@example.com:testing localhost:8000/api/admin/products/  -I
HTTP/1.1 200 OK  # Which is expected, I did not installed any fixture
## BEWARE that's not **exactly** the same json, I had to remove a line about the code of the attributeoptiongroup.
$ cat > repro.json 
[
  {
    "model": "catalogue.productclass",
    "pk": 4,
    "fields": {
      "name": "productclass1",
      "slug": "productclass1",
      "requires_shipping": true,
      "track_stock": true,
      "options": []
    }
  },
  {
    "model": "catalogue.product",
    "pk": 211,
    "fields": {
      "structure": "standalone",
      "is_public": true,
      "upc": "",
      "parent": null,
      "title": "product1",
      "slug": "product1",
      "description": "",
      "meta_title": null,
      "meta_description": "",
      "product_class": 4,
      "rating": null,
      "date_created": "2024-01-15T14:04:23.246Z",
      "date_updated": "2024-01-15T14:04:23.246Z",
      "is_discountable": true,
      "product_options": [
        3
      ]
    }
  },
  {
    "model": "catalogue.attributeoptiongroup",
    "pk": 3,
    "fields": {
      "name": "attributeoptiongroup1"
    }
  },
  {
    "model": "catalogue.option",
    "pk": 3,
    "fields": {
      "name": "option1",
      "code": "option1",
      "type": "select",
      "required": false,
      "option_group": 3,
      "help_text": null,
      "order": null
    }
  }
]

$ ./manage.py loaddata ./repro.json
$ curl --user superuser@example.com:testing localhost:8000/api/admin/products/  -I
HTTP/1.1 500 Internal Server Error

So hum yes that's strange, let's understand why, I bet my fixture just "reuses" existing data:

$ sqlite3 db.sqlite3
sqlite> select * from catalogue_product_product_options;
1|211|3

Boom here it is, there's a link between product 211 and option 3.

Ohhh wait wait wait I see it now in the repro.json, in the product:

      "product_options": [
        3
      ]

So, hum, yes I understand know, it may be strange to assign options to product directly, but it looks like it's supported by oscar, see https://django-oscar.readthedocs.io/en/latest/ref/apps/catalogue.html#oscar.apps.catalogue.abstract_models.AbstractProduct.product_options:

It’s possible to have options product class-wide, and per product.

If I now change the OptionSerializer to look like:

class OptionSerializer(OscarHyperlinkedModelSerializer):
    code = serializers.SlugField()
    option_group = serializers.HyperlinkedIdentityField(view_name="admin-attributeoptiongroup-detail")

    class Meta:
        model = Option
        fields = settings.OPTION_FIELDS
        list_serializer_class = UpdateForwardManyToManySerializer

(I just made the option_group field explicit)

it works again, the serialized product looks like:

  {
    "id": 211,
    "attributes": [],
    "categories": [],
    "product_class": "productclass1",
    "options": [
      {
        "url": "http://localhost:8000/api/options/3/",
        "code": "option1",
        "option_group": "http://localhost:8000/api/admin/attributeoptiongroups/3/",
        "name": "option1",
        "type": "select",
        "required": false,
        "help_text": null,
        "order": null
      }
    ],
    ...

I'm digging further, I'll remove my strange link between a product and an option, and recreate a link through a product type.

So it looks like that it does not depend if I add an option through a product type or to the product directly I have the exception in both cases.

JulienPalard commented 8 months ago

My proposed fix won't be enough though: it would create a link to /admin/attributeoptiongroups/{id}/ from /products/{id}.

Maybe a public /attributeoptiongroups/ is needed too?

JulienPalard commented 8 months ago

I'm also having this issue under /api/options/, probably due to the exact same reason.

JulienPalard commented 7 months ago

A better fix could be:

class OptionSerializer(oscarapi.serializers.product.OptionSerializer):
    option_group = oscarapi.serializers.product.AttributeOptionGroupSerializer()

So it looks like:

    "options": [
        {
            "url": "http://localhost:8000/api/options/13/",
            "code": "test-option",
            "option_group": {
                "url": "http://localhost:8000/api/admin/attributeoptiongroups/5/",
                "options": [
                    "youpi",
                    "test",
                    "foo",
                    "bar"
                ],
                "name": "Test Option",
                "code": ""
            },
            "name": "Test Select",
            "type": "select",
            "required": false,
            "help_text": null,
            "order": null
        }
    ],
ghost commented 6 months ago

Had the same issue. Thank you, @JulienPalard !