Change publisher groups from open to restricted (and vice versa)

[klemay]

Steps to reproduce

Attempt to change a publisher group from open to restricted, or vice versa.

1. While logged in as a Hypothesis admin, go to https://hypothes.is/admin/groups
2. Find a publisher group and go to the admin page, where you would add/remove members (NOT the group activity page)(Hypothesis staff can use our BioPub Editors Restricted group to test with).
3. Attempt to change the "Group Type" value from Open to Restricted, or vice versa, by clicking on the dropdown:
4. Scroll down and click Save.

Expected behaviour

You should see a flash message across the top of the group admin page saying your changes were successfully saved.

Actual behaviour

You'll get an error message:

Browser/system information

I'm using the latest version of Chrome for Mac; Hypothesis team members have reproduced on others systems/browsers

Additional details

This is a regression from former functionality. We do not currently have any groups that need to have their types changed, but this is something we frequently do for partners as they get to know our product and rethink the way they want to use it.

[jon-betts]

Just had a quick chat with @lyzadanger about her ideas on the ticket. A quick summary:

• The error looked like it might be a validation layer problem (webargs, marshmallow and friends)
• The "front-end" here is all really back-end being served as Jinja 2 templates
• The code hasn't been changed in a while, therefore...
• A likely culprit is a library update causing backward incompatibility (we've seen this from webargs with other stuff)

[seanh]

Monthly reminder that we don't use webargs or marshmallow in h! We use jsonschema and colander, maybe one of those got upgraded?

[jon-betts]

Yeah, no such luck. The error "Changing group type is currently not supported" is raised directly by "deform" based on a Colander schema. So I'm none the wiser.

[jon-betts]

As far as I can tell this has always been this way. This appears to have been added by us on purpose in: https://github.com/hypothesis/h/commit/cad0f8e3e91bf072143ff11c2f519b6b5b5d3dbc

Discussions relating to why are here:

• https://github.com/hypothesis/h/pull/4887#pullrequestreview-104543726

@klemay - Is this actually a bug, or expected behavior? It looks like it's been this way since 2018 when it was released?

[klemay]

@jon-betts I've definitely been able to to change open & restricted groups as late as November 2019, according to an email exchange I had with a partner around then!

I am now wondering whether we did not design group switching to work, but it did "just work" for a while. The "regression" may have been the fact that this worked at all.

In any case, I'd argue strongly for returning this function. In an ideal world, we'd see:

• Changing from Restricted to Open = delete all members except the creator
• Changing from Open to Restricted = no functional membership changes

...but a world in which changing from Restricted to Open leaves all memberships in place and the Hypothesis team member has to manually delete members would also be fine for my purposes!

[jon-betts]

There's nothing I can spot in the code to support that, so maybe there was some friendly DB admin style help?

In any case, it doesn't change the fact it needs done if it does.

[seanh]

There's code in h that's written specifically to disallow changing the group type:

https://github.com/hypothesis/h/blob/10fabe1c44ff614f9c75df3a283a00bcf58b7f5d/h/schemas/forms/admin/group.py#L96-L100

That code has been in place since March 2018 (https://github.com/hypothesis/h/commit/cad0f8e3e91bf072143ff11c2f519b6b5b5d3dbc) which was when the admin form was first added (https://github.com/hypothesis/h/pull/4887).

So I don't think you've ever been able to change a group's type.

Unfortunately I think this is a feature request not a bug, and so is a slightly larger thing. Would need to determine the consequences of changing group types and how we'd implement it. It's likely not a simple case of flipping the type flags, think that could lead us into broken situations

[klemay]

@seanh and @jon-betts as I mentioned this morning, given this is a feature request and not a simple "switch-flipping" situation, I've changed the name of the issue, removed from bug backlog, and added to product backlog for prioritization.

I'll make sure it's removed from the current sprint board as well.

Thank you for looking into this & for the helpful explanation re: why this isn't a quick/simple fix!