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 158 forks source link

Follow-up Tick Box customizable via a setting #206

Closed danirus closed 3 years ago

danirus commented 4 years ago

Create a new setting to customize the default value of the follow-up field in the comment form. The new setting could be called COMMENTS_XTD_DEFAULT_FOLLOW_UP and it could be True or False.

drholera commented 3 years ago

Hello @danirus, could I take a look at this issue?

As I understood right if the COMMENTS_XTD_DEFAULT_FOLLOW_UP setting is switched to False we shouldn't display the "Follow up" checkbox, am I right?

Thanks

danirus commented 3 years ago

Hi @drholera, Yes, you can adopt it. Please add some unit tests too. The idea is to use it along with the XtdCommentForm, to provide a default value for the followup field. To avoid displaying the field in the form the user can edit/replace the form.html template and the form validation will accept the value given in the COMMENTS_XTD_DEFAULT_FOLLOW_UP setting, because the field is not required.

drholera commented 3 years ago

Hello @danirus The PR is added, could you please check if it's what we expect here?

danirus commented 3 years ago

Hi @drholera, thanks for adopting it. You have implemented something different.

This new setting is only to feed the form followup field with a default value. There is no need to change the template. Adopters of this app can change the template at will if they want to hide the field. So the implementation is far easier than what you have done. It requires changes in the defaults.py module (to include the new setting, with the default value to False to not break backward compatibility), the form.py module (to feed the followup field with the default attribute coming from this setting) and a couple of tests to check how the form is rendered when the setting takes True and False values.

drholera commented 3 years ago

Thank you for the clarification. I've created a new PR, I think now it should be better. Could you please check if it's what we need?

In this PR we have a new setting COMMENTS_XTD_DEFAULT_FOLLOW_UP with the default value False. If we'll switch it to True - the 'followup' checkbox on the comment form will become checked. Also added tests there.

ogurec-ogurec commented 3 years ago

I understand correctly that after setting the COMMENTS_XTD_DEFAULT_FOLLOW_UP = True setting, the Notify "about subsequent comments checkbox should" be enabled? After updating the setting, the checkmark is still unchecked :(

drholera commented 3 years ago

You should use the COMMENTS_XTD_DEFAULT_FOLLOWUP setting, without the underscore.

danirus commented 3 years ago

Sorry about that. We forgot to modify the docs to mention the new setting.

drholera commented 3 years ago

Hello @danirus Added #283, could you please check if it's enough or we need to add some additional info?

ogurec-ogurec commented 3 years ago

Thanks!, added setting to setting.py: COMMENTS_XTD_DEFAULT_FOLLOWUP = True But all the same, the checkmark in the comments is inactive (

danirus commented 3 years ago

Can you run your project's shell command and run this code?

>>> from django_comments_xtd import get_form, get_version
>>> get_version()
'2.8.3'
>>> get_form()
django_comments_xtd.forms.XtdCommentForm

What do they return? If the version is older than 2.8.3, the setting was not implemented yet. It's only available as of 2.8.3. If on the other hand the form is a class other than django_comments_xtd.forms.XtdCommentForm, the class you would be using might implement the followup field without taking into account the new setting.

Let us know.

danirus commented 3 years ago

@ogurec-ogurec, this questions would better be in Discussions.