Setting form - start to sync preferences with Setting, using trait #12731
Conversation
|
(Standard links)
|
|
test this please |
|
@eileenmcnaughton this needs rebase now that the other PR merged. |
eileenmcnaughton
force-pushed
the
setting_form
branch
4 times, most recently
from
74628e9 to
7c7c694
Compare
|
jenkins, test this please |
1 similar comment
|
jenkins, test this please |
eileenmcnaughton
force-pushed
the
setting_form
branch
from
7c7c694 to
a0a72e3
Compare
| catch (CiviCRM_API3_Exception $e) { | ||
| CRM_Core_Session::setStatus($e->getMessage(), ts('Save Failed'), 'error'); | ||
| } | ||
|
|
||
| foreach ($this->_varNames as $groupName => $groupValues) { |
There was a problem hiding this comment.
going forwards we basically migrate settings out of varNames into protected $_settings per below
| 'is_domain' => '1', | ||
| 'is_contact' => 0, | ||
| 'description' => NULL, | ||
| 'description' => ts("Select the tabs that should be displayed when viewing a contact record. EXAMPLE: If your organization does not keep track of 'Relationships', then un-check this option to simplify the screen display. Tabs for Contributions, Pledges, Memberships, Events, Grants and Cases are also hidden if the corresponding component is not enabled. Go to Administer > System Settings > Enable Components to modify the components which are available for your site."), |
There was a problem hiding this comment.
ideally we also use this for the tpl help text rather than duplicating - I haven't looked at that as yet
|
@eileenmcnaughton what forms should I try when testing this? |
|
@colemanw so far literally only one setting is converted - the top one on display preferences
But ideally you would tinker with converting a few more to test the methodology a bit - @mattwire put up a PR with a lot more in it https://github.com/civicrm/civicrm-core/pull/12714/files#diff-cd9367d6c33a74944159c2f7623db4a7R69 An end goal would be to start using code like SettingsForm.tpl in Display.tpl - replacing the hard coded settings with metadata |
|
Re the tpl - see this commit mattwire@73dad36 - although I'm making the case for aligning settings & preferences forms as part of this change |
|
@eileenmcnaughton When I test this there seems to be something wrong with the checkbox save/load logic. Pledges and mailings are always unchecked and if I uncheck grants cases gets unchecked as well. |
|
@eileenmcnaughton see last comment from @mattwire |
eileenmcnaughton
force-pushed
the
setting_form
branch
from
a0a72e3 to
aca7b7f
Compare
|
@mattwire just figured it out - the setDefaults was the issue - can you recheck? |
|
@eileenmcnaughton Now working fine. Can you fix the two style warnings reported by jenkins and then we can merge. |
eileenmcnaughton
force-pushed
the
setting_form
branch
from
aca7b7f to
c1f7302
Compare
eileenmcnaughton
force-pushed
the
setting_form
branch
from
c1f7302 to
6821aa1
Compare
|
unrelated fails |
Overview
Adds a trait class & starts to align Settings form with preferences form
Before
The settings form & the preferences form do the same thing but evolved separately. Settings uses metadata, preferences doesn't.
After
This adds a trait that can be shared & the parts of both that use metadata can use the shared section
Technical Details
Over time we should remove both forms in favour of a new clean one based on the trait. I think the trait would be merged into that form.
Note that one issue TBC is how to define 'serialize' on the form. In the DAO we use a string but the generator converts it to a constant. I used just a constant here.
I can break this down into some smaller commits.
Comments
@mattwire I think part of #12714 is resolving the form interaction. I took a look on the first setting.