Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CRM-21777 - Set readonly for the fields initialised in civicrm_settings.php #11680

Merged
merged 1 commit into from
Mar 12, 2018

Conversation

jitendrapurohit
Copy link
Contributor

@jitendrapurohit jitendrapurohit commented Feb 16, 2018

Overview

Load input fields as non-editable which have been initialized in civicrm.settings.php.

Before

Directory Preference OR Resource URL settings initialized in civi settings file are shown as editable in civicrm/admin/setting/path?reset=1 OR civicrm/admin/setting/url?reset=1 form. If a user tries to edit and update the path, it is not saved as value which is already set in civicrm.settings.php gets the preference. Check JIRA for more details.

After

Fields initialized in civicrm.settings.php are loaded as readonly as it is of no use to load them as editable. Also, a warning is displayed which lets the user know that some fields are already set (overridden) in settings file.

image


@jitendrapurohit jitendrapurohit changed the title Set readonly for the fields initialised in civicrm_settings.php CRM-21777 - Set readonly for the fields initialised in civicrm_settings.php Feb 16, 2018
@@ -95,6 +95,8 @@ public function buildQuickForm() {

$descriptions = array();
$settingMetaData = $this->getSettingsMetaData();
//Load readonly for the input fields whose values are overridden in civicrm_settings.php.
$overriddenSettings = CRM_Utils_Array::value('civicrm_setting', $GLOBALS);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i would remove this and in 110 use something like Civi::settings()->getMandatory($setting); that should tell you if its overrriden i think iirc

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed and updated. Thanks 👍

@yashodha
Copy link
Contributor

@jitendrapurohit I think this is same as https://issues.civicrm.org/jira/browse/CRM-21495
I will be closing #11351 in favor of this PR

@eileenmcnaughton
Copy link
Contributor

screenshot 2018-03-12 16 59 12

Looks good to me - in my test scenario I had an invalid path & the form was unsubmittable. I think it would be nicer not to validate fields in this scenario - but I acknowledge it's not necessarily something that would happen in the 'real world'

merging

@@ -147,6 +152,9 @@ public function buildQuickForm() {

}
}
if (!empty($setStatus)) {
CRM_Core_Session::setStatus("Some fields are loaded as 'readonly' as they have been set (overridden) in civicrm.settings.php.", '', 'info', array('expires' => 0));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jitendrapurohit This string is missing ts() for translation. Could you do a follow-up PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done - #11802. Thanks @mlutfy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants