Fix raw language keys and validation findings

claims.php

@@ -27,6 +27,9 @@ class claims
 	/** Longest note, in characters; the column holds 255 */
 	const NOTE_MAX_LENGTH = 255;
 
+	/** Seconds in a day, for the release-after-days setting */
+	const SECONDS_PER_DAY = 86400;
+
 	/** Notification reasons */
 	const REASON_TRANSFER = 'transfer';
 	const REASON_TAKEOVER = 'takeover';
@@ -497,7 +500,7 @@ public function find_expired($days)
 	{
 		$sql = 'SELECT topic_id
 			FROM ' . $this->claims_table . '
-			WHERE claim_time < ' . (time() - (int) $days * 86400);
+			WHERE claim_time < ' . (time() - (int) $days * self::SECONDS_PER_DAY);
 
 		return $this->fetch_topic_ids($sql);
 	}

controller/manage.php

@@ -32,6 +32,18 @@ class manage
 	/** Form key name */
 	const FORM_KEY = 'phpbbmodders_topicclaim';
 
+	/** Seconds the result message shows before going back */
+	const REDIRECT_DELAY = 3;
+
+	/**
+	 * Buttons that ask for confirmation first, with the title key confirm_box() uses.
+	 * It adds _CONFIRM for the question: TOPICCLAIM_RELEASE_CONFIRM, TOPICCLAIM_TAKEOVER_CONFIRM.
+	 */
+	const CONFIRM_ACTIONS = [
+		'release'	=> 'TOPICCLAIM_RELEASE',
+		'takeover'	=> 'TOPICCLAIM_TAKEOVER',
+	];
+
 	/** @var auth */
 	protected $auth;
 
@@ -118,22 +130,42 @@ public function handle($topic_id)
 		$u_topic = append_sid("{$this->root_path}viewtopic.{$this->php_ext}", 't=' . $topic['topic_id']);
 		$u_page = $this->helper->route('phpbbmodders_topicclaim_manage', ['topic_id' => $topic['topic_id']]);
 		$u_return = $this->get_return_url($u_topic);
+		$u_action = $u_page . ($u_return !== $u_topic ? (strpos($u_page, '?') === false ? '?' : '&') . 'return=mcp' : '');
 
 		add_form_key(self::FORM_KEY);
 
 		$action = $this->get_action();
 		if ($action !== '')
 		{
-			if (!check_form_key(self::FORM_KEY))
+			// "No" on the confirmation page: back to the claim page
+			if ($this->request->is_set_post('cancel'))
+			{
+				redirect(str_replace('&', '&', $u_action));
+			}
+
+			$needs_confirm = isset(self::CONFIRM_ACTIONS[$action]);
+			$confirmed = $needs_confirm && confirm_box(true);
+
+			// confirm_box() checks its own key; every other submit has the form token
+			if (!$confirmed && !check_form_key(self::FORM_KEY))
 			{
-				trigger_error($this->language->lang('FORM_INVALID') . $this->return_links($u_page, $u_topic, $u_return), E_USER_WARNING);
+				trigger_error($this->language->lang('FORM_INVALID') . $this->return_links($u_topic, $u_return), E_USER_WARNING);
+			}
+
+			if ($needs_confirm && !$confirmed)
+			{
+				confirm_box(false, self::CONFIRM_ACTIONS[$action], build_hidden_fields([
+					'topicclaim_action'	=> $action,
+					// Already HTML-escaped by request_interface
+					'topicclaim_note'	=> $this->request->variable('topicclaim_note', '', true),
+				]), 'confirm_body.html', $u_action);
 			}
 
 			$message = $this->run_action($action, $topic, $claim);
-			$this->finish($message, $u_page, $u_topic, $u_return);
+			$this->finish($message, $u_topic, $u_return);
 		}
 
-		$this->assign_page($topic, $claim, $u_page, $u_topic, $u_return);
+		$this->assign_page($topic, $claim, $u_action, $u_topic, $u_return);
 
 		return $this->helper->render('@phpbbmodders_topicclaim/topicclaim_manage.html', $this->language->lang('TOPICCLAIM_PAGE_TITLE'));
 	}
@@ -189,84 +221,83 @@ protected function run_action($action, array $topic, $claim)
 			return 'TOPICCLAIM_NOT_CLAIMED';
 		}
 
