diff --git a/resources/sql/patches/110.commitaudit.sql b/resources/sql/patches/110.commitaudit.sql new file mode 100644 index 0000000000..3f04e4ed5e --- /dev/null +++ b/resources/sql/patches/110.commitaudit.sql @@ -0,0 +1,11 @@ +ALTER TABLE phabricator_repository.repository_commit + ADD mailKey VARCHAR(20) NOT NULL; + +ALTER TABLE phabricator_repository.repository_commit + ADD authorPHID VARCHAR(64) BINARY; + +ALTER TABLE phabricator_repository.repository_commit + ADD auditStatus INT UNSIGNED NOT NULL; + +ALTER TABLE phabricator_repository.repository_commit + ADD KEY (authorPHID, auditStatus, epoch); \ No newline at end of file diff --git a/resources/sql/patches/111.commitauditmigration.php b/resources/sql/patches/111.commitauditmigration.php new file mode 100644 index 0000000000..c89702f808 --- /dev/null +++ b/resources/sql/patches/111.commitauditmigration.php @@ -0,0 +1,71 @@ +establishConnection('w'); +$data = new PhabricatorRepositoryCommitData(); +$commits = queryfx_all( + $conn, + 'SELECT c.id id, c.authorPHID authorPHID, d.commitDetails details + FROM %T c JOIN %T d ON d.commitID = c.id + WHERE c.authorPHID IS NULL', + $table->getTableName(), + $data->getTableName()); + +foreach ($commits as $commit) { + $id = $commit['id']; + $details = json_decode($commit['details'], true); + $author_phid = idx($details, 'authorPHID'); + if ($author_phid) { + queryfx( + $conn, + 'UPDATE %T SET authorPHID = %s WHERE id = %d', + $table->getTableName(), + $author_phid, + $id); + echo "#{$id}\n"; + } +} + +echo "Done.\n"; + + +echo "Updating old commit mailKeys...\n"; + +$table = new PhabricatorRepositoryCommit(); +$conn = $table->establishConnection('w'); +$commits = queryfx_all( + $conn, + 'SELECT id FROM %T WHERE mailKey = %s', + $table->getTableName(), + ''); + +foreach ($commits as $commit) { + $id = $commit['id']; + queryfx( + $conn, + 'UPDATE %T SET mailKey = %s WHERE id = %d', + $table->getTableName(), + Filesystem::readRandomCharacters(20), + $id); + echo "#{$id}\n"; +} + +echo "Done.\n"; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 610a085893..aebfab9a26 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -444,6 +444,7 @@ phutil_register_library_map(array( 'PhabricatorAuditAddCommentController' => 'applications/audit/controller/addcomment', 'PhabricatorAuditComment' => 'applications/audit/storage/auditcomment', 'PhabricatorAuditCommentEditor' => 'applications/audit/editor/comment', + 'PhabricatorAuditCommitStatusConstants' => 'applications/audit/constants/commitstatus', 'PhabricatorAuditController' => 'applications/audit/controller/base', 'PhabricatorAuditDAO' => 'applications/audit/storage/base', 'PhabricatorAuditEditController' => 'applications/audit/controller/edit', diff --git a/src/applications/audit/constants/commitstatus/PhabricatorAuditCommitStatusConstants.php b/src/applications/audit/constants/commitstatus/PhabricatorAuditCommitStatusConstants.php new file mode 100644 index 0000000000..27c31177a4 --- /dev/null +++ b/src/applications/audit/constants/commitstatus/PhabricatorAuditCommitStatusConstants.php @@ -0,0 +1,43 @@ + 'None', + self::NEEDS_AUDIT => 'Audit Required', + self::CONCERN_RAISED => 'Concern Raised', + self::PARTIALLY_AUDITED => 'Partially Audited', + self::FULLY_AUDITED => 'Audited', + ); + + return $map; + } + + public static function getStatusName($code) { + return idx(self::getStatusNameMap(), $code, 'Unknown'); + } + +} diff --git a/src/applications/audit/constants/commitstatus/__init__.php b/src/applications/audit/constants/commitstatus/__init__.php new file mode 100644 index 0000000000..c1b73f7483 --- /dev/null +++ b/src/applications/audit/constants/commitstatus/__init__.php @@ -0,0 +1,12 @@ +setAuditReasons(array("Voluntary Participant")) ->save(); + $relationships[] = $relationship; } + $commit->updateAuditStatus($relationships); + $commit->save(); + $this->publishFeedStory($comment, array_keys($audit_phids)); PhabricatorSearchCommitIndexer::indexCommit($commit); $this->sendMail($comment, $other_comments); diff --git a/src/applications/diffusion/controller/commit/DiffusionCommitController.php b/src/applications/diffusion/controller/commit/DiffusionCommitController.php index dc3bb2afb9..858a52f51a 100644 --- a/src/applications/diffusion/controller/commit/DiffusionCommitController.php +++ b/src/applications/diffusion/controller/commit/DiffusionCommitController.php @@ -268,6 +268,11 @@ class DiffusionCommitController extends DiffusionController { $props['Differential Revision'] = $handles[$revision_phid]->renderLink(); } + if ($commit->getAuditStatus()) { + $props['Audit'] = PhabricatorAuditCommitStatusConstants::getStatusName( + $commit->getAuditStatus()); + } + $request = $this->getDiffusionRequest(); $contains = DiffusionContainsQuery::newFromDiffusionRequest($request); diff --git a/src/applications/diffusion/controller/commit/__init__.php b/src/applications/diffusion/controller/commit/__init__.php index 570374bb62..6913106d67 100644 --- a/src/applications/diffusion/controller/commit/__init__.php +++ b/src/applications/diffusion/controller/commit/__init__.php @@ -7,6 +7,7 @@ phutil_require_module('phabricator', 'applications/audit/constants/action'); +phutil_require_module('phabricator', 'applications/audit/constants/commitstatus'); phutil_require_module('phabricator', 'applications/audit/editor/comment'); phutil_require_module('phabricator', 'applications/audit/query/audit'); phutil_require_module('phabricator', 'applications/audit/storage/auditcomment'); diff --git a/src/applications/repository/storage/commit/PhabricatorRepositoryCommit.php b/src/applications/repository/storage/commit/PhabricatorRepositoryCommit.php index fa55ab5c05..ee5a9c65f1 100644 --- a/src/applications/repository/storage/commit/PhabricatorRepositoryCommit.php +++ b/src/applications/repository/storage/commit/PhabricatorRepositoryCommit.php @@ -22,9 +22,9 @@ class PhabricatorRepositoryCommit extends PhabricatorRepositoryDAO { protected $phid; protected $commitIdentifier; protected $epoch; - - // TODO: Add this! - // protected $mailKey; + protected $mailKey; + protected $authorPHID; + protected $auditStatus = PhabricatorAuditCommitStatusConstants::NONE; private $commitData; @@ -61,9 +61,11 @@ class PhabricatorRepositoryCommit extends PhabricatorRepositoryDAO { return $this->commitData; } - public function getMailKey() { - // TODO: Fix properly! - return $this->phid; + public function save() { + if (!$this->mailKey) { + $this->mailKey = Filesystem::readRandomCharacters(20); + } + return parent::save(); } public function delete() { @@ -79,4 +81,46 @@ class PhabricatorRepositoryCommit extends PhabricatorRepositoryDAO { return $result; } + /** + * Synchronize a commit's overall audit status with the individual audit + * triggers. + */ + public function updateAuditStatus(array $rships) { + $any_concern = false; + $any_accept = false; + $any_need = false; + + foreach ($rships as $rship) { + switch ($rship->getAuditStatus()) { + case PhabricatorAuditStatusConstants::AUDIT_REQUIRED: + $any_need = true; + break; + case PhabricatorAuditStatusConstants::ACCEPTED: + $any_accept = true; + break; + case PhabricatorAuditStatusConstants::CONCERNED: + $any_concern = true; + break; + } + } + + if ($any_concern) { + $status = PhabricatorAuditCommitStatusConstants::CONCERN_RAISED; + } else if ($any_accept) { + if ($any_need) { + $status = PhabricatorAuditCommitStatusConstants::PARTIALLY_AUDITED; + } else { + $status = PhabricatorAuditCommitStatusConstants::FULLY_AUDITED; + } + } else if ($any_need) { + $status = PhabricatorAuditCommitStatusConstants::NEEDS_AUDIT; + } else { + $status = PhabricatorAuditCommitStatusConstants::NONE; + } + + return $this->setAuditStatus($status); + } + + + } diff --git a/src/applications/repository/storage/commit/__init__.php b/src/applications/repository/storage/commit/__init__.php index 64ca0a27b8..f58cc74617 100644 --- a/src/applications/repository/storage/commit/__init__.php +++ b/src/applications/repository/storage/commit/__init__.php @@ -6,11 +6,14 @@ +phutil_require_module('phabricator', 'applications/audit/constants/commitstatus'); +phutil_require_module('phabricator', 'applications/audit/constants/status'); phutil_require_module('phabricator', 'applications/phid/constants'); phutil_require_module('phabricator', 'applications/phid/storage/phid'); phutil_require_module('phabricator', 'applications/repository/storage/base'); phutil_require_module('phabricator', 'applications/repository/storage/commitdata'); +phutil_require_module('phutil', 'filesystem'); phutil_require_module('phutil', 'utils'); diff --git a/src/applications/repository/worker/herald/PhabricatorRepositoryCommitHeraldWorker.php b/src/applications/repository/worker/herald/PhabricatorRepositoryCommitHeraldWorker.php index 1d2e23a71b..20173916e4 100644 --- a/src/applications/repository/worker/herald/PhabricatorRepositoryCommitHeraldWorker.php +++ b/src/applications/repository/worker/herald/PhabricatorRepositoryCommitHeraldWorker.php @@ -151,9 +151,8 @@ EOBODY; $table = new PhabricatorOwnersPackageCommitRelationship(); $rships = $table->loadAllWhere( - 'commitPHID = %s AND packagePHID IN (%Ls)', - $commit->getPHID(), - array_keys($map)); + 'commitPHID = %s', + $commit->getPHID()); $rships = mpull($rships, null, 'getPackagePHID'); $rules = mpull($rules, null, 'getID'); @@ -179,5 +178,7 @@ EOBODY; $rship->save(); } + $commit->updateAuditStatus($rships); + $commit->save(); } } diff --git a/src/applications/repository/worker/owner/PhabricatorRepositoryCommitOwnersWorker.php b/src/applications/repository/worker/owner/PhabricatorRepositoryCommitOwnersWorker.php index d6728be5c7..01783ab739 100644 --- a/src/applications/repository/worker/owner/PhabricatorRepositoryCommitOwnersWorker.php +++ b/src/applications/repository/worker/owner/PhabricatorRepositoryCommitOwnersWorker.php @@ -30,11 +30,14 @@ class PhabricatorRepositoryCommitOwnersWorker $affected_paths); if ($affected_packages) { - foreach ($affected_packages as $package) { - $relationship = id(new PhabricatorOwnersPackageCommitRelationship()) - ->loadOneWhere('packagePHID=%s AND commitPHID=%s', - $package->getPHID(), + $rships = id(new PhabricatorOwnersPackageCommitRelationship()) + ->loadAllWhere( + 'commitPHID = %s', $commit->getPHID()); + $rships = mpull($rships, null, 'getPackagePHID'); + + foreach ($affected_packages as $package) { + $relationship = idx($rships, $package->getPHID()); // Don't update relationship if it exists already if (!$relationship) { @@ -59,8 +62,13 @@ class PhabricatorRepositoryCommitOwnersWorker $relationship->setAuditStatus($audit_status); $relationship->save(); + + $rships[] = $relationship; } } + + $commit->updateAuditStatus($rships); + $commit->save(); } if ($this->shouldQueueFollowupTasks()) { @@ -90,7 +98,11 @@ class PhabricatorRepositoryCommitOwnersWorker } $revision_id = $data->getCommitDetail('differential.revisionID'); + $revision_author_phid = null; + $commit_reviewedby_phid = null; + $commit_author_phid = null; + if ($revision_id) { $revision = id(new DifferentialRevision())->load($revision_id); if ($revision) {