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
F13211804: D12823.diff
Fri, May 17, 6:06 AM
F13196908: D12823.diff
Sun, May 12, 11:40 PM
Unknown Object (File)
Fri, May 3, 8:37 AM
Unknown Object (File)
Thu, Apr 25, 2:38 AM
Unknown Object (File)
Apr 17 2024, 2:59 PM
Unknown Object (File)
Mar 21 2024, 3:22 AM
Unknown Object (File)
Mar 21 2024, 3:22 AM
Unknown Object (File)
Mar 21 2024, 3:22 AM

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.