Page MenuHomePhabricator

When an edit overrides an object lock, note it in the transaction record
ClosedPublic

Authored by epriestley on Feb 8 2019, 12:20 AM.

Details

Summary

Ref T13244. See PHI1059. When you lock a task, users who can edit the task can currently override the lock by using "Edit Task" if they confirm that they want to do this.

Mark these edits with an emblem, similar to the "MFA" and "Silent" emblems, so it's clear that they may have bent the rules.

Also, make the "MFA" and "Silent" emblems more easily visible.

Test Plan

Edited a locked task, overrode the lock, got marked for it.

Diff Detail

Repository
rP Phabricator
Branch
lock1
Lint
Lint OK
Unit
Unit Tests OK
Build Status
Buildable 21905
Build 29904: Run Core Tests
Build 29903: arc lint + arc unit

Unit TestsFailed

TimeTest
291 msPhabricatorProjectCoreTestCase::Unknown Unit Message ("")
EXCEPTION (PhabricatorDataNotAttachedException): Attempting to access attached data on PhabricatorProject, but the data is not actually attached. Before accessing attachable data on an object, you must load and attach it. Data is normally attached by calling the corresponding needX() method on the Query class when the object is loaded. You can also call the corresponding attachX() method explicitly.
402 msPhabricatorProjectCoreTestCase::Unknown Unit Message ("")
EXCEPTION (PhabricatorDataNotAttachedException): Attempting to access attached data on PhabricatorProject, but the data is not actually attached. Before accessing attachable data on an object, you must load and attach it. Data is normally attached by calling the corresponding needX() method on the Query class when the object is loaded. You can also call the corresponding attachX() method explicitly.
1 msAlmanacNamesTestCase::Unknown Unit Message ("")
30 assertions passed.
0 msAlmanacServiceTypeTestCase::Unknown Unit Message ("")
1 assertion passed.
0 msAphrontHTTPSinkTestCase::Unknown Unit Message ("")
2 assertions passed.
View Full Test Results (2 Failed · 373 Passed)

Event Timeline

epriestley created this revision.Feb 8 2019, 12:20 AM
Harbormaster returned this revision to the author for changes because remote builds failed.Feb 8 2019, 12:22 AM
Harbormaster failed remote builds in B21905: Diff 48068!
epriestley updated this revision to Diff 48069.Feb 8 2019, 12:44 AM
  • Tailor the interaction check to avoid some complications in Projects unit tests, where membership is sometimes not fully loaded and we're editing on behalf of some random actor.
epriestley requested review of this revision.Feb 8 2019, 12:46 AM
aeiser added a subscriber: aeiser.Feb 8 2019, 1:17 AM

Wasn't there supposed to be an icon getting upstreamed in this revision? I can see the change to the celerity map but not the associated icon file.

src/applications/transactions/view/PhabricatorApplicationTransactionView.php
420

I'm pretty sure this was not what you meant, since $lock isn't used again in this function and I've never seen the "use assignment operation's side effect as a function argument" idiom in the Phabricator code base.

There's no new icon, just a new way to render them (as "emblems"). The celerity map change is just because the CSS changed.

($icon = ... was debugging code, though.)

amckinley accepted this revision.Feb 8 2019, 10:21 PM
This revision is now accepted and ready to land.Feb 8 2019, 10:21 PM
epriestley updated this revision to Diff 48072.Feb 9 2019, 1:57 PM
  • Remove stray $lock = .
This revision was automatically updated to reflect the committed changes.