+		// Take over is for another moderator's claim; the rest for the holder or an administrator
+		$allowed = ($action === 'takeover') ? (!$this->claims->is_own($claim) && $this->claims->can_override()) : $this->claims->can_manage($claim, $topic['forum_id']);
+		if (!$allowed)
+		{
+			return 'TOPICCLAIM_NOT_ALLOWED';
+		}
+
 		switch ($action)
 		{
 			case 'note':
-				if (!$this->claims->can_manage($claim, $topic['forum_id']))
-				{
-					break;
-				}
 				$this->claims->set_note($topic, $note);
-				return 'TOPICCLAIM_NOTE_SAVED';
+				$message = 'TOPICCLAIM_NOTE_SAVED';
+			break;
 
 			case 'takeover':
-				if ($this->claims->is_own($claim) || !$this->claims->can_override())
-				{
-					break;
-				}
 				$this->claims->take_over($topic, $claim, $note);
-				return 'TOPICCLAIM_TAKEN_OVER';
+				$message = 'TOPICCLAIM_TAKEN_OVER';
+			break;
 
 			case 'transfer':
-				if (!$this->claims->can_manage($claim, $topic['forum_id']))
-				{
-					break;
-				}
-
 				$user_id = $this->request->variable('topicclaim_user', 0);
 				$moderators = $this->claims->get_moderators($topic['forum_id']);
 				if (!isset($moderators[$user_id]) || $user_id === $claim['user_id'])
 				{
-					return 'TOPICCLAIM_TRANSFER_INVALID';
+					$message = 'TOPICCLAIM_TRANSFER_INVALID';
+					break;
 				}
 
 				$this->claims->transfer($topic, $claim, $user_id, $moderators[$user_id]);
-				return 'TOPICCLAIM_TRANSFERRED';
+				$message = 'TOPICCLAIM_TRANSFERRED';
+			break;
 
 			case 'release':
-				if (!$this->claims->can_manage($claim, $topic['forum_id']))
-				{
-					break;
-				}
 				$this->claims->release($topic, $claim);
-				return 'TOPICCLAIM_RELEASED';
+				$message = 'TOPICCLAIM_RELEASED';
+			break;
+
+			default:
+				// get_action() only returns the buttons above
+				$message = 'TOPICCLAIM_NOT_ALLOWED';
+			break;
 		}
 
-		return 'TOPICCLAIM_NOT_ALLOWED';
+		return $message;
 	}
 
 	/**
 	 * Show the result of a button and go back to where the user came from
 	 *
 	 * @param string $message Language key
-	 * @param string $u_page
 	 * @param string $u_topic
 	 * @param string $u_return
 	 * @return void
 	 */
-	protected function finish($message, $u_page, $u_topic, $u_return)
+	protected function finish($message, $u_topic, $u_return)
 	{
 		$success = in_array($message, ['TOPICCLAIM_CLAIMED', 'TOPICCLAIM_NOTE_SAVED', 'TOPICCLAIM_TAKEN_OVER', 'TOPICCLAIM_TRANSFERRED', 'TOPICCLAIM_RELEASED'], true);
 
 		if ($success)
 		{
-			meta_refresh(3, $u_return);
+			meta_refresh(self::REDIRECT_DELAY, $u_return);
 		}
 
-		trigger_error($this->language->lang($message) . $this->return_links($u_page, $u_topic, $u_return), $success ? E_USER_NOTICE : E_USER_WARNING);
+		trigger_error($this->language->lang($message) . $this->return_links($u_topic, $u_return), $success ? E_USER_NOTICE : E_USER_WARNING);
 	}
 
 	/**
 	 * Fill the claim page
 	 *
 	 * @param array $topic
 	 * @param array|null $claim
-	 * @param string $u_page
+	 * @param string $u_action Where the page's form posts to
 	 * @param string $u_topic
 	 * @param string $u_return
 	 * @return void
 	 */
