From cae8271ecbb1bb2e6647a77b3ada26bcb59cbbec Mon Sep 17 00:00:00 2001
From: Mike Colagrosso <colagrosso@users.noreply.github.com>
Date: Thu, 12 Dec 2024 17:15:30 -0700
Subject: [PATCH] Set ReviewerUserId only if Status changed

For non-admin reviewers (i.e., without `CanReviewOwnArtwork`), the `PATCH` form
has a hidden element with the artwork's current `Status`. If the reviewer
updates the `EbookUrl` and not `Status`, then don't record the reviewer's
`ReviewerUserId` because they didn't review or change the `Status`.

Side note: Sending the `PATCH` form an invalid `Status` will result in a
validation error, but that validation error was hard to read because it was the
wrong exception type. This commit adds a new `InvalidArtworkStatusException` to
fix that.

Fixes #433
---
 lib/Artwork.php                                  |  2 +-
 lib/Exceptions/InvalidArtworkStatusException.php |  7 +++++++
 www/artworks/post.php                            | 16 ++++++++--------
 3 files changed, 16 insertions(+), 9 deletions(-)
 create mode 100644 lib/Exceptions/InvalidArtworkStatusException.php

diff --git a/lib/Artwork.php b/lib/Artwork.php
index 6033cd8f..694f2f0a 100644
--- a/lib/Artwork.php
+++ b/lib/Artwork.php
@@ -393,7 +393,7 @@ class Artwork{
 		}
 
 		if(!isset($this->Status)){
-			$error->Add(new Exceptions\InvalidArtworkException('Invalid status.'));
+			$error->Add(new Exceptions\InvalidArtworkStatusException());
 		}
 
 		if(isset($this->Tags)){
diff --git a/lib/Exceptions/InvalidArtworkStatusException.php b/lib/Exceptions/InvalidArtworkStatusException.php
new file mode 100644
index 00000000..477fb9d5
--- /dev/null
+++ b/lib/Exceptions/InvalidArtworkStatusException.php
@@ -0,0 +1,7 @@
+<?
+namespace Exceptions;
+
+class InvalidArtworkStatusException extends AppException{
+	/** @var string $message */
+	protected $message = 'Invalid artwork status.';
+}
diff --git a/www/artworks/post.php b/www/artworks/post.php
index b639ff34..91bccf4b 100644
--- a/www/artworks/post.php
+++ b/www/artworks/post.php
@@ -93,17 +93,17 @@ try{
 		// We can PATCH the status, the ebook www filesystem path, or both.
 		if(isset($_POST['artwork-status'])){
 			$newStatus = Enums\ArtworkStatusType::tryFrom(HttpInput::Str(POST, 'artwork-status') ?? '');
-			if($newStatus !== null){
-				if($artwork->Status != $newStatus && !$artwork->CanStatusBeChangedBy(Session::$User)){
+			if($artwork->Status != $newStatus){
+				if(!$artwork->CanStatusBeChangedBy(Session::$User)){
 					throw new Exceptions\InvalidPermissionsException();
 				}
 
-				$artwork->ReviewerUserId = Session::$User->UserId;
-
-				$artwork->Status = $newStatus;
-			}
-			else{
-				unset($artwork->Status);
+				if($newStatus !== null){
+					$artwork->ReviewerUserId = Session::$User->UserId;
+					$artwork->Status = $newStatus;
+				}else{
+					unset($artwork->Status);
+				}
 			}
 		}