Multi-user read only drafts

[ricroberts]

In muttnik and drafter...

We want the ability for multiple users to be able to review a draft at once. Currently you need to claim a draft to be able to view it, but this stops others looking at it.

I suggest we add a new role e.g. drafter:reviewer (name up for grabs. Alternative?: drafter:draft-viewer). Assuming we keep cumulative roles for now, editors and publishers would additionally get this role. Users with this new role would be able to view (but not claim) drafts in the available state.

This new role would be available to choose when someone wants to 'submit' a draft for review.

Only editors and publishers should be able to claim drafts. If someone else claims a draft it would immediately become unavailable to viewers.

We would need a new button in the muttnik admin section for users to be able to 'view' but not claim the draft. i.e. the owner of the draft wouldn't change and it would stay available to claim. We also want Editors and publishers to also be able to view (but not claim) drafts. Am happy to debate what these actions are called.

Related: #592

[callum-oakley]

@RicSwirrl (@RickMoynihan as well because I seem to remember you having some thoughts about how this stuff would work) I've been looking at the code and trying to get a handle on what would need to change to do this. Some questions I have at this point:

This description implies "available for review" is a state that a draftset owner explicitly puts a draft into.

1. Is this intended to be the same state as the already existing "available to be claimed"? (i.e. just a draft without an owner)
2. Do we need this state at all? Or can we allow a drafter:reviewer to view owned (all) drafts?
3. If 2 is not restrictive enough, could we instead have a flag on a draft (off by default) to set it as available for review, which would make it orthogonal to ownership?

I'm imagining a scenario where E is editing a draft and R is reviewing:

• E has finished editing and wants a review, so relinquishes ownership
• R views draft with the reviewer role, suggests some changes
• E has to claim draft again, at which point R can no longer see it
• E makes changes
• E relinquishes ownership again
• R has to select the draft to view again
• R reviews changes
• etc

whereas if you could mark a draft as ready for review without relinquishing ownership the workflow could instead be

• E has finished editing and wants a review, so marks as ready for review (or not even that, in scenario 2)
• R views draft with the reviewer role, suggests some changes
• E makes changes
• R reviews changes
• etc

[RickMoynihan]

Yes I agree the flow there is very much sub-optimal.

First to answer your question:

I'm imagining that it would be slightly different to claiming, which is now claiming exclusive edit rights to the draftset, though it would I think parallel it.

i.e.

On the Transfer tab (which might need renamed) you could either transfer to a single editor (for them to claim), or you could add any number of viewers to the draftset, e.g. via checkboxes.

Underneath where we currently have drafter:claimUser and drafter:claimRole for submitting to editors, we'd have drafter:viewerUser and drafter:viewerRole. The main difference being that drafts with you as a viewerUser or matching your role would be visible to you, without a claim and transfer of owner. Also the cardinality of those predicates should I think be many not one. Therefore we'll also need drafter API routes to add/remove people from the set.

Secondly and more broadly my main thoughts around this area are that:

1. It feels inevitable that we’re heading into full on IAM/RBAC territory, and that things would be miles better if we did.
2. We don’t currently attach permissions onto endpoints/draftsets; but instead they’re global across all endpoints/draftsets. So far editors, publishers and now viewers have global permissions, across all endpoints.
3. Our terminology is a little mixed up, for example roles are complected with permissions; but also groups as you can think of users being in our "viewer role" as being a group of global-viewers, etc.

Relatedly it would be very nice if we could add a super user admin role for us to use; so we can view draftsets, and edit/obtain them from our customers if we need to help intervene or debug.

and therefore I think it's inevitable that we will eventually need to live in a world where:

1. Endpoints and draftsets are resources which can enforce their own access for read/write/management/publishing etc.
2. We will need to manage groups of users and their more fine grained permissions and roles via auth0. We should also be allowing our customers to essentially specify their own roles etc...
3. We're going to be adding new resources soon e.g. files, it would be nice to have this tidied up before we make retrofitting this properly an even bigger job.

So this is just a reminder; that perhaps we should get our house in order and refactor to do this stuff properly, before adding more stuff we need to undo. Particularly as files in drafts etc are around the corner too.

I don't think this particular addition will be the straw that breaks the camels back; but how many more straws are we going to pile on before we address these issues?

[ricroberts]

To respond to some of @callum-oakley's points:

This description implies "available for review" is a state that a draftset owner explicitly puts a draft into.

Yeah - it is.

Is this intended to be the same state as the already existing "available to be claimed"? (i.e. just a draft without an owner)

Yes.

Do we need this state at all? Or can we allow a drafter:reviewer to view owned (all) drafts?

My intention was that you'd only make it available for review when you were ready.

If 2 is not restrictive enough, could we instead have a flag on a draft (off by default) to set it as available for review, which would make it orthogonal to ownership?

Possibly, though part of the reason for my idea was that you don't want to be editing it while someone is reviewing it. Not least because edits in a draft aren't transactional - you don't want reviewers to see things part way through a batched insert.

[ricroberts]

Addressing some points from @RickMoynihan:

It feels inevitable that we’re heading into full on IAM/RBAC territory, and that things would be miles better if we did.

At some point drafter might become very different (in some future pmd-cloudey world) so wary to do too much unnecessary work that would get thrown away. But agree this would make life easier while drafter is still kinda like it is now.

Relatedly it would be very nice if we could add a super user admin role for us to use; so we can view draftsets, and edit/obtain them from our customers if we need to help intervene or debug.

Yes! I've often wanted this.

[callum-oakley]

you don't want to be editing it while someone is reviewing it

Good point, and definitely something we need to be careful of, but I was imagining a reviewer would hit the locked draft screen if they refresh while a job is in progress (in fact I think this would already happen, since the job checking is independent of who started the job).

[ricroberts]

i think this might be a bit weird for the reviewer! I was thinking it would be better if drafts were stable while being reviewed.

[callum-oakley]

I'm not sure it's any less stable as far as the reviewer is concerned. From the reviewer's perspective the difference is:

• looking at a draft
• suddenly lose access to it (error page?)
• find it again and re-claim (with no visibility on when it's ready? just have to keep trying until it appears I guess)
• sees changes

vs

• looking at a draft
• there's a change happening, dropped in the lock screen (with visibility on what's happening and auto refresh until the job is done)
• refresh (possibly automatic)
• sees changes

[callum-oakley]

of course it would be even better if the reviewer could be given access to an immutable snapshot of the draft (a commit if you will), and the editor could continue making changes in parallel, but perhaps I'm getting ahead of myself... 🤔

[RickMoynihan]

@RicSwirrl:

At some point drafter might become very different (in some future pmd-cloudey world) so wary to do too much unnecessary work that would get thrown away. But agree this would make life easier while drafter is still kinda like it is now.

I'm currently assuming drafter will incrementally change and be part of the future solution regardless. Whether it's one system or many; I think the drafter state graph and draftset model is what we need to evolve and extend, it is the source of truth that will tie things together, now and in the future, and consequently I think any IAM/RBAC stuff we do will likely remain. Anything else will I think be too big a change for customers to adopt.

[ricroberts]

After some discussion we decided that the behaviour that we want is actually:

• For drafts owned by someone, additionally allow other users to view it. e.g. These users /roles can be added removed via some mechanism on the (poss renamed to 'share'?) transfer tab.
• The owner can still edit it. And if they do, the viewers will just see the 'busy' screen while it's happening.