-	protected function assign_page(array $topic, $claim, $u_page, $u_topic, $u_return)
+	protected function assign_page(array $topic, $claim, $u_action, $u_topic, $u_return)
 	{
 		$can_manage = $claim !== null && $this->claims->can_manage($claim, $topic['forum_id']);
 		$moderators = [];
@@ -290,12 +321,11 @@ protected function assign_page(array $topic, $claim, $u_page, $u_topic, $u_retur
 			'TOPICCLAIM_FORUM_NAME'		=> $topic['forum_name'],
 			'U_TOPICCLAIM_TOPIC'		=> $u_topic,
 			'U_TOPICCLAIM_FORUM'		=> append_sid("{$this->root_path}viewforum.{$this->php_ext}", 'f=' . $topic['forum_id']),
-			'U_TOPICCLAIM_ACTION'		=> $u_page . ($u_return !== $u_topic ? (strpos($u_page, '?') === false ? '?' : '&') . 'return=mcp' : ''),
+			'U_TOPICCLAIM_ACTION'		=> $u_action,
 			'U_TOPICCLAIM_RETURN'		=> $u_return,
 			'S_TOPICCLAIM_RETURN_MCP'	=> $u_return !== $u_topic,
 
 			'S_TOPICCLAIM_CLAIMED'		=> $claim !== null,
-			'S_TOPICCLAIM_OWN'			=> $claim !== null && $this->claims->is_own($claim),
 			'S_TOPICCLAIM_CAN_MANAGE'	=> $can_manage,
 			'S_TOPICCLAIM_CAN_TAKEOVER'	=> $claim !== null && !$this->claims->is_own($claim) && $this->claims->can_override(),
 			'S_TOPICCLAIM_CAN_TRANSFER'	=> $can_manage && !empty($moderators),
@@ -344,14 +374,13 @@ protected function get_return_url($u_topic)
 	}
 
 	/**
-	 * Links back to the page, the topic and, when it is different, the MCP list
+	 * Links back to the topic and, when the page was opened from it, the MCP list
 	 *
-	 * @param string $u_page
 	 * @param string $u_topic
 	 * @param string $u_return
 	 * @return string HTML
 	 */
-	protected function return_links($u_page, $u_topic, $u_return)
+	protected function return_links($u_topic, $u_return)
 	{
 		$links = '<br /><br />' . $this->language->lang('RETURN_TOPIC', '<a href="' . $u_topic . '">', '</a>');
 

language/en/email/topicclaim_reassign.txt

@@ -0,0 +1,11 @@
+Subject: "{SITENAME}" — Your topic claim was given to another moderator
+
+Hello {USERNAME},
+
+{AUTHOR_NAME} gave your claim on the topic "{TOPIC_TITLE}" to another moderator.
+
+View the topic: {U_TOPIC}
+
+You are receiving this because you held this claim. Change how you are notified under Notification settings in your control panel.
+
+{EMAIL_SIG}

language/en/email/topicclaim_release.txt

@@ -0,0 +1,11 @@
+Subject: "{SITENAME}" — Your topic claim was released
+
+Hello {USERNAME},
+
+{AUTHOR_NAME} released your claim on the topic "{TOPIC_TITLE}".
+
+View the topic: {U_TOPIC}
+
+You are receiving this because you held this claim. Change how you are notified under Notification settings in your control panel.
+
+{EMAIL_SIG}

language/en/email/topicclaim_takeover.txt

@@ -0,0 +1,11 @@
+Subject: "{SITENAME}" — Your topic claim was taken over
+
+Hello {USERNAME},
+
+{AUTHOR_NAME} took over your claim on the topic "{TOPIC_TITLE}".
+
+View the topic: {U_TOPIC}
+
+You are receiving this because you held this claim. Change how you are notified under Notification settings in your control panel.
+
+{EMAIL_SIG}

language/en/email/topicclaim_transfer.txt

@@ -0,0 +1,11 @@
+Subject: "{SITENAME}" — Topic claim transferred to you
+
+Hello {USERNAME},
+
+{AUTHOR_NAME} transferred the claim on the topic "{TOPIC_TITLE}" to you.
+
+View the topic: {U_TOPIC}
+
+You are receiving this because a claim was transferred to you. Change how you are notified under Notification settings in your control panel.
+
+{EMAIL_SIG}

language/en/topicclaim.php

@@ -51,6 +51,8 @@
 	'TOPICCLAIM_TRANSFER_EXPLAIN'	=> 'Moderators who can claim topics in this forum. They are told.',
 	'TOPICCLAIM_RELEASE'			=> 'Release claim',
 	'TOPICCLAIM_RELEASE_EXPLAIN'	=> 'Removes the claim, so another moderator can claim the topic.',
+	'TOPICCLAIM_RELEASE_CONFIRM'	=> 'Are you sure you want to release the claim on this topic?',
+	'TOPICCLAIM_TAKEOVER_CONFIRM'	=> 'Are you sure you want to take over this claim? The moderator who holds it is told.',
 	'TOPICCLAIM_VIEW_ONLY'			=> 'Only the moderator who holds the claim, or an administrator, can change it.',
 	'TOPICCLAIM_LOCKED_NOTICE'		=> 'This topic is locked. The board releases claims on locked topics, so it cannot be claimed.',
 	'TOPICCLAIM_LOCKED_RELEASE'		=> 'The board releases claims on locked topics. Locking this topic releases its claim.',
@@ -59,6 +61,8 @@
 		2	=> 'The board releases claims after %d days.',
 	],
 	'TOPICCLAIM_RETURN_LIST'		=> '%sReturn to the claimed topics%s',
+	'TOPICCLAIM_RETURN_TOPIC'		=> 'Return to the topic',
+	'TOPICCLAIM_RETURN_CLAIMS'		=> 'Return to the claimed topics',
 
 	// Results
 	'TOPICCLAIM_CLAIMED'			=> 'You claimed the topic.',
@@ -91,11 +95,6 @@
 	'TOPICCLAIM_NOTIFICATION_TAKEOVER'	=> '<strong>Topic claim</strong> taken over by %1$s:',
 	'TOPICCLAIM_NOTIFICATION_RELEASE'	=> '<strong>Topic claim</strong> released by %1$s:',
 	'TOPICCLAIM_NOTIFICATION_REASSIGN'	=> '<strong>Topic claim</strong> given to another moderator by %1$s:',
-	'TOPICCLAIM_EMAIL_TRANSFER'			=> 'transferred to you the claim on',
-	'TOPICCLAIM_EMAIL_TAKEOVER'			=> 'took over your claim on',
-	'TOPICCLAIM_EMAIL_RELEASE'			=> 'released your claim on',
-	'TOPICCLAIM_EMAIL_REASSIGN'			=> 'gave another moderator your claim on',
-
 	// Logs
 	'LOG_TOPICCLAIM_CLAIM'			=> '<strong>Claimed topic</strong><br />» %s',
 	'LOG_TOPICCLAIM_TAKEOVER'		=> '<strong>Took over the claim of %2$s on topic</strong><br />» %1$s',

mcp/main_module.php

@@ -51,7 +51,16 @@ public function main($id, $mode)
 			array_keys($auth->acl_getf('f_read', true))
 		));
 
