From 1be3ef02276812296c01e41122f19d6ea8077f81 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 27 Jan 2017 11:03:34 -0800 Subject: [PATCH] Make some Audit status comparisons more lax, so state transactions only post once Summary: Ref T10978. Currently, too many "This audit now " transactions are posting, because this strict `===` check is failing to detect that the audit is already in the same state. This is because audit states are currently integers, and saving an integer to the database and then reading it back turns it into a string. This is a whole separate can of worms. For now, just weaken the comparison. I'd eventually like to use string constants here instead of integer constants. Test Plan: Commented on a "no audit required" commit, didn't see a double "this doesn't need audit" transaction anymore. Also made a legit state change and did see a state transaction. Reviewers: chad Reviewed By: chad Maniphest Tasks: T10978 Differential Revision: https://secure.phabricator.com/D17258 --- src/applications/audit/editor/PhabricatorAuditEditor.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/applications/audit/editor/PhabricatorAuditEditor.php b/src/applications/audit/editor/PhabricatorAuditEditor.php index fe904913ae..952cf50a77 100644 --- a/src/applications/audit/editor/PhabricatorAuditEditor.php +++ b/src/applications/audit/editor/PhabricatorAuditEditor.php @@ -289,8 +289,8 @@ final class PhabricatorAuditEditor // If the commit has changed state after this edit, add an informational // transaction about the state change. - if ($old_status !== $new_status) { - if ($new_status === $partial_status) { + if ($old_status != $new_status) { + if ($new_status == $partial_status) { // This state isn't interesting enough to get a transaction. The // best way we could lead the user forward is something like "This // commit still requires additional audits." but that's redundant and