From eb9645e9b4f4dad9b108fa6bd26a4ba22eb89a66 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 7 May 2012 18:37:04 -0700 Subject: [PATCH] When a user resigns from a commit they have authority over auditing projects for, write resign explicitly Summary: For most actions (like "accept"), we write a row only if you aren't acting on behalf of anything else. This avoids cases like every accept causing two relationships: Some Project | Accept Some User | Accept For "Resign", we must always write the row. Break the logic out and handle it separately. Test Plan: Poked it locally, but let me know if this fixes things? Reviewers: 20after4, btrahan Reviewed By: 20after4 CC: aran Differential Revision: https://secure.phabricator.com/D2423 --- .../comment/PhabricatorAuditCommentEditor.php | 36 ++++++++++++------- 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/src/applications/audit/editor/comment/PhabricatorAuditCommentEditor.php b/src/applications/audit/editor/comment/PhabricatorAuditCommentEditor.php index a503774c9f..1771507857 100644 --- a/src/applications/audit/editor/comment/PhabricatorAuditCommentEditor.php +++ b/src/applications/audit/editor/comment/PhabricatorAuditCommentEditor.php @@ -139,6 +139,30 @@ final class PhabricatorAuditCommentEditor { $request->save(); } } + } else if ($action == PhabricatorAuditActionConstants::RESIGN) { + // "Resign" has unusual rules for writing user rows, only affects the + // user row (never package/project rows), and always affects the user + // row (other actions don't, if they were able to affect a package/project + // row). + $user_request = null; + foreach ($requests as $request) { + if ($request->getAuditorPHID() == $user->getPHID()) { + $user_request = $request; + break; + } + } + if (!$user_request) { + $user_request = id(new PhabricatorRepositoryAuditRequest()) + ->setCommitPHID($commit->getPHID()) + ->setAuditorPHID($user->getPHID()) + ->setAuditReasons(array("Resigned")); + } + + $user_request + ->setAuditStatus(PhabricatorAuditStatusConstants::RESIGNED) + ->save(); + + $requests[] = $user_request; } else { $have_any_requests = false; foreach ($requests as $request) { @@ -170,13 +194,6 @@ final class PhabricatorAuditCommentEditor { $new_status = PhabricatorAuditStatusConstants::CONCERNED; } break; - case PhabricatorAuditActionConstants::RESIGN: - // NOTE: Resigning resigns ONLY your user request, not the requests - // of any projects or packages you are a member of. - if ($request_is_for_user) { - $new_status = PhabricatorAuditStatusConstants::RESIGNED; - } - break; default: throw new Exception("Unknown action '{$action}'!"); } @@ -202,11 +219,6 @@ final class PhabricatorAuditCommentEditor { case PhabricatorAuditActionConstants::CONCERN: $new_status = PhabricatorAuditStatusConstants::CONCERNED; break; - case PhabricatorAuditActionConstants::RESIGN: - // If you're on an audit because of a package, we write an explicit - // resign row to remove it from your queue. - $new_status = PhabricatorAuditStatusConstants::RESIGNED; - break; case PhabricatorAuditActionConstants::CLOSE: // Impossible to reach this block with 'close'. default: