Page MenuHomePhabricator

Fix PhabricatorProjectCustomFields to use storage correctly.
ClosedPublic

Authored by bluehawk on Jul 12 2014, 3:15 AM.
Referenced Files
F13098315: D9908.diff
Fri, Apr 26, 5:36 AM
F13096506: D9908.id23783.diff
Thu, Apr 25, 4:56 PM
F13096505: D9908.id23783.diff
Thu, Apr 25, 4:56 PM
Unknown Object (File)
Wed, Apr 24, 11:21 PM
Unknown Object (File)
Tue, Apr 23, 6:50 PM
Unknown Object (File)
Thu, Apr 18, 3:54 PM
Unknown Object (File)
Wed, Apr 17, 6:40 AM
Unknown Object (File)
Wed, Apr 17, 4:49 AM
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
Branch
project-custom-field-fix
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 1662
Build 1663: [Placeholder Plan] Wait for 30 Seconds

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.