recaptime.dev / phorge
@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Projects: improve quality of destroy workflow

Summary:
When you permanently destroy a specific project from the command line, the following
additional good things now happen:

- all direct milestones are removed too (since they cannot live alone, and they cannot be just "moved" to the parent project since their resulting sequence would have no sense)
- closing T15913: direct milestones are not orphan, broken, invisible anymore, do not become junk in the database anymore, do not cause project query overhead anymore
- all children projects emerge like cute bubbles by one level
- closing T15918: children projects are not orphan, broken, invisible anymore
- the parent project is eventually restored as "without sub-projects", so, joinable again
- closing T15697: the parent project has not bugged memberships anymore

This change will only affect future usages of the destroy workflow on a project (or root-project or sub-project), that is, this command line workflow:

./bin/remove destroy PHID-OF-YOUR-PROJECT-TO-BE-DESTROYED

## Limitations

This change has nothing to do with the action "Archive" on projects (btw, archiving still works and it's still very recommended).

This change does not try to improve the destruction of a specific milestone (still not recommended because milestones are in sequence and more discussion is needed to improve this corner case of the destruction of a single very specific milestone, btw it works).

This change does not try to improve the destruction of a specific workboard (btw, a workboard does not really exist, it's just a flag in the project).

## Example initial situation

```
Project A
[projectDepth: 0]
[hasSubprojects: 1]
[members: *automatic*, foo, bar]
> Project B
[projectDepth: 1]
[hasSubprojects: 1]
[members: *automatic*, foo, bar]
> Project C
[hasSubprojects: 0]
[projectDepth: 2]
[members: foo, bar]
> Milestone C.1
[projectDepth: 3]
```

Situation after destroying only project A:

```
Project B
[projectDepth: 0]
[hasSubprojects: 1]
[members: *automatic*, foo, bar]
> Project C
[projectDepth: 1]
[hasSubprojects: 0]
[members: foo, bar]
> Milestone C.1
[projectDepth: 2]
```

Situation after destroying only project B:

```
Project A
[projectDepth: 0]
[hasSubprojects: 1]
[members: *automatic*, foo, bar]
> Project C
[projectDepth: 1]
[hasSubprojects: 0]
[members: foo, bar]
> Milestone C.1
[projectDepth: 2]
```

Situation after destroying only project C:

```
Project A
[projectDepth: 0]
[hasSubprojects: 1]
[members: *automatic*, foo, bar]
> Project B
[projectDepth: 1]
[hasSubprojects: 0]
[members: foo, bar]
```

Situation after destroying only milestone C.1:

```
Project A
[projectDepth: 0]
[hasSubprojects: 1]
[members: *automatic*, foo, bar]
> Project B
[projectDepth: 1]
[hasSubprojects: 1]
[members: *automatic*, foo, bar]
> Project C
[hasSubprojects: 0]
[projectDepth: 2]
[members: foo, bar]
```

Closes T15697
Closes T15913
Closes T15918
Closes T16043

Test Plan:
Run multiple times the destroy workflow under every imaginable case,
to violently destroy a projects tree, leaf by leaf. We suggest to run it
with the trace option. Example:

./bin/remove destroy --trace PHID-PROJ-s0met1ng

To run the above workflow multiple times, you may appreciate a dump:

./bin/storage dump | gzip > dump.sql.gz

So you can easily restore it whenever you want:

zcat dump.sql.gz | ./bin/storage shell

- Delete simple milestone
- create a new simple project "A" with milestone "M" and delete "M"
- deletion still works on milestone "M"
- Delete simple project
- create a new simple project "A" with a workboard and destroy "A":
- deletion still works on project "A"
- workboard is still not accessible
- if there were tasks, they are still there but without tag "A"
- Delete project with milestone (T15913)
- create a new project "A" with a milestone "B" and delete "A"
- deletion still works on project "A"
- milestone "B" is finally nuked from the database
and you do not see it anymore in the phabricator_project table
- Delete a project with sub-projects (T15918)
- create a new project "A" with a sub-project called "B" and delete "A"
- deletion still works on project "A"
- project "B" becomes a root-project and it has depth "0" again
and it's usable, instead of having it borked with "You Shall Not Pass"
- Delete a sub-project (T15697)
- create a new project "A" with a sub-project called "B" and delete "B"
- deletion still works on project "B"
- project "A" is usable again like "B" never existed, so you can
change memberships and all the things, instead of having it borked

If you don't have enough time to test this, just enjoy the new unit tests covering the destroy process (T16043):

arc unit src/applications/project/storage/PhabricatorProject.php

In particular, in the unit test results, check the new green light about this method:

PhabricatorProjectCoreTestCase::testProjectDestroy

Reviewers: O1 Blessed Committers, mainframe98

Reviewed By: O1 Blessed Committers, mainframe98

Subscribers: waldyrious, mainframe98, tobiaswiese, Matthew, Cigaryno

Tags: #projects

Maniphest Tasks: T15913, T15697, T15918, T16043

Differential Revision: https://we.phorge.it/D25772

Valerio Bozzolan 8b21213c a10f322f

+354
+154
src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php
··· 1273 1273 } 1274 1274 } 1275 1275 1276 + /** 1277 + * Test that you can successfully destroy a complex projects tree, 1278 + * leaf by leaf, and the depths and the join policies remain consistent. 1279 + */ 1280 + public function testProjectDestroy() { 1281 + // Create test actors. 1282 + $author = $this->createUser()->save(); 1283 + $mario = $this->createUser()->save(); 1284 + $luigi = $this->createUser()->save(); 1285 + 1286 + // Create a project "A". It will have children later. 1287 + $a = $this->createProject($author)->save(); 1288 + 1289 + // Add "Mario" in the project "A", successfully. 1290 + $this->joinProject($a, $mario); 1291 + 1292 + // Under project "A", create "B". 1293 + // Side-effects: 1294 + // - members of "A" are moved to "B". 1295 + // - members of "A" are not editable anymore. 1296 + $b = $this->createProject($author, $a)->save(); 1297 + 1298 + // Under project "B", create the milestone "M". 1299 + // Note that the members of "B" are also members of "M". 1300 + $m = $this->createProject($author, $b, true)->save(); 1301 + 1302 + // Under project "B", create "C". 1303 + // Side-effects: 1304 + // - members of "B" are automatically moved to "C". 1305 + // - members of "B" are not editable anymore. 1306 + $c = $this->createProject($author, $b)->save(); 1307 + 1308 + // Refresh the perspective of Mario. 1309 + $this->refreshProjectInPlace($a, $mario); 1310 + $this->refreshProjectInPlace($b, $mario); 1311 + $this->refreshProjectInPlace($c, $mario); 1312 + $this->refreshProjectInPlace($m, $mario); 1313 + 1314 + // Check the desired project tree: 1315 + // 1316 + // Project A > Project B > Milestone M 1317 + // Project A > Project B > Project C 1318 + // \ Depth: 0 \ \ 1319 + // \ Depth: 1 \ 1320 + // \ Depth: 2 1321 + // 1322 + // - Mario is an indirect member of "A" 1323 + // - Mario is an indirect member of "B" 1324 + // - Mario is an indirect member of "M" 1325 + // - Mario is a direct member of "C" 1326 + // - Users cannot join "A" directly. 1327 + // - Users cannot join "B" directly. 1328 + // 1329 + $this->assertEqual(0, (int)$a->getProjectDepth()); 1330 + $this->assertEqual(1, (int)$b->getProjectDepth()); 1331 + $this->assertEqual(2, (int)$m->getProjectDepth()); 1332 + $this->assertEqual(2, (int)$c->getProjectDepth()); 1333 + $this->assertTrue($a->isUserMember($mario->getPHID())); 1334 + $this->assertTrue($b->isUserMember($mario->getPHID())); 1335 + $this->assertTrue($c->isUserMember($mario->getPHID())); 1336 + $this->assertTrue($m->isUserMember($mario->getPHID())); 1337 + $this->assertCannotJoinProject($a, $luigi, pht( 1338 + 'Test users cannot join project A, because B exists')); 1339 + $this->assertCannotJoinProject($b, $luigi, pht( 1340 + 'Test users cannot join project B, because C exists')); 1341 + 1342 + // Destroy project "B" and refresh its relatives. 1343 + // Note that its milestone "M" is also nuked automatically. 1344 + $engine = new PhabricatorDestructionEngine(); 1345 + $engine->destroyObject($b); 1346 + $this->refreshProjectInPlace($a, $mario); 1347 + $this->refreshProjectInPlace($c, $mario); 1348 + 1349 + // Check the desired project tree: 1350 + // 1351 + // Project A > Project C 1352 + // \ Depth: 0 \ 1353 + // \ Depth: 1 1354 + // 1355 + // - Mario is an indirect member of "A" 1356 + // - Mario is an direct member of "C" 1357 + // - Users cannot join "A" directly. 1358 + // 1359 + $this->assertEqual(null, $this->refreshProject($b, $mario)); 1360 + $this->assertEqual(null, $this->refreshProject($m, $mario)); 1361 + $this->assertEqual(0, (int)$a->getProjectDepth()); 1362 + $this->assertEqual(1, (int)$c->getProjectDepth()); 1363 + $this->assertTrue($a->isUserMember($mario->getPHID())); 1364 + $this->assertTrue($c->isUserMember($mario->getPHID())); 1365 + $this->assertCannotJoinProject($a, $luigi, pht( 1366 + 'Test users cannot join project A, because C exists')); 1367 + 1368 + // Destroy project "C" and refresh relatives. 1369 + $engine->destroyObject($c); 1370 + $this->refreshProjectInPlace($a, $author); 1371 + 1372 + // Check the desired project tree: 1373 + // 1374 + // Project A 1375 + // \- Depth: 0 1376 + // 1377 + // - Mario is a direct member of "A" 1378 + // - Users can join and leave "A" directly again \o/ 1379 + // 1380 + $this->assertEqual(null, $this->refreshProject($c, $mario)); 1381 + $this->assertEqual(0, (int)$a->getProjectDepth()); 1382 + $this->joinProject($a, $luigi); // No crash 1383 + $this->leaveProject($a, $luigi); // No crash 1384 + 1385 + // Destroy "A". It works and nothing remains. 1386 + $engine->destroyObject($a); 1387 + $this->assertEqual(null, $this->refreshProject($a, $mario)); 1388 + } 1276 1389 1277 1390 private function moveToColumn( 1278 1391 PhabricatorUser $viewer, ··· 1496 1609 $this->assertEqual($expect_phids, $actual_phids, $label); 1497 1610 } 1498 1611 1612 + /** 1613 + * Refresh a project in place. 1614 + * @param PhabricatorProject $project Project that will be refreshed. 1615 + * @param PhabricatorUser $viewer 1616 + * @return void 1617 + */ 1618 + private function refreshProjectInPlace( 1619 + PhabricatorProject &$project, 1620 + PhabricatorUser $viewer): void { 1621 + $project = $this->refreshProject($project, $viewer); 1622 + } 1623 + 1624 + /** 1625 + * Get a project, refreshed. 1626 + * @param PhabricatorProject $project Project. 1627 + * @param PhabricatorUser $viewer 1628 + * @param mixed $need_members If you also need project members. 1629 + * @param mixed $need_watchers If you also need project watchers. 1630 + * @return PhabricatorProject|null Refreshed project. 1631 + */ 1499 1632 private function refreshProject( 1500 1633 PhabricatorProject $project, 1501 1634 PhabricatorUser $viewer, ··· 1609 1742 $user->setRealName(pht('Unit Test User %d', $rand)); 1610 1743 1611 1744 return $user; 1745 + } 1746 + 1747 + /** 1748 + * Assert that the specified project is not joinable by the specified user. 1749 + * 1750 + * @param $project PhabricatorProject Test project 1751 + * @param $user PhabricatorUser Test user 1752 + * @param $assert_message string Test assert message 1753 + */ 1754 + private function assertCannotJoinProject( 1755 + PhabricatorProject $project, 1756 + PhabricatorUser $user, 1757 + string $assert_message) { 1758 + $join_crashed = false; 1759 + try { 1760 + $this->joinProject($project, $user); 1761 + } catch (PhabricatorApplicationTransactionValidationException $e) { 1762 + // You cannot join this project. 1763 + $join_crashed = true; 1764 + } 1765 + $this->assertTrue($join_crashed, $assert_message); 1612 1766 } 1613 1767 1614 1768 private function joinProject(
+83
src/applications/project/storage/PhabricatorProject.php
··· 748 748 $slug->delete(); 749 749 } 750 750 751 + // If I'm a project with milestones, these milestones cannot live 752 + // without me (without their project), so, delete milestones as well. 753 + // FAQ: can we just move milestones to my parent? 754 + // Nope: milestones are numbered, so it would not make sense 755 + // to try to move my milestones to the parent project, 756 + // especially since the parent project may have its own milestones, 757 + // with conflicting numbering. 758 + // To find milestones, do not use PhabricatorProjectQuery, to avoid a 759 + // circular dependency, and to avoid a silent fail, since these 760 + // milestones do not have their parent anymore. 761 + // Micro-optimization: find milestones, only if I support them. 762 + if ($this->supportsMilestones()) { 763 + $milestones = id(new self())->loadAllWhere( 764 + 'parentProjectPHID = %s AND milestoneNumber IS NOT NULL', 765 + $this->getPHID()); 766 + foreach ($milestones as $milestone) { 767 + $milestone->attachParentProject($this); 768 + $engine->destroyObject($milestone); 769 + } 770 + } 771 + 772 + // Refresh the 'depth' of my children, fix gaps in the tree, etc. 773 + $this->onDestroyTouchChildren(); 774 + 775 + // After the tree is fixed, update the field 'hasSubProjects' 776 + // of my parent project. 777 + // In this way, my parent project may become root-project again. 778 + if ($this->getParentProject()) { 779 + id(new PhabricatorProjectsMembershipIndexEngineExtension()) 780 + ->rematerialize($this->getParentProject()); 781 + } 782 + 751 783 $this->saveTransaction(); 784 + } 785 + 786 + /** 787 + * On destroy, refresh my children, and their children recursively, 788 + * to consolidate depth, path key, etc. 789 + * As default, only during the initial call, bubble up my direct children, 790 + * to close the gap in our project tree. 791 + * @param bool $first_call True, only during our initial call. 792 + */ 793 + private function onDestroyTouchChildren(bool $first_call = true): void { 794 + // Micro-optimization: proceed only if we may have at least one child. 795 + if (!$this->supportsSubprojects() && !$this->supportsMilestones()) { 796 + return; 797 + } 798 + 799 + // Find my direct children. 800 + // Do not use 'PhabricatorProjectQuery' to avoid a circular dependency. 801 + $table_project = new self(); 802 + if ($first_call) { 803 + // We must skip my direct milestones since they are under removal. 804 + $children = $table_project->loadAllWhere( 805 + 'parentProjectPHID = %s AND milestoneNumber IS NULL', 806 + $this->getPHID()); 807 + } else { 808 + // We must take all sub-projects and milestones. 809 + $children = $table_project->loadAllWhere( 810 + 'parentProjectPHID = %s', 811 + $this->getPHID()); 812 + } 813 + 814 + // Refresh all children. 815 + foreach ($children as $child) { 816 + 817 + if ($first_call) { 818 + // This is a direct children of the deleted project. 819 + // Bubble up it, to don't stay orphan and broken. 820 + $desired_parent = $this->getParentProject(); 821 + $desired_parent_phid = 822 + $desired_parent ? $desired_parent->getPHID() : null; 823 + $child->attachParentProject($desired_parent); 824 + $child->setParentProjectPHID($desired_parent_phid); 825 + } 826 + 827 + // Refresh 'pathKey' and 'depth'. 828 + $child->setProjectPathKey(null); 829 + 830 + $child->save(); 831 + 832 + // Recursively refresh all its children (if any). 833 + $child->onDestroyTouchChildren(false); 834 + } 752 835 } 753 836 754 837
+117
src/docs/user/userguide/projects.diviner
··· 337 337 338 338 Form customization also provides a powerful tool for making many policy 339 339 management tasks easier (see @{article:User Guide: Customizing Forms}). 340 + 341 + Archiving 342 + ========= 343 + 344 + In Phorge, you can both destroy a project (dangerous) or archive it (very safe). 345 + 346 + Archiving a project (or a milestone, which is a kind of project) 347 + is very much recommended, for example, to archive a project when it stops 348 + being useful, when the project reaches its deadline, or when its investor turns 349 + out to be a scam. You might be surprised how useful it is to know which 350 + colleagues have worked on a certain very old archived project, which fortunately 351 + somebody decided to archive rather than fully destroy. 352 + 353 + The {nav icon=ban,name=Archive} action is visible to all people who 354 + can {nav icon=pencil,name=Edit} a project. As usual in Phorge, 355 + there is a confirmation dialog. 356 + 357 + After you confirm the archival, these things will happen: 358 + 359 + - wherever the tag or its hashtag are mentioned, the corresponding badge is 360 + appropriately de-colorized or struck-through. 361 + - the archived project stops appearing in the active projects list at 362 + [ /project/query/active/ ]( /project/query/active/ ) 363 + - the archived project is de-prioritized from most search results and selectors, 364 + including the top search bar, the tag pickers, etc. 365 + - the archived project is muted, and does not cause "watch" notifications. 366 + - the performer of this action is logged in the recent actions. 367 + 368 + All these consequences are reversible. You can bring a project back 369 + to life anytime using the {nav icon=check,name=Activate project} action. 370 + 371 + After archiving a project, all tagged objects, tagged tasks, etc. will be 372 + intentionally kept as-is. In particular, no special read-only policy is 373 + enforced on these tagged objects. This is in line with the above section 374 + about "Policies In Depth". In fact, an object can have many tags, and 375 + if a specific team group ceases its operations, that does not mean that 376 + others should stop working on the same tasks, etc. 377 + 378 + In case additional hiding of information is needed, you can reduce the 379 + visibility policy of that project. For example, the visibility policy 380 + "No One" makes the project effectively invisible to others. 381 + Mastering the visibility policies helps a lot in making sure your cleanup 382 + requests are managed professionally and in a secure way, while still allowing 383 + future auditing, when needed. 384 + 385 + If you still decide against archiving a project in a recoverable way, 386 + continue to the following scary and dangerous section about 387 + permanent destruction. 388 + 389 + Permanently Destroying 390 + ====================== 391 + 392 + Phorge is designed as a safe collaborative platform that rarely 393 + requires @{article:Permanently Destroying Data}. 394 + 395 + If you have read that article, and if you have done a backup, and if 396 + you have access to the command line, and if you still want to permanently 397 + destroy a project (or a milestone), these will be the consequences: 398 + 399 + - The project is destroyed permanently from the database, forever 400 + (unless you have a good backup and sufficient recovery skills). 401 + - All objects, including tasks, repositories, wiki documents, calendars, 402 + secrets, other projects, etc. to which you have set visibility restrictions 403 + involving that project (example: "visible to project members") 404 + will be broken, and everyone will be locked out of them. 405 + That means these objects will become completely invisible from the web 406 + interface and API results. 407 + You can still recover from these particular policy problems, 408 + by reading the @{article:User Guide: Unlocking Objects}. 409 + - Users that were members or watchers will lose that relationship. 410 + - Tagged items will lose that relationship, including tasks, commits, 411 + wiki documents, calendar events, repositories, etc.; these objects 412 + will simply not be associated anymore with that project tag 413 + (but will remain associated with other tags, of course). 414 + - Comments and texts mentioning the hashtag of that `#project` will no longer 415 + render that project link. 416 + You will still be able to add that hashtag to another project, 417 + to revive these links. 418 + - If the project has a workboard, that workboard will be destroyed as well 419 + (tasks in that workboard will always be kept and will remain associated 420 + with other workboards). 421 + - If the project has direct milestones, these milestones will be destroyed 422 + as well. This has sense because milestones are numbered in a linear 423 + chronological sequence, and this sequence cannot "just" be moved elsewhere. 424 + For example if we have projects {nav A > B > Milestones} and if you destroy 425 + the project "B", we cannot just move these milestones up to "A", since 426 + the parent project "A" may already have its own milestones, 427 + and we would lead to sequence conflicts. 428 + (Recall that milestones are technically projects, so consider reading this 429 + list again to understand what will happen while destroying each milestone.) 430 + - If the project has a parent project, and if that parent will end up with 431 + no other child projects, that parent can be promoted to root-project again. 432 + This means membership of the parent project will be editable again. 433 + - If the project has subprojects, all subprojects and all their descendant 434 + subprojects will climb the tree depth by one level, to fill the gap 435 + you caused. Grandchildren become children, children become parents, 436 + etc. - a real mess for family photos. 437 + - You increase the risk of something completely unexpected happening, 438 + such as the destruction of your entire datacenter by Psyduck Pokemons 439 + out of recursion control. 440 + 441 + To permanently destroy a project, you will need to execute a command like this, 442 + from the root directory of the Phorge repository on your server: 443 + 444 + ``` 445 + ./bin/remove destroy PHID-PROJ-abcdef123456 446 + ``` 447 + 448 + The command needs the "PHID" code of the project. 449 + Every project has a PHID which can be retrieved in multiple ways, 450 + including the {nav icon=cog,name=Manage} menu of that project, or hovering 451 + the cursor on the {nav icon=flag,name=Flag For Later} feature. 452 + 453 + This command requires manual confirmation. Before proceeding, 454 + take the disclaimer seriously and read the previous section again about 455 + archiving projects (safe), instead of permanently destroying them (unsafe), 456 + to hopefully change your mind.