Adds an authorPHIDs, populates olds ones.
Details
Details
- Reviewers
epriestley - Commits
- rP59f3b5125d5a: Add authorPHID to Dashboards
Make a new Dashboard, see that I created it.
Diff Detail
Diff Detail
- Repository
- rP Phabricator
- Branch
- authored-dashboards (branched from master)
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 14865 Build 19459: Run Core Tests Build 19458: arc lint + arc unit
Event Timeline
Comment Actions
Let's make the column non-nullable since it will always be populated going forward.
You can backpopulate it by writing a migration that does this:
- Iterate over every dashboard.
- For each dashboard, select the authorPHID of the first transaction which applied to the dashboard, something like SELECT authorPHID FROM %T WHERE objectPHID = %s ORDER BY id ASC LIMIT 1
- If there's no such transaction, just use the Dashboards application PHID so the UI doesn't get too goofy? This should be rare.
| src/applications/dashboard/query/PhabricatorDashboardQuery.php | ||
|---|---|---|
| 29 | To be more explicit, prefer withAuthorPHIDs(). | |
| src/applications/dashboard/query/PhabricatorDashboardSearchEngine.php | ||
| 23 | For consistency, prefer to use authorPHIDs as the internal key. You can use setAliases(array('author', 'authors', 'authored', 'authorPHID')) or similar if you want to support more readable GET parameters. | |
| resources/sql/autopatches/21061210.dashboards.01.author.php | ||
|---|---|---|
| 22 | is this correct? | |
Comment Actions
One actual issue and a couple of nitpicks. Like 98% of this is correct.
| resources/sql/autopatches/20161210.dashboards.01.author.sql | ||
|---|---|---|
| 1–2 | Should be NOT NULL now. | |
| resources/sql/autopatches/21061210.dashboards.01.author.php | ||
| 14 | Maybe do if ($dashboard->getAuthorPHID()) { continue; } here so we don't mess anything up if someone accidentally runs this later and the data is whack. | |
| 22 | I don't think this will work, since we access $author_phid['authorPHID'] below, but we're just assigning a plain PHID here. Maybe call the first variable $author_row, then derive $author_phid here? if (!$author_row) {
$author_phid = // application PHID
} else {
$author_phid = $author_row['authorPHID'];
}Then just use plain $author_phid below. You can re-run this to test your changes by doing UPDATE whatever SET authorPHID = '', then using bin/storage upgrade --apply phabricator:2016110.... | |
| src/applications/dashboard/query/PhabricatorDashboardSearchEngine.php | ||
| 24 | Oh, also maybe this should be PhabricatorPeopleUserFunctionDatasource since viewer() should work here. | |
| src/applications/dashboard/storage/PhabricatorDashboard.php | ||
| 66 | Just phid now? | |