[aur-dev] [PATCH v3 3/3] Set correct 'My Account' link after changing username
Lukas Fleischer
lfleischer at archlinux.org
Thu Jul 16 07:07:11 UTC 2015
On Thu, 16 Jul 2015 at 00:56:32, Marcel Korpel wrote:
> Don't print messages (and the account form) in process_account_form()
> anymore, but return them to the caller. When updating accounts, this
> function will be called before the headers are written.
>
There is a lot of information missing here. Why does moving the
process_account_form() invocation help? You should mention that the
header template retrieves the user name from the database itself and
will obtain updated information if the perform the UPDATE query before
including that template. I had a hard time understanding this change and
had to browse the code to check why this works.
> Signed-off-by: Marcel Korpel <marcel.korpel at gmail.com>
> ---
> Change since v2:
> * Don't buffer output anymore and send a new (GET) request, but process
> process the data before outputting headers.
>
> web/html/account.php | 29 ++++++++++++++++++-----------
> web/html/register.php | 11 ++++++++++-
> web/lib/acctfuncs.inc.php | 31 ++++++++++++++++---------------
> 3 files changed, 44 insertions(+), 27 deletions(-)
>
> diff --git a/web/html/account.php b/web/html/account.php
> index f5e6c19..9aa48a3 100644
> --- a/web/html/account.php
> +++ b/web/html/account.php
> @@ -19,6 +19,23 @@ if (in_array($action, $need_userinfo)) {
> $PK = implode("\n", account_get_ssh_keys($row["ID"]));
> }
>
> +/* This has to be done before the navigation headers are written */
> +if ($action == "UpdateAccount") {
> + $updateAccountMessage = '';
We use underscores ($update_account_message) instead of camelCase.
> + /* Details for account being updated */
> + /* Verify user permissions and that the request is a valid POST */
> + if (can_edit_account($row) && check_token()) {
> + /* Update the details for the existing account */
> + list($success, $updateAccountMessage) = process_account_form(
> + "edit", "UpdateAccount",
> + in_request("U"), in_request("T"), in_request("S"),
> + in_request("E"), in_request("P"), in_request("C"),
> + in_request("R"), in_request("L"), in_request("I"),
> + in_request("K"), in_request("PK"), in_request("J"),
> + in_request("ID"), $row["Username"]);
> + }
> +}
> +
> if ($action == "AccountInfo") {
> html_header(__('Account') . ' ' . $row['Username']);
> } else {
> @@ -91,17 +108,7 @@ if (isset($_COOKIE["AURSID"])) {
> }
>
> } elseif ($action == "UpdateAccount") {
> - /* Details for account being updated */
> - /* Verify user permissions and that the request is a valid POST */
> - if (can_edit_account($row) && check_token()) {
> - /* Update the details for the existing account */
> - process_account_form("edit", "UpdateAccount",
> - in_request("U"), in_request("T"), in_request("S"),
> - in_request("E"), in_request("P"), in_request("C"),
> - in_request("R"), in_request("L"), in_request("I"),
> - in_request("K"), in_request("PK"), in_request("J"),
> - in_request("ID"), $row["Username"]);
> - }
> + print $updateAccountMessage;
> } else {
> if (has_credential(CRED_ACCOUNT_SEARCH)) {
> # display the search page if they're a TU/dev
> diff --git a/web/html/register.php b/web/html/register.php
> index cb3e8dd..9c5c1cc 100644
> --- a/web/html/register.php
> +++ b/web/html/register.php
> @@ -19,11 +19,20 @@ echo '<div class="box">';
> echo '<h2>' . __('Register') . '</h2>';
>
> if (in_request("Action") == "NewAccount") {
> - process_account_form("new", "NewAccount", in_request("U"), 1, 0,
> + list($success, $message) = process_account_form(
> + "new", "NewAccount", in_request("U"), 1, 0,
> in_request("E"), '', '', in_request("R"),
> in_request("L"), in_request("I"), in_request("K"),
> in_request("PK"));
>
> + print $message;
> +
> + if (!$success) {
> + display_account_form("NewAccount", in_request("U"), 1, 0,
> + in_request("E"), '', '', in_request("R"),
> + in_request("L"), in_request("I"), in_request("K"),
> + in_request("PK"));
> + }
> } else {
> print '<p>' . __("Use this form to create an account.") . '</p>';
> display_account_form("NewAccount", "", "", "", "", "", "", "", $LANG);
> diff --git a/web/lib/acctfuncs.inc.php b/web/lib/acctfuncs.inc.php
> index 9d6f5ee..8dd26c7 100644
> --- a/web/lib/acctfuncs.inc.php
> +++ b/web/lib/acctfuncs.inc.php
> @@ -89,13 +89,14 @@ function display_account_form($A,$U="",$T="",$S="",$E="",$P="",$C="",$R="",
> * @param string $UID The user ID of the modified account
> * @param string $N The username as present in the database
> *
> - * @return string|void Return void if successful, otherwise return error
> + * @return array boolean indication success and message to be printed
s/boolean indication/Boolean indicating/
> */
> function process_account_form($TYPE,$A,$U="",$T="",$S="",$E="",$P="",$C="",
> $R="",$L="",$I="",$K="",$PK="",$J="",$UID=0,$N="") {
> global $SUPPORTED_LANGS;
>
> $error = '';
> + $message = '';
>
> if (is_ipbanned()) {
> $error = __('Account registration has been disabled ' .
> @@ -247,10 +248,8 @@ function process_account_form($TYPE,$A,$U="",$T="",$S="",$E="",$P="",$C="",
> }
>
> if ($error) {
> - print "<ul class='errorlist'><li>".$error."</li></ul>\n";
> - display_account_form($A, $U, $T, $S, $E, "", "",
> - $R, $L, $I, $K, $PK, $J, $UID, $N);
You dropped this here but only added it to one of the call sites
(account registration). Which part of the code includes the account form
when an error occurs during an update? Either needs a fix or
clarification (in the commit message and in the form of a comment).
Thanks!
> - return;
> + $message = "<ul class='errorlist'><li>".$error."</li></ul>\n";
> + return array(false, $message);
> }
>
> if ($TYPE == "new") {
> @@ -278,25 +277,25 @@ function process_account_form($TYPE,$A,$U="",$T="",$S="",$E="",$P="",$C="",
> $q.= "$I, $K)";
> $result = $dbh->exec($q);
> if (!$result) {
> - print __("Error trying to create account, %s%s%s.",
> + $message = __("Error trying to create account, %s%s%s.",
> "<strong>", htmlspecialchars($U,ENT_QUOTES), "</strong>");
> - return;
> + return array(false, $message);
> }
>
> $uid = $dbh->lastInsertId();
> account_set_ssh_keys($uid, $ssh_keys, $ssh_fingerprints);
>
> - print __("The account, %s%s%s, has been successfully created.",
> + $message = __("The account, %s%s%s, has been successfully created.",
> "<strong>", htmlspecialchars($U,ENT_QUOTES), "</strong>");
> - print "<p>\n";
> + $message .= "<p>\n";
>
> if ($send_resetkey) {
> send_resetkey($email, true);
> - print __("A password reset key has been sent to your e-mail address.");
> - print "</p>\n";
> + $message .= __("A password reset key has been sent to your e-mail address.");
> + $message .= "</p>\n";
> } else {
> - print __("Click on the Login link above to use your account.");
> - print "</p>\n";
> + $message .= __("Click on the Login link above to use your account.");
> + $message .= "</p>\n";
> }
> } else {
> /* Modify an existing account. */
> @@ -341,13 +340,15 @@ function process_account_form($TYPE,$A,$U="",$T="",$S="",$E="",$P="",$C="",
> $ssh_key_result = account_set_ssh_keys($UID, $ssh_keys, $ssh_fingerprints);
>
> if ($result === false || $ssh_key_result === false) {
> - print __("No changes were made to the account, %s%s%s.",
> + $message = __("No changes were made to the account, %s%s%s.",
> "<strong>", htmlspecialchars($U,ENT_QUOTES), "</strong>");
> } else {
> - print __("The account, %s%s%s, has been successfully modified.",
> + $message = __("The account, %s%s%s, has been successfully modified.",
> "<strong>", htmlspecialchars($U,ENT_QUOTES), "</strong>");
> }
> }
> +
> + return array(true, $message);
> }
>
> /**
> --
> 2.4.5
More information about the aur-dev
mailing list