revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2025-02-02 09:58:24 +01:00
Code Issues Releases Wiki Activity

Make some Audit status comparisons more lax, so state transactions only post once

Browse source 
Summary:
Ref T10978. Currently, too many "This audit now <something something>" 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
This commit is contained in:
epriestley 2017-01-27 11:03:34 -08:00
parent 9829ecddd6
commit 1be3ef0227
1 changed files with 2 additions and 2 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
src/applications/audit/editor/PhabricatorAuditEditor.php
View file

@ -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