Other URLs

https://github.com/xwiki-contrib/application-changerequest
https://jira.xwiki.org/projects/CRAPP/summary

Description

The idea is to allow contributions to XWiki by requesting for changes, instead of doing them directly. This workflow changes the paradigm of "anyone can edit the wiki" to "anyone can edit the wiki but someone has to validate the changes". We consider a change request as very close to the concept of Github "Pull Request" and we might reuse some Git related vocabulary (like commit, merge, branch, etc).

Note that we will use CR to talk about change request in this document.

Usecases

Actors

For the usecases we consider different actors:

• the editors are the people who cannot perform direct editions on a page, but propose changes through change requests
• the reviewers are able to view and comment the proposed changes, but they cannot merge them or mark them as approved or rejected
• the approvers can also accept or reject a change request even if they cannot necessarily merge it by themselves
• the mergers are approvers who can actually close the PR by merging the changes to the actual pages
• the wiki administrator can decide who will be an approver, a reviewer or an editor and in which location.

Scenario

We propose the following usecases:

• UC1 Create a CR: an editor starts editing a page and opens a CR when saving the changes. People watching that page will receive a notification.
• UC2 Edit page in an existing CR: an editor goes to an existing CR, gets the current status of a page in that CR, performs other changes and save them in the same CR. People watching the page, and people who interacted on the CR will receive a notification.
• UC3 Add another page changes in an existing CR: an editor starts editing a page, and contribute to an existing CR when saving the changes. People watching that page will receive a notification.
• UC4 Comments a CR: a reviewer goes to a CR, check the changes and makes a comment about them. People who interacted in the CR will receive a notification.
• UC5 Approve a CR: an approver goes to a CR, check the changes and comments, and mark the changes as approved for merging. People who interacted in the CR will receive a notification.
• UC6 Request changes on a CR: an approver goes to a CR, check the changes and comments, and mark the changes as needed to be improved with a comment. People who interacted in the CR will receive a notification.
• UC7 Merge a CR without conflict: a merger goes to a CR, check that the approval strategy and the CR status allows to merge the CR and click on merge: the CR is closed and the pages the CR refers to receive the changes. The original page history also reflects what happened in the CR, in particular if various authors edited it in the CR. People who interacted in the CR will receive a notification.
• UC8 Merge a CR with conflicts: a merger goes to a CR, check that the approval strategy and the CR status allows to merge the CR and click on merge: the merge operations lead to a conflict, the merger can either handle them by himself, or put the merge on hold and ask the editor to fix them in which case the CR is not closed.
• UC9 Rebase a CR: an editor goes to a CR, sees that the original page the CR refers to has been edited since latest change of the CR, so they calls for a rebase in order to get latest change in the CR:  the rebase might lead to conflict that they solves to complete the rebase
• UC10 Close a CR: a merger goes to a CR that is not maintained anymore and close it, preventing other editors to perform changes on it. People who interacted in the CR will receive a notification.
• UC11 Change status of a CR: an editor goes to a CR and mark the CR as ready to be reviewed, they suggest some approvers to approve it. People who interacted in the CR will receive a notification as well as the suggested approvers.
• UC12 Manage approvers globally: an administrator decides which users will be approvers and for which location (wiki, space or page)
• UC13 Chose approval strategy: an administrator decides which approval strategy will be used for merging the CRs of a wiki
• UC14 Display all existing CRs for a page with the status of each PR and with the capability to sort and filter them
• UC15 Display all CRs an approver is assigned to with the capability to sort and filter them
• UC16 Manage the locations where PR workflow is enabled: an administrator should be able to decide which locations are subject to use a CR, and which ones can only be edited with standard mechanism.
• UC17 Display all CRs in which an editor is involved in
• UC18 Switch off notifications for a specific CR: any user should be able to switch off the notifications of a CR.
• UC19 CR edition lock: two people should not be able to edit the same page of the same CR at the same moment, the page in the CR should be locked for edition to avoid any issue.
• UC20 CR created by guest user: as a guest user, if CR creation is allowed for them, I can create a CR or contribute to an existing one, assuming that I solve a captcha to submit my changes.
• UC21 Activity stream of the CR: when accessing a CR, I want to be able to see the different activities of this CR since its creation.

Technical details

CR Storage

We consider that each CR is made of a set of commits, a commit containing a saved change on a specific page: a commit cannot contain changes for several pages.

For each commit we need:

•  Metadata of the change: reference of the page, of the author, date of the change, link to previous "commit" and version of the page from which the change has been made.
•  Data of the changes: new content, be it wiki content, attachment, objects and classes changes
•  Comments related to part of the changes: every part of the change can be subject to comment

For each CR we need:

•  Data of the changes: linked-list of the commits
•  Metadata of the CR: creator of the CR, dates of creation / update, status (draft/ready to review/change requested/merged/rejected/), list of pages included in the CR, global comments, approval / rejection marks,

