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.

Sign up for GitHub

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

Jump to bottom

Custom Fields toggle display on read only rights #20068

Merged
merged 10 commits into from
Apr 7, 2018
Merged

Custom Fields toggle display on read only rights #20068

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
b2c4d4e
[com_fields] Normalise the request com_fields data (#19884)
laoneo Mar 25, 2018
8c87352
Custom fields view on form via toggle on read-only rights
Ruud68 Apr 3, 2018
78b8d0f
fix back-end new article
Ruud68 Apr 4, 2018
f474bee
first / seperate check on read-only access
Ruud68 Apr 4, 2018
2738876
refactor code so show_on parameter is part of helper function
Ruud68 Apr 4, 2018
602cc08
implement inherit value in fields + language things
Ruud68 Apr 4, 2018
b0156d2
loadmodel only when needed
Ruud68 Apr 4, 2018
70fb494
changed function comment
Ruud68 Apr 4, 2018
b983c4f
change values order so default value (inherit) is displayed first
Ruud68 Apr 4, 2018
41df5e9
Must use self:: for local static member reference
Ruud68 Apr 4, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions administrator/components/com_fields/helpers/fields.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -542,6 +542,52 @@ public static function canEditFieldValue($field)
return JFactory::getUser()->authorise('core.edit.value', $parts[0] . '.field.' . (int) $field->id);
}

/**
* Return a boolean based on field and field group display read-only setting
*
* @param stdClass $field The field
*
* @return boolean
*
* @since __DEPLOY_VERSION__
*/
public static function displayFieldOnForm($field)
{
$app = JFactory::getApplication();

// Detect if the field should be shown at all
if ($field->params->get('show_on') == 1 && $app->isClient('administrator'))
{
return false;
}
elseif ($field->params->get('show_on') == 2 && $app->isClient('site'))
{
return false;
}

if (!FieldsHelper::canEditFieldValue($field))
Copy link
Contributor

Choose a reason for hiding this comment

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

please change FieldsHelper::canEditFieldValue to self::canEditFieldValue

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! Thanks Robert :)

{
$groupModel = JModelLegacy::getInstance('Group', 'FieldsModel', array('ignore_request' => true));
Copy link
Member

Choose a reason for hiding this comment

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

Small performance improvement, load the model in the if below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, in next PR

$groupDisplayReadOnly = $groupModel->getItem($field->group_id)->params->get('display_readonly', '1');
$fieldDisplayReadOnly = $field->params->get('display_readonly', '2');

if ($fieldDisplayReadOnly == '2')
{
// Inherit from field group display read-only setting
$fieldDisplayReadOnly = $groupDisplayReadOnly;
}

if ($fieldDisplayReadOnly == '0')
{
// Do not display field on form when field is read-only
return false;
}
}

// Display field on form
return true;
}

/**
* Adds Count Items for Category Manager.
*
Expand Down
10 changes: 2 additions & 8 deletions administrator/components/com_fields/libraries/fieldsplugin.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,14 +152,8 @@ public function onCustomFieldsPrepareDom($field, DOMElement $parent, JForm $form
return null;
}

$app = JFactory::getApplication();

// Detect if the field should be shown at all
if ($field->params->get('show_on') == 1 && $app->isClient('administrator'))
{
return;
}
elseif ($field->params->get('show_on') == 2 && $app->isClient('site'))
// Detect if the field is configured to be displayed on the form
if (!FieldsHelper::displayFieldOnForm($field))
{
return null;
}
Expand Down
13 changes: 13 additions & 0 deletions administrator/components/com_fields/models/forms/field.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,19 @@
<option value="3">COM_FIELDS_FIELD_DISPLAY_AFTER_DISPLAY</option>
<option value="0">COM_FIELDS_FIELD_DISPLAY_NO_DISPLAY</option>
</field>

<field
name="display_readonly"
type="radio"
label="JFIELD_DISPLAY_READONLY_LABEL"
description="JFIELD_DISPLAY_READONLY_DESC"
class="btn-group btn-group-yesno"
default="2"
>
<option value="1">JYES</option>
<option value="0">JNO</option>
<option value="2">JGLOBAL_INHERIT</option>
</field>
</fieldset>
</fields>
</form>
20 changes: 18 additions & 2 deletions administrator/components/com_fields/models/forms/group.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,8 @@
filter="user_utc"
/>

<field
name="modified_by"
<field
name="modified_by"
type="user"
label="JGLOBAL_FIELD_MODIFIED_BY_LABEL"
class="readonly"
Expand Down Expand Up @@ -150,4 +150,20 @@
class="inputbox"
/>
</fieldset>

<fields name="params" label="COM_FIELDS_FIELD_BASIC_LABEL">
<fieldset name="basic">
<field
name="display_readonly"
type="radio"
label="JFIELD_DISPLAY_READONLY_LABEL"
description="JFIELD_DISPLAY_READONLY_DESC"
class="btn-group btn-group-yesno"
default="1"
>
<option value="1">JYES</option>
<option value="0">JNO</option>
</field>
</fieldset>
</fields>
</form>
12 changes: 12 additions & 0 deletions administrator/components/com_fields/models/group.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
*/
defined('_JEXEC') or die;

