removing FlatPage.enable_comments field?

133 views
Skip to first unread message

Tim Graham

unread,
Aug 11, 2015, 6:43:29 PM8/11/15
to Django developers (Contributions to Django itself)
Aymeric raised a ticket [1] noting that the FlatPage.enable_comments field appears in the admin by default, but isn't used anywhere which could be a bit confusing. It seems that django-contrib-comments has a moderation feature where you can specify a field on your model to control how comments are handled [2], so this was presumably used for that. I'm not sure removing the field is a great optional unless we introduce a setting to make the flatpage model swappable (do we want more of those?!) for backwards compatibility. A compromise could be to hide the field in the default ModelAdmin and let those who want it to enable it with a custom ModelAdmin. What do you think?

[1] https://code.djangoproject.com/ticket/25262
[2] http://django-contrib-comments.readthedocs.org/en/latest/moderation.html

Fabrizio Messina

unread,
Aug 12, 2015, 2:52:10 AM8/12/15
to Django developers (Contributions to Django itself)
I think that the compromise is a good idea, but on the long term it implies the creation of a field in order to support a third party library.

While there are historical reasons it would be best to just let the third party library solve this problem on its own.

Loïc Bistuer

unread,
Aug 13, 2015, 2:39:40 AM8/13/15
to django-d...@googlegroups.com
How about we deprecate contrib.flatpages:

- I see people abuse it all the time (do way more with it than they should) just because it's in core.

- FlatpageFallbackMiddleware is hardly an example to follow.

- It can easily live as a third-party app.

Thoughts?

--
Loïc
> --
> You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
> To post to this group, send email to django-d...@googlegroups.com.
> Visit this group at http://groups.google.com/group/django-developers.
> To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/ddffdfa8-9c6c-4d42-a816-1717478313f5%40googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

Jannis Leidel

unread,
Aug 13, 2015, 3:41:02 AM8/13/15
to django-d...@googlegroups.com

> On 13 Aug 2015, at 08:39, Loïc Bistuer <loic.b...@gmail.com> wrote:
>
> How about we deprecate contrib.flatpages:
>
> - I see people abuse it all the time (do way more with it than they should) just because it's in core.
>
> - FlatpageFallbackMiddleware is hardly an example to follow.
>
> - It can easily live as a third-party app.
>
> Thoughts?

+1 Move it into another project under the django/ namespace.

Jannis

>
> --
> Loïc
>
>> On Aug 12, 2015, at 05:43, Tim Graham <timog...@gmail.com> wrote:
>>
>> Aymeric raised a ticket [1] noting that the FlatPage.enable_comments field appears in the admin by default, but isn't used anywhere which could be a bit confusing. It seems that django-contrib-comments has a moderation feature where you can specify a field on your model to control how comments are handled [2], so this was presumably used for that. I'm not sure removing the field is a great optional unless we introduce a setting to make the flatpage model swappable (do we want more of those?!) for backwards compatibility. A compromise could be to hide the field in the default ModelAdmin and let those who want it to enable it with a custom ModelAdmin. What do you think?
>>
>> [1] https://code.djangoproject.com/ticket/25262
>> [2] http://django-contrib-comments.readthedocs.org/en/latest/moderation.html
>>
>> --
>> You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
>> To post to this group, send email to django-d...@googlegroups.com.
>> Visit this group at http://groups.google.com/group/django-developers.
>> To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/ddffdfa8-9c6c-4d42-a816-1717478313f5%40googlegroups.com.
>> For more options, visit https://groups.google.com/d/optout.
>
> --
> You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
> To post to this group, send email to django-d...@googlegroups.com.
> Visit this group at http://groups.google.com/group/django-developers.
> To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/A201319D-6EC9-486F-B789-1365B0372A7E%40gmail.com.

Carl Meyer

unread,
Aug 13, 2015, 11:57:17 AM8/13/15
to django-d...@googlegroups.com
On 08/13/2015 12:39 AM, Loïc Bistuer wrote:
> How about we deprecate contrib.flatpages:

I'm not sure I agree with this. I think flatpages is actually an example
of the kind of thing that belongs in contrib: a simple app to fulfill a
simple, frequent need.

> - I see people abuse it all the time (do way more with it than they should) just because it's in core.

What kinds of things do you see people doing with it that they shouldn't?

> - FlatpageFallbackMiddleware is hardly an example to follow.

I agree - I'd be happy to deprecate that middleware, I don't think it's
necessary or a best practice for use of flatpages.

> - It can easily live as a third-party app.

Sure, it can. And I don't really use it, so it won't bother me if it
does. Call me -0.

Carl

signature.asc

James Bennett

unread,
Aug 13, 2015, 1:03:26 PM8/13/15
to django-d...@googlegroups.com
On Tue, Aug 11, 2015 at 5:43 PM, Tim Graham <timog...@gmail.com> wrote:
It seems that django-contrib-comments has a moderation feature where you can specify a field on your model to control how comments are handled [2], so this was presumably used for that.