-		list($rows, $total) = $claims->get_list($forum_ids, $show === 'mine' ? (int) $user->data['user_id'] : 0, $start, self::PER_PAGE);
+		$user_id = ($show === 'mine') ? (int) $user->data['user_id'] : 0;
+		list($rows, $total) = $claims->get_list($forum_ids, $user_id, $start, self::PER_PAGE);
+
+		// A page past the end (claims released since) goes to the last page
+		$valid_start = $pagination->validate_start($start, self::PER_PAGE, $total);
+		if ($valid_start !== $start)
+		{
+			$start = $valid_start;
+			list($rows, $total) = $claims->get_list($forum_ids, $user_id, $start, self::PER_PAGE);
+		}
 
 		foreach ($rows as $row)
 		{
@@ -66,7 +75,6 @@ public function main($id, $mode)
 				'U_TOPIC'		=> append_sid("{$phpbb_root_path}viewtopic.$phpEx", 't=' . $row['topic_id']),
 				'U_FORUM'		=> append_sid("{$phpbb_root_path}viewforum.$phpEx", 'f=' . $row['forum_id']),
 				'U_MANAGE'		=> $u_manage . (strpos($u_manage, '?') === false ? '?' : '&') . 'return=mcp',
-				'S_OWN'			=> $claims->is_own($row),
 				'S_CAN_MANAGE'	=> $claims->can_manage($row, $row['forum_id']),
 				'S_LOCKED'		=> $row['topic_status'] === ITEM_LOCKED,
 			]);
@@ -82,7 +90,6 @@ public function main($id, $mode)
 			'U_TOPICCLAIM_MINE'		=> $this->u_action . '&show=mine',
 			'S_TOPICCLAIM_EXPIRE'	=> (int) $config['topicclaim_expire_days'] > 0,
 			'TOPICCLAIM_EXPIRE'		=> $language->lang('TOPICCLAIM_EXPIRE_NOTICE', (int) $config['topicclaim_expire_days']),
-			'S_TOPICCLAIM_RELEASE_LOCKED'	=> !empty($config['topicclaim_release_locked']),
 		]);
 	}
 }