Regarding how we store the data, two big options:

1.  we store all the new content. Pros: probably easier to store and less fragile since even if you lose the version of the page where you started the change, you still have the whole content. Cons: it's taking more disk space since you duplicate the page at each commit and you might have conflicts more often.
2.  we store only the content diff (e.g. what has been added/removed). Pros: less duplicated content in DB, easier to display the diff since we don't need to recompute it each time. Cons: we necessarily need to keep track of the attached version, since we can mess up quickly everything by not being attached to the right version so it's more fragile.

We decided to go with storing the whole content since it's less fragile and in theory the duplicated content should not be that much an issue, compared to the various problem we could have by only keeping the diff.

We considered 4 options about where we store the data:

1. Storing everything as XObjects
2. Storing commit changes as specific documents (e.g. in a dedicated space) and PR info as xobjects
3. Storing everything in another DB (e.g. Solr)
4. Storing commit changes as serialized pages on the filesystem and PR info as xobjects

We decided to start with this 4th solution since it avoids the main drawback of the other solution, which is to pollute the wiki by storing "not published data content" in published content which is a bit contradictory.

Rights

The different roles of the CR workflow might be represented with rights.

In particular we consider that we need a specific right to decide who can be an approver of the PRs for that page. We could decide in the future that specific rights are needed to allow specific people to review CR of specific spaces, but for now we consider that anyone with view right on the original page and comment rights on the CR can review it. We also need a CR creation specific right to define which spaces can be subject to CR and which cannot be, and to define who are the editors, in particular we can use that to allow guest to create CR or not.

Notifications

We can imagine different kinds of events related to the CR workflow:

• a new CR is created
• the status of a CR changed: it's been rejected / merged / marked as draft / marked ready for review
• the content of a CR changed: a new commit has been added to it
• a new comment has been made on a CR
• a new approval / request for changes has been made on a CR

Each of those events can be switched on/off by the users. We probably should allow to filter them by locations too (e.g. I want to stop receive notifications but only for a specific CR where people are too noisy). The default strategy for sending the notification should be by using the location watches: people watching a location should receive CR notifications about it, unless they disable their notification for those. Once a CR is created, any people who interacted in the CR should also receive the notifications. So we might need to use two locations for the event: the original page location, and the CR location.

Merge Approval Strategy

The idea of the Merge approval strategy is to allow to define different kind of strategies to unlock the capability to merge a CR. So we can imagine for example a strategy in which a single approval on a CR is enough to enable the merge, vs a strategy where we'd want all approvers to approve the CR to enable the merge, or again another strategy where we could select a % of approval among the approvers who voted to enable the merge.

Limitations and constraints

• Refactoring operations such as page deletions or page move cannot be part of a CR change for now.
• A CR can only integrate changes related to pages of the same wiki.
• Merger should have edit rights on all the pages of the CR to be able to merge
• In case of view right change in the original page of a CR, the CR should always be hidden for the people who don't have rights anymore
• As a first implementation we won't consider the capability to search inside the modified content of the CRs, this can be part of a future work
• By storing commit changes on the filesystem we create a new limitation on using CR workflow on clustering if the filesystem is not shared, which is an already existing limitation for attachment storage.

UI

This part will need to be improved with proper mock-ups, in the meantime here some first thoughts about it:

• the CR workflow should be seamless for users, so they should be able still see the big Edit button even when they cannot edit a page, and to then click on "Request changes" to create the CR: a modal should be opened to ask users if they want to add their changes to an existing CR (in which case we might also have to deal with merging the changes if the CR already contained changes for that page) and to ask other info, such as a CAPTCHA for guest, or the title of the new CR if it's a new one.
• Displaying a CR should display its title / description, the global CR status, the people involved in it (editors, approvers), the approval / rejection marks, the list of pages changed in that CR with the number of comments next to them, and the activity stream of the CR. Global comments of the CR should also be visible.
• Accessing a page changes inside a CR should display first the diff of latest changes (compared with the original page) with the comments put on those changes. Another tab should allow to display the securely rendered content (i.e. with using last editor for the execution, and without executing any script as we do in WYSIWYG). The history doctab should allow to navigate between the modifications of the page made in that CR.
• Approving a CR or asking for changes should be a button on the CR viewable anywhere in it and the status of the approval / request changes should also be immediately visible on the CR. Note that in case of change request, it should link to the actual related comment. Also it should display which people approved or rejected.
• If the approval strategy goal is reached, an automatic check should be performed to check if the merge can be done without conflict, and the merge button should be made available. If the approval strategy goal is not reached, the merge button should be usable to rebase and fix conflicts.
• When merging a CR, the comment of the save should contain a link to the actual CR
• On the bottom of any page of the wiki, a specific doctab should allow to view the existing CRs for that page.
• A user profile tab should allow to access the CR created by that user. If the user can approve CR we should also list the CR they approved / rejected.