Peer assessor can delete another peers assessment

[Robert Lyon]

When we have the following setup:

1) User A creates Page One and adds
- a peer assessment block
- a sign-off block

2) User A shares Page One to
- User B as 'peer'
- User C as 'peer'
- User D as 'no special role'

3) Login as User B and add an assessment to Page One

4) Login as User C and view Page One
*** problem ***
 - you shouldn't be able to delete the peer assessment of User B

This is also a problem when 'sign-off' block is not present

[Robert Lyon (robertl-9)]

The manual mentions:

"If the peer assessment block is used in conjunction with the sign-off block, the portfolio author must sign off the page before anybody other than themselves and the peer assessor can see published peer assessments."

So that is the way it should be working - but currently isn't

[Robert Lyon (robertl-9)]

This issue was reported in https://mahara.org/interaction/forum/topic.php?id=8566

[Robert Lyon (robertl-9)]

Ok, I think I have worked out what is going on.

When peer assessment / sign-off was designed we had (as per the manual)

"If the peer assessment block is used in conjunction with the sign-off block, the portfolio author must sign off the page before anybody other than themselves and the peer assessor can see published peer assessments."

And the code handled that accordingly.

But then came Bug 1835321 - https://bugs.launchpad.net/mahara/+bug/1835321 - which was based on a request from WR 315075

So now peer assessments, once published, are visible before a page is signed off

This has now created this bug report where a user can delete a fellow peer's assessment

So is the note in the manual the correct way it should work for core versions of Mahara, and the change for WR 315075 just be a customisation for that client?
- therefore we need to revert the fix (bug 1835321) in core and make a custom fix for the client for bug 1859355?

Or is the manual wrong?
- therefore we need to fix up bug 1859355 in core and update the manual

[Kristina Hoeppner (kris-hoeppner)]

