Skip to content
This repository has been archived by the owner on Nov 26, 2024. It is now read-only.

Fix moderation #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

joecot
Copy link

@joecot joecot commented Mar 27, 2017

We do not want to automatically moderate all members of a group if the group is set to moderate specific members, and there are none moderated currently

…e group is set to moderate specific members, and there are none moderated currently
@mpj17
Copy link
Member

mpj17 commented Mar 29, 2017

You are totally right, but I am still loathe to pull this. The problem is that the utterly broken behaviour has been the behaviour for over a decade, and I don't want to break existing systems with little benefit.

Tell you what, I will take the heat for this if you can give me a heap of unit-tests to ensure that GSGroupMembersInfo.moderatees works in a whole heap of cases. Deal?

@joecot
Copy link
Author

joecot commented Mar 29, 2017

Currently, if you have a list set to moderate specific members, with no one in the moderatee list, and you go to add a member to the moderatee list, you cannot add them -- because the code returns all members as moderated, it's already in the list, and therefore you can't add members.

You have to set it to moderate new members, add the member, and then set it back to moderate specific members.

So the functionality is already broken.

@mpj17
Copy link
Member

mpj17 commented Mar 29, 2017

Awesome, so I take from your response that you will write the unit tests. I will make the merge when they are in the PR ;)

@joecot
Copy link
Author

joecot commented Mar 29, 2017

My point was that no one using it currently can moderate members without jumping through hoops -- it's not a functionality preference issue, using "Moderate specified members only" just doesn't work currently. If set to that setting, it moderates all members, and if you try to moderate a member, it throws an error saying they're already moderated.

I'll try to enlist someone to write unit tests for this, but I personally know very little Python and am just trying to get the system to work. If I'm not able to get unit tests setup for this I guess I'll just run my own version of this module.

@joecot
Copy link
Author

joecot commented Mar 29, 2017

How about this.

  1. I add a new "Moderate all members" setting, which specifically moderates all members.
  2. I add a "restore_broken_moderation" site setting in Zope, which will restore the current functionality.

So there's still an option to moderate all members if someone wants it, and when you make a release with this change you can provide folks a way to keep the current, quite broken, functionality. If I do that, can I get out of adding unit tests here? Because I know how to make all those changes, but I do not know how to add these unit tests. If you can link me some sort of docs on how the unit tests work I can take a gander.

@mpj17
Copy link
Member

mpj17 commented Mar 29, 2017

Works for me :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants