Page MenuHomePhabricator

Prevent crashes when more than one buildable for an object exists
ClosedPublic

Authored by hach-que on Nov 9 2013, 4:32 AM.
Tags
None
Referenced Files
F15384558: D7543.id17028.diff
Fri, Mar 14, 8:15 PM
F15383548: D7543.id.diff
Fri, Mar 14, 5:07 PM
F15331975: D7543.id17020.diff
Fri, Mar 7, 3:40 PM
Unknown Object (File)
Feb 14 2025, 8:39 AM
Unknown Object (File)
Feb 8 2025, 11:32 AM
Unknown Object (File)
Feb 7 2025, 11:45 PM
Unknown Object (File)
Feb 4 2025, 10:25 PM
Unknown Object (File)
Jan 30 2025, 11:46 PM

Details

Summary

This prevents a crash in applying build plans when more than one buildable exists for the same object. It also adds a check into the "New Manual Build" page to ensure that users can't create a buildable for an object that already has one.

Test Plan

Tried to create a buildable for an object that already has one and a nice friendly error appeared. Applied a build plan to a buildable whose object has two buildables and didn't get a crash any more.

Diff Detail

Branch
fix-crash
Lint
Lint Errors
SeverityLocationCodeMessage
Errorsrc/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php:233PHL1Unknown Symbol
Unit
No Test Coverage

Event Timeline

It also adds a check into the "New Manual Build" page to ensure that users can't create a buildable for an object that already has one.

At least tentatively, I think this should be allowed. If you're testing build plans, it lets you basically create a copy of some buildable and muck around with it without messing up the "real" one. There probably should be a flag to distinguish the "real" one from synthetic copies, though. For example, I wouldn't want a build to go red and send a bunch of alerts out because I messed up when configuring a new plan or whatever.

The existing code looks up the buildable by the object's PHID when applying a plan, so we'd need to change the code to allow a buildable's PHID to be passed through (otherwise it will always pick the first buildable, and not necessarily the one that you applied the plan to).

hach-que updated this revision to Unknown Object (????).Nov 9 2013, 4:46 AM

Remove change to buildable edit controller.

(Updating libphutil/ should clear that lint -- that class is new.)

Closed by commit rP43847d6bd7bb (authored by @hach-que, committed by @epriestley).