search

danirus / django-comments-xtd

A pluggable Django comments application with thread support, follow-up notifications, mail confirmation, like/dislike flags, moderation, a ReactJS plugin and Bootstrap 5.3.
https://django-comments-xtd.readthedocs.io
BSD 2-Clause "Simplified" License
594 stars 157 forks source link

fix defaults for DRF views #338

Closed PetrDlouhy closed 3 years ago

PetrDlouhy commented 3 years ago

Fixes problems described in #334

Although I can fix this in my project, I think that using project wise DRF defaults is not something, that should django-comments-xtd do, especially if they can break the JavaScript code.

PetrDlouhy commented 3 years ago

This might be something for further discussion because there might be two uses of the API - one for the JS loading and other for communication with different infrastructure. I have this exact case in my project (BlenderKit.com). We need JS comments on frontend, but also we would like to be able to access them from Blender add-on.

In the case of JS loading, the API needs to be exactly the same as it was used in this library. But in the case of library API, I would like to keep consistency with the rest of current API - camelcase names and same basic API endpoint. So I would like to connect second version of API with project wise defaults.

PetrDlouhy commented 3 years ago

I have figured this out. I have moved the API urls into separate file and set there kwargs property, that switches off the DRF defaults. So now I can include the API endpoint to my project API with my project defaults and still have the JS comments vorking on frontend. I include it with:

path('comments/', include('django_comments_xtd.api.urls')),

I also changed few re_paths into simple paths.

danirus commented 3 years ago

Thanks @PetrDlouhy! I think you forgot to add the module django_comments_xtd/api/urls.py.

PetrDlouhy commented 3 years ago

@danirus You are right. Now I have added it.

danirus commented 3 years ago

Thanks @PetrDlouhy. I like your contribution. There are two little PEP8 issues. If you could split the lines mentioned in the report, so that they don't go over 80 characters long, it will be done.