[aur-dev] [PATCH 1/5] Allow for selecting a reason when closing a request
When closing a package request, Trusted Users can now pick a reason ("Accepted" or "Rejected"). This allows for marking a request as accepted, even if the corresponding package base has already been deleted. Also, the notification email now always explicitly states whether a request has been accepted or closed in the message body. Signed-off-by: Lukas Fleischer <archlinux@cryptocrack.de> --- web/html/index.php | 19 +++++++++++++++++++ web/html/pkgbase.php | 2 +- web/html/pkgreq.php | 12 ++++++++---- web/lib/pkgbasefuncs.inc.php | 4 ++-- web/lib/pkgreqfuncs.inc.php | 27 +++++++++++++++++++++++---- web/template/pkgreq_close_form.php | 24 ++++++++++++++++++++++++ web/template/pkgreq_results.php | 14 +++----------- 7 files changed, 80 insertions(+), 22 deletions(-) create mode 100644 web/template/pkgreq_close_form.php diff --git a/web/html/index.php b/web/html/index.php index 40063f0..554e86c 100644 --- a/web/html/index.php +++ b/web/html/index.php @@ -89,6 +89,25 @@ if (!empty($tokens[1]) && '/' . $tokens[1] == get_pkg_route()) { } include get_route('/' . $tokens[1]); +} elseif (!empty($tokens[1]) && '/' . $tokens[1] == get_pkgreq_route()) { + if (!empty($tokens[2])) { + /* TODO: Create a proper data structure to pass variables from + * the routing framework to the individual pages instead of + * initializing arbitrary variables here. */ + if (!empty($tokens[3]) && $tokens[3] == 'close') { + $pkgreq_id = $tokens[2]; + } else { + $pkgreq_id = null; + } + + if (!$pkgreq_id) { + header("HTTP/1.0 404 Not Found"); + include "./404.php"; + return; + } + } + + include get_route('/' . $tokens[1]); } elseif (!empty($tokens[1]) && '/' . $tokens[1] == get_user_route()) { if (!empty($tokens[2])) { $_REQUEST['ID'] = uid_from_username($tokens[2]); diff --git a/web/html/pkgbase.php b/web/html/pkgbase.php index 5e27e4e..6202180 100644 --- a/web/html/pkgbase.php +++ b/web/html/pkgbase.php @@ -105,7 +105,7 @@ if (check_token()) { $ret = false; } } elseif (current_action("do_CloseRequest")) { - list($ret, $output) = pkgreq_close($_POST['reqid'], false); + list($ret, $output) = pkgreq_close($_POST['reqid'], $_POST['reason']); } if (isset($_REQUEST['comment'])) { diff --git a/web/html/pkgreq.php b/web/html/pkgreq.php index 4b7d6cf..9dec1f6 100644 --- a/web/html/pkgreq.php +++ b/web/html/pkgreq.php @@ -8,7 +8,14 @@ include_once("pkgfuncs.inc.php"); set_lang(); check_sid(); -if (!isset($base_id)) { +if (isset($base_id)) { + html_header(__("File Request")); + include('pkgreq_form.php'); +} elseif (isset($pkgreq_id)) { + html_header(__("Close Request")); + $pkgbase_name = pkgreq_get_pkgbase_name($pkgreq_id); + include('pkgreq_close_form.php'); +} else { if (!check_user_privileges()) { header('Location: /'); exit(); @@ -63,9 +70,6 @@ if (!isset($base_id)) { html_header(__("Requests")); include('pkgreq_results.php'); -} else { - html_header(__("File Request")); - include('pkgreq_form.php'); } html_footer(AUR_VERSION); diff --git a/web/lib/pkgbasefuncs.inc.php b/web/lib/pkgbasefuncs.inc.php index 0ee2c31..4c1f452 100644 --- a/web/lib/pkgbasefuncs.inc.php +++ b/web/lib/pkgbasefuncs.inc.php @@ -540,7 +540,7 @@ function pkgbase_delete ($atype, $base_ids, $merge_base_id, $via) { $dbh->exec($q); if ($via) { - pkgreq_close(intval($via), true); + pkgreq_close(intval($via), 'accepted'); } return array(true, __("The selected packages have been deleted.")); @@ -598,7 +598,7 @@ function pkgbase_adopt ($atype, $base_ids, $action=true, $via) { $dbh->exec($q); if ($via) { - pkgreq_close(intval($via), true); + pkgreq_close(intval($via), 'accepted'); } if ($action) { diff --git a/web/lib/pkgreqfuncs.inc.php b/web/lib/pkgreqfuncs.inc.php index a79677e..0309bc3 100644 --- a/web/lib/pkgreqfuncs.inc.php +++ b/web/lib/pkgreqfuncs.inc.php @@ -41,6 +41,22 @@ function pkgreq_list($offset, $limit) { } /** + * Obtain the package base that belongs to a package request. + * + * @param int $id Package request ID to retrieve the package base for + * + * @return int The name of the corresponding package base + */ +function pkgreq_get_pkgbase_name($id) { + $dbh = DB::connect(); + + $q = "SELECT PackageBaseName FROM PackageRequests "; + $q.= "WHERE ID = " . intval($id); + $result = $dbh->query($q); + return $result->fetch(PDO::FETCH_COLUMN, 0); +} + +/** * File a deletion/orphan request against a package base * * @global string $AUR_LOCATION The AUR's URL used for notification e-mails @@ -139,14 +155,18 @@ function pkgreq_file($ids, $type, $merge_into, $comments) { * @global string $AUR_LOCATION The AUR's URL used for notification e-mails * @global string $AUR_REQUEST_ML The request notification mailing list * @param int $id The package request to close - * @param bool $accepted True if the request has been accepted, false otherwise + * @param string $reason Whether the request was accepted or rejected * * @return array Tuple of success/failure indicator and error message */ -function pkgreq_close($id, $accepted) { +function pkgreq_close($id, $reason) { global $AUR_LOCATION; global $AUR_REQUEST_ML; + if ($reason != 'accepted' && $reason != 'rejected') { + return array(false, __("Invalid reason.")); + } + $dbh = DB::connect(); $id = intval($id); @@ -181,8 +201,7 @@ function pkgreq_close($id, $accepted) { * user who posted the comment was in. */ $username = username_from_sid($_COOKIE['AURSID']); - $body = "Request #" . intval($id) . " has been " . - ($accepted ? "accepted" : "closed") . " by " . + $body = "Request #" . intval($id) . " has been " . $reason . " by " . $username . " [1].\n\n" . "[1] " . $AUR_LOCATION . get_user_uri($username) . "\n"; $body = wordwrap($body, 70); diff --git a/web/template/pkgreq_close_form.php b/web/template/pkgreq_close_form.php new file mode 100644 index 0000000..be36aef --- /dev/null +++ b/web/template/pkgreq_close_form.php @@ -0,0 +1,24 @@ +<div class="box"> + <h2><?= __('Close Request: %s', htmlspecialchars($pkgbase_name)) ?></h2> + <p> + <?= __('Use this form to close the request for package base %s%s%s.', + '<strong>', htmlspecialchars($pkgbase_name), '</strong>'); ?> + </p> + <form action="<?= get_uri('/pkgbase/'); ?>" method="post"> + <fieldset> + <input type="hidden" name="reqid" value="<?= $pkgreq_id ?>" /> + <input type="hidden" name="token" value="<?= htmlspecialchars($_COOKIE['AURSID']) ?>" /> + <p> + <label for="id_reason"><?= __("Reason") ?>:</label> + <select name="reason" id="id_reason"> + <option value="accepted"><?= __('Accepted') ?></option> + <option value="rejected"><?= __('Rejected') ?></option> + </select> + </p> + <p> + <input type="submit" class="button" name="do_CloseRequest" value="<?= __("Close Request") ?>" /> + </p> + </fieldset> + </form> +</div> + diff --git a/web/template/pkgreq_results.php b/web/template/pkgreq_results.php index 9cd6faf..b5e1177 100644 --- a/web/template/pkgreq_results.php +++ b/web/template/pkgreq_results.php @@ -51,8 +51,10 @@ <?php if ($row['BaseID']): ?> <?php if ($row['Type'] == 'deletion'): ?> <a href="<?= get_pkgbase_uri($row['Name']) ?>delete/?via=<?= intval($row['ID']) ?>"><?= __('Accept') ?></a> + <br/ > <?php elseif ($row['Type'] == 'merge'): ?> <a href="<?= get_pkgbase_uri($row['Name']) ?>merge/?into=<?= urlencode($row['MergeInto']) ?>&via=<?= intval($row['ID']) ?>"><?= __('Accept') ?></a> + <br /> <?php elseif ($row['Type'] == 'orphan'): ?> <form action="<?= get_pkgbase_uri($row['Name']) . 'disown/'; ?>" method="post"> <input type="hidden" name="token" value="<?= htmlspecialchars($_COOKIE['AURSID']) ?>" /> @@ -61,17 +63,7 @@ </form> <?php endif; ?> <?php endif; ?> - <form action="<?= get_uri('/pkgbase/'); ?>" method="post"> - <fieldset> - <input type="hidden" name="IDs[<?= $row['BaseID'] ?>]" value="1" /> - <input type="hidden" name="ID" value="<?= $row['BaseID'] ?>" /> - <input type="hidden" name="token" value="<?= htmlspecialchars($_COOKIE['AURSID']) ?>" /> - <input type="hidden" name="reqid" value="<?= $row['ID'] ?>" /> - <div> - <input type="submit" class="button text-button" name="do_CloseRequest" value="<?= __("Close") ?>" /> - </div> - </fieldset> - </form> + <a href="<?= get_pkgreq_route() . '/' . intval($row['ID']) ?>/close/"><?= __('Close') ?></a> </td> <?php else: ?> <td><?= __("Closed") ?></td> -- 2.0.1
This allows Trusted Users to optionally add a comment when closing a request. The comment is included in the notification email that is sent to the requests mailing list. Signed-off-by: Lukas Fleischer <archlinux@cryptocrack.de> --- web/html/pkgbase.php | 2 +- web/lib/pkgbasefuncs.inc.php | 4 ++-- web/lib/pkgreqfuncs.inc.php | 12 +++++++++--- web/template/pkgreq_close_form.php | 8 ++++++++ 4 files changed, 20 insertions(+), 6 deletions(-) diff --git a/web/html/pkgbase.php b/web/html/pkgbase.php index 6202180..9725db7 100644 --- a/web/html/pkgbase.php +++ b/web/html/pkgbase.php @@ -105,7 +105,7 @@ if (check_token()) { $ret = false; } } elseif (current_action("do_CloseRequest")) { - list($ret, $output) = pkgreq_close($_POST['reqid'], $_POST['reason']); + list($ret, $output) = pkgreq_close($_POST['reqid'], $_POST['reason'], $_POST['comments']); } if (isset($_REQUEST['comment'])) { diff --git a/web/lib/pkgbasefuncs.inc.php b/web/lib/pkgbasefuncs.inc.php index 4c1f452..fed46c8 100644 --- a/web/lib/pkgbasefuncs.inc.php +++ b/web/lib/pkgbasefuncs.inc.php @@ -540,7 +540,7 @@ function pkgbase_delete ($atype, $base_ids, $merge_base_id, $via) { $dbh->exec($q); if ($via) { - pkgreq_close(intval($via), 'accepted'); + pkgreq_close(intval($via), 'accepted', ''); } return array(true, __("The selected packages have been deleted.")); @@ -598,7 +598,7 @@ function pkgbase_adopt ($atype, $base_ids, $action=true, $via) { $dbh->exec($q); if ($via) { - pkgreq_close(intval($via), 'accepted'); + pkgreq_close(intval($via), 'accepted', ''); } if ($action) { diff --git a/web/lib/pkgreqfuncs.inc.php b/web/lib/pkgreqfuncs.inc.php index 0309bc3..fccf0b2 100644 --- a/web/lib/pkgreqfuncs.inc.php +++ b/web/lib/pkgreqfuncs.inc.php @@ -156,10 +156,11 @@ function pkgreq_file($ids, $type, $merge_into, $comments) { * @global string $AUR_REQUEST_ML The request notification mailing list * @param int $id The package request to close * @param string $reason Whether the request was accepted or rejected + * @param string $comments Comments to be added to the notification email * * @return array Tuple of success/failure indicator and error message */ -function pkgreq_close($id, $reason) { +function pkgreq_close($id, $reason, $comments) { global $AUR_LOCATION; global $AUR_REQUEST_ML; @@ -202,8 +203,13 @@ function pkgreq_close($id, $reason) { */ $username = username_from_sid($_COOKIE['AURSID']); $body = "Request #" . intval($id) . " has been " . $reason . " by " . - $username . " [1].\n\n" . - "[1] " . $AUR_LOCATION . get_user_uri($username) . "\n"; + $username . " [1]"; + if (!empty(trim($comments))) { + $body .= ":\n\n" . $comments . "\n\n"; + } else { + $body .= ".\n\n"; + } + $body .= "[1] " . $AUR_LOCATION . get_user_uri($username) . "\n"; $body = wordwrap($body, 70); $headers = "MIME-Version: 1.0\r\n" . "Content-type: text/plain; charset=UTF-8\r\n"; diff --git a/web/template/pkgreq_close_form.php b/web/template/pkgreq_close_form.php index be36aef..00efafe 100644 --- a/web/template/pkgreq_close_form.php +++ b/web/template/pkgreq_close_form.php @@ -4,6 +4,10 @@ <?= __('Use this form to close the request for package base %s%s%s.', '<strong>', htmlspecialchars($pkgbase_name), '</strong>'); ?> </p> + <p> + <em><?= __('Note') ?>:</em> + <?= __('The comments field can be left empty. However, it is highly recommended to add a comment when rejecting a request.') ?> + </p> <form action="<?= get_uri('/pkgbase/'); ?>" method="post"> <fieldset> <input type="hidden" name="reqid" value="<?= $pkgreq_id ?>" /> @@ -16,6 +20,10 @@ </select> </p> <p> + <label for="id_comments"><?= __("Comments") ?>:</label> + <textarea name="comments" id="id_comments" rows="5" cols="50"></textarea> + </p> + <p> <input type="submit" class="button" name="do_CloseRequest" value="<?= __("Close Request") ?>" /> </p> </fieldset> -- 2.0.1
Change the subject of notification emails sent when closing a request to be either "Request Accepted" or "Request Rejected". Signed-off-by: Lukas Fleischer <archlinux@cryptocrack.de> --- web/lib/pkgreqfuncs.inc.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web/lib/pkgreqfuncs.inc.php b/web/lib/pkgreqfuncs.inc.php index fccf0b2..8266c67 100644 --- a/web/lib/pkgreqfuncs.inc.php +++ b/web/lib/pkgreqfuncs.inc.php @@ -221,8 +221,8 @@ function pkgreq_close($id, $reason, $comments) { "In-Reply-To: $thread_id\r\n" . "References: $thread_id\r\n" . "X-Mailer: AUR"; - @mail($AUR_REQUEST_ML, "[PRQ#" . $id . "] Request Closed", $body, - $headers); + @mail($AUR_REQUEST_ML, "[PRQ#" . $id . "] Request " . ucfirst($reason), + $body, $headers); return array(true, __("Request closed successfully.")); } -- 2.0.1
Store whether a package request is accepted or rejected and display the reason of each closed request in the request list. Signed-off-by: Lukas Fleischer <archlinux@cryptocrack.de> --- web/lib/pkgreqfuncs.inc.php | 16 ++++++++++++---- web/template/pkgreq_results.php | 12 ++++++++++-- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/web/lib/pkgreqfuncs.inc.php b/web/lib/pkgreqfuncs.inc.php index 8266c67..53cf328 100644 --- a/web/lib/pkgreqfuncs.inc.php +++ b/web/lib/pkgreqfuncs.inc.php @@ -30,11 +30,11 @@ function pkgreq_list($offset, $limit) { $q.= "PackageRequests.MergeBaseName AS MergeInto, "; $q.= "RequestTypes.Name AS Type, PackageRequests.Comments, "; $q.= "Users.Username AS User, PackageRequests.RequestTS, "; - $q.= "PackageRequests.Status "; + $q.= "PackageRequests.Status, PackageRequests.Status = 0 AS Open "; $q.= "FROM PackageRequests INNER JOIN RequestTypes ON "; $q.= "RequestTypes.ID = PackageRequests.ReqTypeID "; $q.= "INNER JOIN Users ON Users.ID = PackageRequests.UsersID "; - $q.= "ORDER BY Status ASC, RequestTS DESC "; + $q.= "ORDER BY Open DESC, RequestTS DESC "; $q.= "LIMIT " . $limit . " OFFSET " . $offset; return $dbh->query($q)->fetchAll(); @@ -164,7 +164,14 @@ function pkgreq_close($id, $reason, $comments) { global $AUR_LOCATION; global $AUR_REQUEST_ML; - if ($reason != 'accepted' && $reason != 'rejected') { + switch ($reason) { + case 'accepted': + $status = 2; + break; + case 'rejected': + $status = 3; + break; + default: return array(false, __("Invalid reason.")); } @@ -175,7 +182,8 @@ function pkgreq_close($id, $reason, $comments) { return array(false, __("Only TUs and developers can close requests.")); } - $q = "UPDATE PackageRequests SET Status = 1 WHERE ID = " . intval($id); + $q = "UPDATE PackageRequests SET Status = " . intval($status) . " "; + $q.= "WHERE ID = " . intval($id); $dbh->exec($q); /* diff --git a/web/template/pkgreq_results.php b/web/template/pkgreq_results.php index b5e1177..261a1f7 100644 --- a/web/template/pkgreq_results.php +++ b/web/template/pkgreq_results.php @@ -45,8 +45,8 @@ <td> <a href="<?= get_uri('/account/') . htmlspecialchars($row['User'], ENT_QUOTES) ?>" title="<?= __('View account information for %s', htmlspecialchars($row['User'])) ?>"><?= htmlspecialchars($row['User']) ?></a> </td> - <td<?php if ($row['Status'] == 0 && time() - intval($row['RequestTS']) > $REQUEST_IDLE_TIME): ?> class="flagged"<?php endif; ?>><?= gmdate("Y-m-d H:i", intval($row['RequestTS'])) ?></td> - <?php if ($row['Status'] == 0): ?> + <td<?php if ($row['Open'] && time() - intval($row['RequestTS']) > $REQUEST_IDLE_TIME): ?> class="flagged"<?php endif; ?>><?= gmdate("Y-m-d H:i", intval($row['RequestTS'])) ?></td> + <?php if ($row['Open']): ?> <td> <?php if ($row['BaseID']): ?> <?php if ($row['Type'] == 'deletion'): ?> @@ -66,7 +66,15 @@ <a href="<?= get_pkgreq_route() . '/' . intval($row['ID']) ?>/close/"><?= __('Close') ?></a> </td> <?php else: ?> + <?php if ($row['Status'] == 1): ?> <td><?= __("Closed") ?></td> + <?php elseif ($row['Status'] == 2): ?> + <td><?= __("Accepted") ?></td> + <?php elseif ($row['Status'] == 3): ?> + <td><?= __("Rejected") ?></td> + <?php else: ?> + <td><?= __("unknown") ?></td> + <?php endif; ?> <?php endif; ?> </tr> <?php endwhile; ?> -- 2.0.1
Move the check introduced in 06b7099 (Validate package base name when filing requests, 2014-07-02) from pkgbase.php to pkgreq_file(). Signed-off-by: Lukas Fleischer <archlinux@cryptocrack.de> --- web/html/pkgbase.php | 7 +------ web/lib/pkgreqfuncs.inc.php | 4 ++++ 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/web/html/pkgbase.php b/web/html/pkgbase.php index 9725db7..cf2b774 100644 --- a/web/html/pkgbase.php +++ b/web/html/pkgbase.php @@ -98,12 +98,7 @@ if (check_token()) { } elseif (current_action("do_ChangeCategory")) { list($ret, $output) = pkgbase_change_category($base_id, $atype); } elseif (current_action("do_FileRequest")) { - if (empty($_POST['merge_into']) || preg_match("/^[a-z0-9][a-z0-9\.+_-]*$/", $_POST['merge_into'])) { - list($ret, $output) = pkgreq_file($ids, $_POST['type'], $_POST['merge_into'], $_POST['comments']); - } else { - $output = __("Invalid name: only lowercase letters are allowed."); - $ret = false; - } + list($ret, $output) = pkgreq_file($ids, $_POST['type'], $_POST['merge_into'], $_POST['comments']); } elseif (current_action("do_CloseRequest")) { list($ret, $output) = pkgreq_close($_POST['reqid'], $_POST['reason'], $_POST['comments']); } diff --git a/web/lib/pkgreqfuncs.inc.php b/web/lib/pkgreqfuncs.inc.php index 53cf328..76780fe 100644 --- a/web/lib/pkgreqfuncs.inc.php +++ b/web/lib/pkgreqfuncs.inc.php @@ -72,6 +72,10 @@ function pkgreq_file($ids, $type, $merge_into, $comments) { global $AUR_LOCATION; global $AUR_REQUEST_ML; + if (!empty($merge_into) && !preg_match("/^[a-z0-9][a-z0-9\.+_-]*$/", $merge_into)) { + return array(false, __("Invalid name: only lowercase letters are allowed.")); + } + if (empty($comments)) { return array(false, __("The comment field must not be empty.")); } -- 2.0.1
participants (1)
-
Lukas Fleischer