peer_reviewed_article: Show review checklist #352

Merged

matthijskooijman merged 11 commits from article-checklist into master

2020-11-17 14:30:57 +00:00

matthijskooijman commented

2020-10-20 13:58:54 +00:00

(Migrated from github.com)

In the DRAFT, REVIEW and APPROVED stages, this shows a checklist of things to do before progressing to the next stage, which helps to quickly see if anything is forgotten.

Some of these things are marked as required, meaning they prevent progress (i.e. starting review or publishing). Others are marked as recommended, meaning they are shown but not enforced. Whether something is required or recommended is hardcoded for now, but the code is writting to allow making this configurable later.

This additionally does some more or less related factorings and small cleanups.

The new functions have very verbose type definitions (intended to be consumed by Psalm), we will have to see if this is workable or simpler types should be used (which allow less checking).

Originally, I had intended for updateToNewStatus to also check the checklist and prevent state changes when the checklist was not completed, which would allow removing canBeSetAsApproved entirely (just try to set the status to APPROVED and if it fails, e.g. because the checklist says more approvals are needed, just ignore that failure). However, it turns out this could create a corner case where removing e.g. all tags after review start would prevent the status from progressing to APPROVED, so I ended up only checking the checklist in statusChangeAction. This results in code that is a bit more messy, but should be correct. @laurensmartina, any suggestions on this?

This fixes #304.

In the DRAFT, REVIEW and APPROVED stages, this shows a checklist of things to do before progressing to the next stage, which helps to quickly see if anything is forgotten. Some of these things are marked as required, meaning they prevent progress (i.e. starting review or publishing). Others are marked as recommended, meaning they are shown but not enforced. Whether something is required or recommended is hardcoded for now, but the code is writting to allow making this configurable later. This additionally does some more or less related factorings and small cleanups. The new functions have very verbose type definitions (intended to be consumed by Psalm), we will have to see if this is workable or simpler types should be used (which allow less checking). Originally, I had intended for `updateToNewStatus` to also check the checklist and prevent state changes when the checklist was not completed, which would allow removing `canBeSetAsApproved` entirely (just try to set the status to `APPROVED` and if it fails, e.g. because the checklist says more approvals are needed, just ignore that failure). However, it turns out this could create a corner case where removing e.g. all tags after review start would prevent the status from progressing to `APPROVED`, so I ended up only checking the checklist in `statusChangeAction`. This results in code that is a bit more messy, but should be correct. @laurensmartina, any suggestions on this? This fixes #304.

matthijskooijman commented

2020-10-20 14:37:03 +00:00

(Migrated from github.com)

Here's how an empty draft article would look in practice:

Here's how a complete draft article would look

And here's the various ways an article under review can look

Here's the various ways a draft article could look with different settings

Here's an overview of all the possible text that can be shown. Note that this includes situations that cannot occur with the current code or even do not make real sense (i.e. article content will probably always be required), but I've added them all to allow future expansion. # Here's how an empty draft article would look in practice: ![image](https://user-images.githubusercontent.com/194491/96601328-2fe73800-12f2-11eb-8eb8-f4475b482398.png) ![image](https://user-images.githubusercontent.com/194491/96601201-11813c80-12f2-11eb-94ab-283b648cbf87.png) # Here's how a complete draft article would look ![image](https://user-images.githubusercontent.com/194491/96597000-9d449a00-12ed-11eb-9607-1ec4d43d778a.png) ![image](https://user-images.githubusercontent.com/194491/96599449-3d032780-12f0-11eb-9b03-bdf99f9acc75.png) # And here's the various ways an article under review can look ![image](https://user-images.githubusercontent.com/194491/96600538-535db300-12f1-11eb-9abb-fe3efc0ced69.png) ![image](https://user-images.githubusercontent.com/194491/96600434-37f2a800-12f1-11eb-9335-45be239cfc73.png) ![image](https://user-images.githubusercontent.com/194491/96600647-70928180-12f1-11eb-8a57-a8a1b896fb43.png) ![image](https://user-images.githubusercontent.com/194491/96600342-23161480-12f1-11eb-8640-3c5f229fc947.png) # Here's the various ways a draft article could look with different settings ![image](https://user-images.githubusercontent.com/194491/96599207-f9102280-12ef-11eb-8d0a-b42e2f0cabca.png) ![image](https://user-images.githubusercontent.com/194491/96599544-560bd880-12f0-11eb-9368-87e23bbaa987.png) ![image](https://user-images.githubusercontent.com/194491/96599134-e564bc00-12ef-11eb-9d03-cc6423806c53.png) ![image](https://user-images.githubusercontent.com/194491/96599390-29f05780-12f0-11eb-8edb-8b8d900ca16c.png)

laurensmartina (Migrated from github.com) reviewed

2020-10-28 08:20:38 +00:00

system/datatypes/peer_reviewed_article.php

Perfect! Next step: make it configurable. :-)

laurensmartina (Migrated from github.com) requested changes

2020-10-28 08:46:35 +00:00

system/core/language.php

The comment block is not correctly formatted

/**
 * <--
 */

The comment block is not correctly formatted ``` /** * <-- */ ```

system/core/tags.php

Here there should be an empty check.

system/datatypes/peer_reviewed_article.php

The status gotten from getStatus() does not change while being in this method, so it would be better to assign in to a variable outside the foreach loop.

The status gotten from `getStatus()` does not change while being in this method, so it would be better to assign in to a variable outside the foreach loop.

Typo at returne

Typo at `returne`

system/datatypes/peer_reviewed_article.php

Strict comparison is recommended.

system/datatypes/peer_reviewed_article.php

didn't know a better place to put this comment.. There is a typo in the commit message