use Joomla\Registry\Registry;

/**
* Group Model
*
Expand Down Expand Up @@ -69,6 +71,11 @@ public function save($data)
*/
public function getTable($name = 'Group', $prefix = 'FieldsTable', $options = array())
{
if (strpos(JPATH_COMPONENT, 'com_fields') === false)
{
$this->addTablePath(JPATH_ADMINISTRATOR . '/components/com_fields/tables');
}

return JTable::getInstance($name, $prefix, $options);
}

Expand Down Expand Up @@ -314,6 +321,11 @@ public function getItem($pk = null)
$item->context = $this->getState('filter.context');
}

if (property_exists($item, 'params'))
{
$item->params = new Registry($item->params);
Copy link
Member

Choose a reason for hiding this comment

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

I'm a bit hesitant to approve the pr, because this here is a minor BC break. Are we sure that it breaks nothing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @laoneo , have you tested this? Do you have any scenario's in which this would break?
Hi have tested extensively, but that is of course no guarantee as I only test on my own sites / combinations with 3rd party extensions.
Who in your opinion could / should have a go / say / test in this?

Copy link
Member

Choose a reason for hiding this comment

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

In core I guess it should be fine. But if an extension is using the params of a group, loaded through the model it has now a different behavior. But this chance is relative small and the pr got merged, so I guess the discussion is superfluous.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but there was no params of a group, I created that in this PR. So the only thing that could break is if an extension developer created its own params for a group and 'forgot' to share that back to Joomla. And then it would only break if the implementation is different. If the developer followed joomla guidelines, then it should not break. But then again... maybe I am a bit naive :)

Copy link
Member

Choose a reason for hiding this comment

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

What I'm talking about is the following code $group->params returns before the pr a string and after a Registry. Which can be counted as a minir BC break.

}

// Convert the created and modified dates to local user time for display in the form.
$tz = new DateTimeZone(JFactory::getApplication()->get('offset'));

Expand Down
28 changes: 28 additions & 0 deletions administrator/components/com_fields/models/groups.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
*/
defined('_JEXEC') or die;

use Joomla\Registry\Registry;
use Joomla\Utilities\ArrayHelper;

/**
Expand Down Expand Up @@ -213,4 +214,31 @@ protected function getListQuery()

return $query;
}

/**
* Gets an array of objects from the results of database query.
*
* @param string $query The query.
* @param integer $limitstart Offset.
* @param integer $limit The number of records.
*
* @return array An array of results.
*
* @since __DEPLOY_VERSION__
* @throws RuntimeException
*/
protected function _getList($query, $limitstart = 0, $limit = 0)
{
$result = parent::_getList($query, $limitstart, $limit);

if (is_array($result))
{
foreach ($result as $group)
{
$group->params = new Registry($group->params);
Copy link
Member

Choose a reason for hiding this comment

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

See above.

}
}

return $result;
}
}
2 changes: 2 additions & 0 deletions administrator/language/en-GB/en-GB.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,8 @@ JFIELD_BASIS_LOGOUT_DESCRIPTION_LABEL="Logout Description Text"
JFIELD_BASIS_LOGOUT_DESCRIPTION_SHOW_DESC="Show or hide logout description."
JFIELD_BASIS_LOGOUT_DESCRIPTION_SHOW_LABEL="Logout Description"
JFIELD_CATEGORY_DESC="The category that this item is assigned to. You may select an existing category or enter a new category by typing the name in the field and pressing enter."
JFIELD_DISPLAY_READONLY_DESC="Whether to display the field on forms when read-only. Inherit defaults to value set in field group."
JFIELD_DISPLAY_READONLY_LABEL="Display When Read-Only"
JFIELD_ENABLED_DESC="The enabled status of this item."
JFIELD_FIELDS_CATEGORY_DESC="Select the category that this field is assigned to."
JFIELD_KEY_REFERENCE_DESC="Used to store information referring to an external resource."
Expand Down