Page MenuHomePhabricator

Fix PhabricatorProjectCustomFields to use storage correctly.
ClosedPublic

Authored by bluehawk on Jul 12 2014, 3:15 AM.
Referenced Files
F14096861: D9908.diff
Tue, Nov 26, 5:59 AM
Unknown Object (File)
Sat, Nov 23, 12:38 AM
Unknown Object (File)
Mon, Nov 18, 6:27 PM
Unknown Object (File)
Mon, Nov 18, 6:09 PM
Unknown Object (File)
Mon, Nov 18, 6:09 PM
Unknown Object (File)
Thu, Nov 14, 9:57 PM
Unknown Object (File)
Sun, Nov 10, 9:41 PM
Unknown Object (File)
Wed, Nov 6, 4:51 PM
Subscribers

Details

Summary

Prevents infinite recursion when trying to save custom fields on projects.

Test Plan

Add a custom field (that is a class, not one configured in the UI) to a project, and try to save it.

Diff Detail

Repository
rP Phabricator
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

bluehawk retitled this revision from to Fix PhabricatorProjectCustomFields to use storage correctly. .
bluehawk updated this object.
bluehawk edited the test plan for this revision. (Show Details)
bluehawk added a reviewer: epriestley.
bluehawk set the repository for this revision to rP Phabricator.
bluehawk added a project: Phabricator.
epriestley edited edge metadata.
This revision is now accepted and ready to land.Jul 12 2014, 3:17 AM

This isn't quite consistent with how other applications work, but probably makes more sense than the approach taken elsewhere.

epriestley updated this revision to Diff 23784.

Closed by commit rP17badfacac31 (authored by @bluehawk, committed by @epriestley).

How do other applications do it? I copied this over from ManiphestCustomField because those were working for me.

Differential impelments these on a subclass (DifferentialStoredCustomField), as does People (PhabricatorUserConfiguredCustomField). However, I think this approach is generally cleaner. The implementation on PhabricatorProjectStandardCustomField is redundant now and could be removed. The implementations in applications other than Maniphest could be lifted into the application's custom field base class.