Page MenuHomePhabricator

Trigger notifications when a dependent differential is closed
Needs ReviewPublic

Authored by joshuaspence on May 13 2015, 12:01 PM.
Tags
None
Referenced Files
Unknown Object (File)
Apr 24 2025, 4:11 AM
Unknown Object (File)
Apr 22 2025, 3:14 AM
Unknown Object (File)
Apr 21 2025, 4:27 AM
Unknown Object (File)
Apr 20 2025, 10:27 PM
Unknown Object (File)
Apr 20 2025, 5:10 PM
Unknown Object (File)
Apr 19 2025, 2:15 PM
Unknown Object (File)
Apr 10 2025, 9:02 PM
Unknown Object (File)
Apr 10 2025, 9:02 PM

Details

Summary

Fixes T7453. Adds a new transaction type (DifferentialTransaction::TYPE_UNBLOCK) so that transactions are created (and notifications are generated) when a revision's dependencies are closed/reopen.

Test Plan

Closed and reopened a dependent diff several times. Admiteddly, I didn't test the notifications side-of-things, which I just assumed plays nicely with the transactions.

Diff Detail

Repository
rP Phabricator
Branch
master
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 6954
Build 6976: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

joshuaspence retitled this revision from to Trigger notifications when a dependent differential is closed.
joshuaspence updated this object.
joshuaspence edited the test plan for this revision. (Show Details)
joshuaspence edited edge metadata.

Seems to mostly work

This seems to mostly work except that the following exception is thrown:

Attempting to apply a transaction (of class "DifferentialTransaction", with type "differential:unblock") which has not been constructed correctly: This transaction should generate its oldValue automatically, but has already had one set!
joshuaspence edited the test plan for this revision. (Show Details)
src/applications/differential/editor/DifferentialTransactionEditor.php
792–793

I assume that these are needed in order to be able to generate notifications for the susbcribers to the revision, but DifferentialRevisionQuery doesn't provide these methods.

src/applications/differential/storage/DifferentialTransaction.php
7–12

This is somewhat inconsistent with other PhabricatorApplicationTransaction classes. For example:

final class ManiphestTransaction
  extends PhabricatorApplicationTransaction {

  const TYPE_TITLE = 'title';
  const TYPE_STATUS = 'status';
  const TYPE_DESCRIPTION = 'description';
  const TYPE_OWNER  = 'reassign';
  const TYPE_PRIORITY = 'priority';
  const TYPE_EDGE = 'edge';
  const TYPE_SUBPRIORITY = 'subpriority';
  const TYPE_PROJECT_COLUMN = 'projectcolumn';
  const TYPE_MERGED_INTO = 'mergedinto';
  const TYPE_MERGED_FROM = 'mergedfrom';
  const TYPE_UNBLOCK = 'unblock';
326–338

Not sure if I need a default here

497–516

As above.