The manual would be wrong in this case. :-( Nothing to revert.

[Mahara Bot (dev-mahara)]

Patch for "master" branch: https://reviews.mahara.org/10710

[Rangi Daymond (rangid)]

- Code/Gerrit: https://reviews.mahara.org/#/c/10710/ Patch set 1
- Browser tested: Chrome
- Theme: Raw

Test Scenario 1: Only a peer assessment author (or site administrator) can delete it.

Preconditions:
1. PersonA has created Page1 with a peer assessment block.
2. PersonA shares Page1 as follows:
   - PersonB: role 'Peer and manager'
   - PersonC: role 'Peer'
   - Site admin: 'No special role'
3. PersonB publishes PeerAssessmentB.
4. PersonC publishes PeerAssessmentC1 and PeerAssessmentC2.

Steps:
1. PersonA is logged in and views Page1, 'Add peer assessment' is NOT present and PeerAssessmentB is visible and does NOT have a delete icon, PeerAssessmentC1 and PeerAssessmentC2 are visible and each does NOT have a delete icon. ✔
2. PersonB is logged in and views Page1, 'Add peer assessment' is present and PeerAssessmentB is visible and has a delete icon, PeerAssessmentC1 and PeerAssessmentC2 are visible and neither have a delete icon. ✔
3. PersonC is logged in and views Page1, 'Add peer assessment' is present and PeerAssessmentB is visible and does NOT have a delete icon, PeerAssessmentC1 and PeerAssessmentC2 are visible and both have a delete icon. ✔
4. Site admin is logged in and views Page1, 'Add peer assessment' is NOT present and PeerAssessmentB, C1 & C2 are visible and each have a delete icon. ✔
5. Site admin can select the PeerAssessmentB delete icon and confirm to delete it. ✔
6. PersonB is logged in and views Page1. 'Add peer assessment' is present and PeerAssessmentB and PeerAssessmentC2 are displayed. Select the PeerAssessmentB delete icon and confirm the deletion and removal. ✔
7. PersonB can publish another peer assessment for PersonA. ✔

Test Scenario 2: Only a peer assessment author (or site administrator) can delete their own peer assessment when the sign-off block is present.

Preconditions:
1. PersonA has created Page1 with a peer assessment block and the sign-off block, but the page is not yet signed off
2-4 are as per preconditions in test scenario 1.

Steps:
Follow the steps from test scenario 1. ✔

Test sceanrio 3: Page has peer assessments and is signed off, only site admin can delete those.
1. PersonA has created Page1 with a peer assessment block and a sign-off block, the page is signed off
2-4 are as per preconditions in test scenario 1.

Steps:
Follow the steps from test scenario 1 - however, only the Site admin user will see and be able to use the delete icons ✔

Catalyst QA Approved ✔

[Mahara Bot (dev-mahara)]

Reviewed: https://reviews.mahara.org/10710
Committed: https://git.mahara.org/mahara/mahara/commit/8894df2c77457833ad2d0f5900c6a3c4b91d3e6a
Submitter: Cecilia Vela Gurovic (<email address hidden>)
Branch: master

commit 8894df2c77457833ad2d0f5900c6a3c4b91d3e6a
Author: Robert Lyon <email address hidden>
Date: Mon Jan 13 16:01:01 2020 +1300

Bug 1859355: Peers should not be able to delete other peer's assessment

To test: see bug report for steps

behatnotneeded

Change-Id: I1d345fcf033aca380e3367b130f24b68f737c0aa
Signed-off-by: Robert Lyon <email address hidden>

[Mahara Bot (dev-mahara)]

Patch for "19.10_STABLE" branch: https://reviews.mahara.org/10747

[Mahara Bot (dev-mahara)]

Reviewed: https://reviews.mahara.org/10747
Committed: https://git.mahara.org/mahara/mahara/commit/d7fdaad8cd87bb169e58d1359b74551adefaccea
Submitter: Cecilia Vela Gurovic (<email address hidden>)
Branch: 19.10_STABLE

commit d7fdaad8cd87bb169e58d1359b74551adefaccea
Author: Robert Lyon <email address hidden>
Date: Mon Jan 13 16:01:01 2020 +1300

Bug 1859355: Peers should not be able to delete other peer's assessment

To test: see bug report for steps

behatnotneeded

Change-Id: I1d345fcf033aca380e3367b130f24b68f737c0aa
Signed-off-by: Robert Lyon <email address hidden>
(cherry picked from commit 8894df2c77457833ad2d0f5900c6a3c4b91d3e6a)

[Mahara Bot (dev-mahara)]

Patch for "19.04_STABLE" branch: https://reviews.mahara.org/10748

[Mahara Bot (dev-mahara)]

Reviewed: https://reviews.mahara.org/10748
Committed: https://git.mahara.org/mahara/mahara/commit/141f3856bebd2bb0dc169981e31709ff986a7fd6
Submitter: Cecilia Vela Gurovic (<email address hidden>)
Branch: 19.04_STABLE

commit 141f3856bebd2bb0dc169981e31709ff986a7fd6
Author: Robert Lyon <email address hidden>
Date: Mon Jan 13 16:01:01 2020 +1300

Bug 1859355: Peers should not be able to delete other peer's assessment

To test: see bug report for steps

behatnotneeded

Change-Id: I1d345fcf033aca380e3367b130f24b68f737c0aa
Signed-off-by: Robert Lyon <email address hidden>
(cherry picked from commit 8894df2c77457833ad2d0f5900c6a3c4b91d3e6a)
(cherry picked from commit d7fdaad8cd87bb169e58d1359b74551adefaccea)

[Mahara Bot (dev-mahara)]

Patch for "18.10_STABLE" branch: https://reviews.mahara.org/10749

[Mahara Bot (dev-mahara)]

Reviewed: https://reviews.mahara.org/10749
Committed: https://git.mahara.org/mahara/mahara/commit/438f2043a5915606df241a4f2831ef567cf57bc7
Submitter: Cecilia Vela Gurovic (<email address hidden>)
Branch: 18.10_STABLE

commit 438f2043a5915606df241a4f2831ef567cf57bc7
Author: Robert Lyon <email address hidden>
Date: Mon Jan 13 16:01:01 2020 +1300

Bug 1859355: Peers should not be able to delete other peer's assessment

To test: see bug report for steps

behatnotneeded

Change-Id: I1d345fcf033aca380e3367b130f24b68f737c0aa
Signed-off-by: Robert Lyon <email address hidden>
(cherry picked from commit 8894df2c77457833ad2d0f5900c6a3c4b91d3e6a)
(cherry picked from commit d7fdaad8cd87bb169e58d1359b74551adefaccea)
(cherry picked from commit 141f3856bebd2bb0dc169981e31709ff986a7fd6)