Joomla! Issue Tracker | Joomla! CMS #24506 - [4.0] Code cleanup: remove unnecessary onContentAfterSave in PlgFieldsRepeatable

? ?

Fixed in Code Base
18 May 2019
Medium
Build: 4.0-dev
# 24506
Diff
nhusung:patch-1

Success

Success Hound No violations found. Woof! Details
Success continuous-integration/travis-ci/pr The Travis CI build passed Details
Success continuous-integration/appveyor/pr AppVeyor build succeeded Details
Success continuous-integration/drone/pr the build was successful Details

User tests: Successful: Unsuccessful:

nhusung
5 Apr 2019

Summary of Changes

Prior to this pull request PlgFieldsRepeatable::onContentAfterSave was responsible for converting the field values to JSON. However PlgSystemFields::onContentAfterSave (see below) does the same task. This pull request removes the unnecessary processing.

joomla-cms/plugins/system/fields/fields.php

Lines 66 to 148 in 27371ee

    
           	/** 
        
           	 * The save event. 
        
           	 * 
        
           	 * @param   string   $context  The context 
        
           	 * @param   JTable   $item     The table 
        
           	 * @param   boolean  $isNew    Is new item 
        
           	 * @param   array    $data     The validated data 
        
           	 * 
        
           	 * @return  boolean 
        
           	 * 
        
           	 * @since   3.7.0 
        
           	 */ 
        
           	public function onContentAfterSave($context, $item, $isNew, $data = array()) 
        
           	{ 
        
           		// Check if data is an array and the item has an id 
        
           		if (!is_array($data) || empty($item->id) || empty($data['com_fields'])) 
        
           		{ 
        
           			return true; 
        
           		} 
        
           		// Create correct context for category 
        
           		if ($context === 'com_categories.category') 
        
           		{ 
        
           			$context = $item->extension . '.categories'; 
        
           			// Set the catid on the category to get only the fields which belong to this category 
        
           			$item->catid = $item->id; 
        
           		} 
        
           		// Check the context 
        
           		$parts = FieldsHelper::extract($context, $item); 
        
           		if (!$parts) 
        
           		{ 
        
           			return true; 
        
           		} 
        
           		// Compile the right context for the fields 
        
           		$context = $parts[0] . '.' . $parts[1]; 
        
           		// Loading the fields 
        
           		$fields = FieldsHelper::getFields($context, $item); 
        
           		if (!$fields) 
        
           		{ 
        
           			return true; 
        
           		} 
        
           		// Loading the model 
        
           		$model = new \Joomla\Component\Fields\Administrator\Model\FieldModel(array('ignore_request' => true)); 
        
           		// Loop over the fields 
        
           		foreach ($fields as $field) 
        
           		{ 
        
           			// Determine the value if it is (un)available from the data 
        
           			if (key_exists($field->name, $data['com_fields'])) 
        
           			{ 
        
           				$value = $data['com_fields'][$field->name] === false ? null : $data['com_fields'][$field->name]; 
        
           			} 
        
           			// Field not available on form, use stored value 
        
           			else 
        
           			{ 
        
           				$value = $field->rawvalue; 
        
           			} 
        
           			// If no value set (empty) remove value from database 
        
           			if (is_array($value) ? !count($value) : !strlen($value)) 
        
           			{ 
        
           				$value = null; 
        
           			} 
        
           			// JSON encode value for complex fields 
        
           			if (is_array($value) && (count($value, COUNT_NORMAL) !== count($value, COUNT_RECURSIVE) || !count(array_filter(array_keys($value), 'is_numeric')))) 
        
           			{ 
        
           				$value = json_encode($value); 
        
           			} 
        
           			// Setting the value for the field and the item 
        
           			$model->setFieldValue($field->id, $item->id, $value); 
        
           		} 
        
           		return true; 
        
           	}

Testing Instructions

Create a custom field of type repeatable and save data to it. After the saving process the entered data should still be there.

Expected result

Same as before this commit

d7987bd 5 Apr 2019

Code cleanup: remove onContentAfterSave

nhusung - open - 5 Apr 2019

nhusung - change - 5 Apr 2019

Status

New

⇒

Pending

joomla-cms-bot - change - 5 Apr 2019

Category

⇒

Front End Plugins

nhusung - change - 5 Apr 2019

Title

…

Code cleanup: remove unnecessary onContentAfterSave in PlgFieldsRepeatable