The field on FlatPage has been there since the beginning. The moderation stuff in the comments app was added later (I originally wrote it as a third-party app, and it got integrated into contrib.comments afterward).

As to removing the field from FlatPage, I'm ambivalent enough to be against it just because removing it is too much effort. But if a stronger argument can be made for getting rid of it, I could change my mind.

Aymeric Augustin

unread,
Aug 13, 2015, 4:01:59 PM8/13/15
to django-d...@googlegroups.com
On 13 août 2015, at 08:39, Loïc Bistuer <loic.b...@gmail.com> wrote:

> How about we deprecate contrib.flatpages:

I think it’s sufficiently widely used and not sufficiently bad to make deprecation a good tradeoff.

> - I see people abuse it all the time (do way more with it than they should) just because it's in core.

Perhaps we could improve the situation by better documenting the intended use case, pointing out inappropriate use cases, and suggesting alternatives.

> - FlatpageFallbackMiddleware is hardly an example to follow.

We can fix that with the URL resolving improvements. (I’m not sure what the status of the GSoC is).

> - It can easily live as a third-party app.

That’s true of any contrib app :-)

--
Aymeric.

Fabrizio Messina

unread,
Aug 14, 2015, 3:35:32 AM8/14/15
to Django developers (Contributions to Django itself)
On Thursday, August 13, 2015 at 7:03:26 PM UTC+2, James Bennett wrote:

As to removing the field from FlatPage, I'm ambivalent enough to be against it just because removing it is too much effort. But if a stronger argument can be made for getting rid of it, I could change my mind.

I don't think these are strong arguments but lets recap the arguments pro removal and decide if these are a reason to care:
  1. Up to what I think flat pages should not be intended for a blog like thing but for literal flat pages like "/contact" or "/about", usually one does not want comments in that. Using contrib.flatpages + contrib.comment was probably a "marketing necessity" in order to better withstand Php CMS like Wordpress or Joomla,  since most developers were coming from these projects and these features are pretty common in there. Up to my opinion there is little justification to do this now.
  2. I think that this hook give a risk of implicit coupling, putting it simple flatpages seem to "expect" the presence of a third party app using it, and could change its behavior accordingly, or in other words shutting off contrb.comments, or another similar app, could broke the behavior of contrib.flatpages: the very fact that comments are not enabled when one sign enable_comments is a broken behavior.
  3. Developers reading the model definition could have doubts on how to use this field, or even suspect that flatpages has some comment ability hidden somewhere, 
  4. The boolean could be hacked to do out-of-scope things like pseudo-categories etc. if it is truthful that developers are responsible for their code  it is a fact that young developer do a lot of rubbish, a framework should up to some extent protect them from this or, at least, not offer easy-stupid hacks openings that will burn them later on.
 On the other hand there is potentially a lot of existing code that depends on this field and would be irreparably broken when this field will be shut down. This means a lot of work to design, document and develop a fallback solution.

Aymeric Augustin

unread,
Aug 14, 2015, 3:53:35 AM8/14/15
to django-d...@googlegroups.com
On 12 août 2015, at 00:43, Tim Graham <timog...@gmail.com> wrote:

> A compromise could be to hide the field in the default ModelAdmin and let those who want it to enable it with a custom ModelAdmin. What do you think?

That would resolve my problem of “WTF is this field” without causing too much headache.

We should also put a comment in the source code stating that the field is unused but we don’t want to remove it because of backwards-compatibility.

--
Aymeric.




Shai Berger

unread,
Aug 14, 2015, 12:22:29 PM8/14/15
to django-d...@googlegroups.com


On 13 באוגוסט 2015 19:03:09 CEST, James Bennett <ubern...@gmail.com> wrote:

>>
>The field on FlatPage has been there since the beginning. The
>moderation
>stuff in the comments app was added later (I originally wrote it as a
>third-party app, and it got integrated into contrib.comments
>afterward).
>
So, what was the field for? Is the purpose still pertinent? If so, since there are no uses in current code, how is it addressed?

--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Aymeric Augustin

unread,
Aug 14, 2015, 3:22:45 PM8/14/15
to django-d...@googlegroups.com
On 14 août 2015, at 18:22, Shai Berger <sh...@platonix.com> wrote:

> So, what was the field for? Is the purpose still pertinent? If so, since there are no uses in current code, how is it addressed?

I reviewed `git log -S enable_comments`. All occurrences fall in one of three categories: refactoring the FlatPage model or its admin, adding or removing test fixtures, and changing the comment moderation example in the docs.

enable_comments was already there in the initial public release of Django. The model was called FlatFile and lived in django/models/core.py. There’s never been any reference to this field anywhere other than the model and its admin.

The documentation of comments moderation system included a blog Entry model which had an enable_comments field. But there wasn’t any special integration with flatpages. As far as I can tell, we’re just being careful in case some developers have interpreted the naming coincidence as an encouragement to use this field to control comments on flat pages.

Hiding the field may be a step towards a future removal.

--
Aymeric.

[1] https://docs.djangoproject.com/en/1.6/ref/contrib/comments/moderation/#overview

Reply all
Reply to author
Forward
0 new messages