Fix overriding form field kwargs

[ddabble]

This should provide a better fix for #708 (which was caused by a bug in #707), while also enabling code to override the widget keyword argument of RichTextFormField and RichTextUploadingFormField. Also added some tests for the mentioned bug.

See each commit message for more details.

[matthiask]

Hi @ddabble

The CKEditor widget doesn't seem to appear (again) after applying this pull request.

You can run e.g. SELENIUM=firefox tox -e py39-dj40 locally (if geckodriver is available) to run the tests and see for yourself. Merging this would be a regression for now.

[matthiask]

The problem when using the rich text field in the admin panel may be that the admin panel unconditionally specifies a widget and this widget always overrides the CKEditor widget. It may work outside django.contrib.admin (the unittests should verify this) but it's really important that the CKEditor widget is also shown inside django.contrib.admin.

I'm not sure. Why would you want to override the widget instance when using the fields provided by django-ckeditor, when basically their only reason of existing is that they override the widget for you? You could always use a bare models.TextField and specify the CKEditor widget if you didn't want this behavior.

[ddabble]

The CKEditor widget doesn't seem to appear (again) after applying this pull request.

Hm, that's slightly annoying 😅

You make some good points; I'll have to look at it when I have the time 🙂

[ddabble]

@matthiask I found a fairly simple fix: adding empty formfield defaults in Django's global FORMFIELD_FOR_DBFIELD_DEFAULTS dictionary (used in the admin forms); see 6d5e167. It's not the prettiest, but it works well and should be both robust and non-interfering with other developers' code 🙂

[matthiask]

I'm sorry but I'm still not convinced. You're writing all this code and changing private (undocumented) data structures of django.contrib.admin to avoid the only behavior which is actually added by the RichTextField and the RichTextFormField, and that is adding the CKEditorWidget: https://github.com/django-ckeditor/django-ckeditor/blob/master/ckeditor/fields.py

If you wanted to specify your own widget why even use the RichTextField and RichTextFormField in the first place? You could simply use a standard models.TextField and a forms.CharField(widget=CKEditorWidget(...)) while being more explicit and relying on composition instead of inheritance, which seems strictly better to me.

[ddabble]

changing private (undocumented) data structures of django.contrib.admin

Yes, I agree; it's not pretty, despite its few strengths, as I mentioned.

[...] to avoid the only behavior which is actually added by the RichTextField and the RichTextFormField, and that is adding the CKEditorWidget

That's actually a fair point I hadn't realized until now 😅 I think the original "hunch" I had, was that making behavior like this overridable, always has enough merit for it to be done - especially in case the code ever updates with added functionality (even though this might not be very likely when considering the particular model and form fields of this project). So in this case, it might just be prematurely adding functionality that's never going to be used (i.e. YAGNI).

[ddabble]

But I guess some of the code can still be used, like most of the tests 🙂 @matthiask Should I add a commit to this PR that removes the code letting the widget be overridden, or should I open a new PR where the commits have been rebased to only include the usable code?

[matthiask]

Yes, indeed. Do what is easier for you, both ways are good. I slightly prefer the rebased version because it's a bit cleaner.

[ddabble]

Moved the usable code to #715.