[4.0] Code cleanup: remove unnecessary onContentAfterSave in PlgFieldsRepeatable

nhusung - edited - 5 Apr 2019

stutteringp0et - comment - 21 Apr 2019

Isn't the repeatable field type deprecated? I thought everything deprecated was being removed in J4.

https://docs.joomla.org/Repeatable_form_field_type

nhusung - comment - 21 Apr 2019

There is an issue concerning the plugin‘s name. It is true that the repeatable form field is deprecated but this is only the repeatable com_fields plugin, which internally uses the subform form field. I already considered renaming this plugin, however the only use case I could imagine is to have repeatable fields. Anyway, this plugin was first introduced in J4.0 and backported to J3.9 afterwards.

euismod2336 - comment - 4 May 2019

I have tested this item ✅ successfully on d7987bd

After merging latest dev branch into this (to actually be able to add fields) data is stored in the repeatable field after the removal of the code.

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/24506.}

euismod2336 - comment - 4 May 2019

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/24506.}

euismod2336 - test_item - 4 May 2019 - Tested successfully

sanderpotjer - comment - 4 May 2019

I have tested this item ✅ successfully on d7987bd

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/24506.}

sanderpotjer - test_item - 4 May 2019 - Tested successfully

Quy - change - 4 May 2019

Status

Pending

⇒

Ready to Commit

Quy - comment - 4 May 2019

RTC

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/24506.}

8af47a7 13 May 2019

Merge branch '4.0-dev' into patch-1

nhusung - change - 13 May 2019

Labels

Added: ?

wilsonge - change - 18 May 2019

Labels

Removed: J4 Issue

3e87def 18 May 2019

Missing new line

wilsonge - change - 18 May 2019

Status	Ready to Commit	⇒	Fixed in Code Base
Closed_Date	0000-00-00 00:00:00	⇒	2019-05-18 01:58:08
Closed_By		⇒	wilsonge

wilsonge - close - 18 May 2019

wilsonge - merge - 18 May 2019

wilsonge - comment - 18 May 2019

Thanks!

Add a Comment

Older
Newer

Joomla! Issue Tracker - CMS

[#24506] - [4.0] Code cleanup: remove unnecessary onContentAfterSave in PlgFieldsRepeatable

Summary of Changes

Testing Instructions

Expected result

Add a Comment

	/**
	* The save event.
	*
	* @param string $context The context
	* @param JTable $item The table
	* @param boolean $isNew Is new item
	* @param array $data The validated data
	*
	* @return boolean
	*
	* @since 3.7.0
	*/
	public function onContentAfterSave($context, $item, $isNew, $data = array())
	{
	// Check if data is an array and the item has an id
	if (!is_array($data) \|\| empty($item->id) \|\| empty($data['com_fields']))
	{
	return true;
	}

	// Create correct context for category
	if ($context === 'com_categories.category')
	{
	$context = $item->extension . '.categories';

	// Set the catid on the category to get only the fields which belong to this category
	$item->catid = $item->id;
	}

	// Check the context
	$parts = FieldsHelper::extract($context, $item);

	if (!$parts)
	{
	return true;
	}

	// Compile the right context for the fields
	$context = $parts[0] . '.' . $parts[1];

	// Loading the fields
	$fields = FieldsHelper::getFields($context, $item);

	if (!$fields)
	{
	return true;
	}

	// Loading the model
	$model = new \Joomla\Component\Fields\Administrator\Model\FieldModel(array('ignore_request' => true));

	// Loop over the fields
	foreach ($fields as $field)
	{
	// Determine the value if it is (un)available from the data
	if (key_exists($field->name, $data['com_fields']))
	{
	$value = $data['com_fields'][$field->name] === false ? null : $data['com_fields'][$field->name];
	}
	// Field not available on form, use stored value
	else
	{
	$value = $field->rawvalue;
	}

	// If no value set (empty) remove value from database
	if (is_array($value) ? !count($value) : !strlen($value))
	{
	$value = null;
	}

	// JSON encode value for complex fields
	if (is_array($value) && (count($value, COUNT_NORMAL) !== count($value, COUNT_RECURSIVE) \|\| !count(array_filter(array_keys($value), 'is_numeric'))))
	{
	$value = json_encode($value);
	}

	// Setting the value for the field and the item
	$model->setFieldValue($field->id, $item->id, $value);
	}

	return true;
	}