Fixes T12832. Adds a basic table (not paginated?) to view tracking and autoclose status.
Details
Details
- Reviewers
epriestley - Maniphest Tasks
- T12832: Add autoclose table to Diffusion Manage
- Commits
- rPa7124f8f7a11: Add status table to Diffusion Branch manage page
Review a large repository (Krita) with setting various states of tracking and autoclose.
Diff Detail
Diff Detail
- Repository
- rP Phabricator
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
- You can paginate by following the logic in DiffusionBranchTableController, i.e. use PHUIPagerView to manage an offset and limit, and pass offset and limit to branchquery.
- You should use $repository->shouldAutocloseBranch() and $repository->shouldTrackBranch() to test if a branch is autoclose and tracked, respectively. The logic as written is incorrect because these fields accept regular expressions, not just literal branch names (see "Branches" in Diffusion User Guide: Managing Repositories).
| src/applications/diffusion/management/DiffusionRepositoryBranchesManagementPanel.php | ||
|---|---|---|
| 198–203 | This is unnecessary if all columns are visible. | |
Comment Actions
$repository->shouldAutocloseBranch() seems to not work if the repository is still importing. It will just say "off". I can get a reason, "auto/importing". Not sure what to show the user.
Comment Actions
That's PhabricatorRepository::BECAUSE_REPOSITORY_IMPORTING which means "This is an autoclose branch, but we aren't autoclosing commits on it yet because the repository hasn't finished its initial first-time import.".
This rule addresses the case where installs may choose to (for example) import the phabricator/ repository locally, but forget to disable "Autoclose". If we autoclosed during initial import, their install would see a lot of commits appear with Fixes Txxx (our commits from the Phabricator upstream) and incorrectly close thousands of tasks -- e.g., when I wrote Fixes T123 in some commit I meant our T123 -- https://secure.phabricator.com/T123 -- but if phabricator/ is imported into another install that install will have a different local T123 -- https://phabricator.some-company.com/T123.
You could display "Yes" or "Yes, But Not Until The Import Finishes", or hide the whole table.