django-machina
django-machina copied to clipboard
Triple approval status that allows disapproved posts to be revised
Currently, disapproved posts are deleted, which does not allow posters to improve the posts and re-publish
This PR allows Post.approved to have an initial None (unknown) status, so that only None posts are moderated to be True or False. Most of the changes are from
filter(approved=True)
to
exclude(approved=False)
In addition,
Post.delete()will setapproved=Falsebefore the posts can be permanently deleted. This effectively allows for soft-deletion.- Consequently, the
self.object.delete()call inPostDisapproveViewwill only set theapprovedstatus, instead of actually deleting the post immediately. - Posters are allowed to view and edit disapproved posts, or permanently delete it.
The behavior is controlled by a setting MACHINA_DEFAULT_APPROVAL_STATUS to keep backward compatibility.
This is a quick PR in response to #246. I will improve it (check tests, templates etc) if @ellmetha shows enough interest.
@ellmetha Please let me know if this feature is potentially acceptable so that I can either close or improve it. Thanks.
Hey @BoPeng! 👋
I think that the idea of not deleting disapproved posts right away could be explored, but I don't think that this should be treated as a regular soft-deletion case. I see soft-deletion as a low-level mechanism that you can choose to use (or not use) for your models depending on the requirements of your application and how critical the data you are dealing with is.
In the present case I think that what we want is a new feature that would allow to better handle disapproved posts, but this should not translate in an override of the standard delete() method in my opinion. Indeed, with your proposal you are making the assumption that the deletion of a Post record that is not disapproved translates into the disapproval of it, but delete() is a very low-level method that shouldn't introduce such side effects in my opinion (because this could lead to some unwanted consequences).
Maybe the approved field should be replaced by something more powerful like a proper status field that could accept more values (such as approved, pending_approval, disapproved). Using a boolean for this can be tempting but this is very limiting and not future-proof (what if more status values have to be added later on?).
This would also require a few more things like:
- the ability to see the posts that were disapproved
- the ability to undo the disapproval of a post
So to answer your question I would be open with to addition of such feature, but I am against leveraging a soft-deletion mechanism for this: it appears that what we want is to "disapprove" posts, and not delete them. So the current Pull Request would need to be revisited.
Also, this is a very big feature that would probably introduce backward incompatible changes, which would translate into a new major release. I won't have time to work on this personally as I am very involved with another project at the moment, but if you want to keep working on this and implement the full solution I'll definitely review your Pull Request.
Hi, @ellmetha
This PR is essentially a quick patch to support my own website so the implementation was certainly not finalized (and overriding delete() was just a less-invasive change compared to replacing delete() with disapprove() everywhere it is called). Let us start from deciding what approval status we should have. Currently,
- Posts can have default
approved=Truestatus and somehow be assignedapproved=False(perhaps reported by users), or posts can have defaultapproved=Falseso all posts need to be moderated before going public. - Admin moderates all
approved=Falseposts. Either approve them or delete them.
So essentially speaking, approved=False is a misnomer that actually means pending moderation.
To allow users to revise disapproved posts, the moderation process has to be changed:
-
Posts should have status
approved(the same asapproved=True),pending moderation(the same asapproved=Falseabove), anddisapproved(viewable only to poster). -
Admin moderates all
pending moderationposts, and change them toapprovedordisapprovedstatus.
The PR therefore has
approved=Trueasapprovedapproved=Noneaspending moderationapproved=Falseasdisapproved
There could be another status revised but I treat it the same as pending moderation. I cannot think of another moderation-related status.
Maybe the approved field should be replaced by something more powerful like a proper status field that could accept more values (such as approved, pending_approval, disapproved). Using a boolean for this can be tempting but this is very limiting and not future-proof (what if more status values have to be added later on?).
So we agree with each other on the three states here but disagree on reusing the approved field. I much prefer reusing approved because the changes are much less invasive, and are very likely to be good enough. Actually, my project already adds an status field for transaction status so adding status is out of the question for me.
@ellmetha I have updated the PR
- use
approve()anddisapprove()instead of overridingdelete(). - add an additional setting to control if pending posts are displayed.
I continued to use the approved field to keep backward compatibility. If you believe that another more general field could be used, we can expose APPROVED_FILTER (which is currently derived from other settings) so that users can define it using their own fields.
I can work on tests if you agree with the direction this PR is going.