Update Item Form Helper with some changes

[teraßyte]

1) As per title, I am using the Item form helper in one of my applications and I setup the option 'maxItems' => 1, I was expecting it to work like the Node helper which returns the single object when you don't allow multiple nodes to be selected:

			if ( !empty( $return ) )
			{
				return $this->options['multiple'] ? $return : array_pop( $return );
			}
			else
			{
				return NULL;
			}

This is the current Item code instead, no matter what it doesn't even check if you have any results:

	/**
	 * Format Value
	 *
	 * @return	array|NULL
	 */
	public function formatValue()
	{
		$itemClass = $this->options['class'];
		$order     = NULL;
		$items     = array();
		$idField = $itemClass::$databaseColumnId;
		if ( ! $this->value )
		{
			$this->value = array();
		}
		else
		{
			$this->value = ( is_string( $this->value ) ) ? explode( ',', $this->value ) : $this->value;
		}

		if ( count( $this->value ) )
		{
			[...]

			$where = array( \IPS\Db::i()->in( $itemClass::$databaseTable . '.' . $itemClass::$databasePrefix . $itemClass::$databaseColumnId, $this->value ) );
			foreach( $itemClass::getItemsWithPermission( array( $where ), $order, NULL, $this->options['permissionCheck'] ) as $item )
			{
				$items[ $item->$idField ] = $item;
			}
		}

		$this->value = $items;

		return $this->value;
	}

 

2) The Node helper returns NULL if there are no results, however Item always return an array (even if empty). That makes the phpDoc wrong too since it lists NULL as a possible result when infact it never does (see the code box just above).

 

3) The PHP backend doesn't seem to check/validate anywhere the maxItems option. It seems to be only checked on the user side with JS, if you tamper with the HTML code you can probably submit more items too (not tested though, I just had a quick look at the code)

 

Here is how I would update the function in order to (1) return the single Item object when the max limit is 1, and (2) return NULL when there are no results:

	/**
	 * Format Value
	 *
	 * @return	\IPS\Content\Item|array|NULL
	 */
	public function formatValue()
	{
		$itemClass = $this->options['class'];
		$order     = NULL;
		$items     = array();
		$idField = $itemClass::$databaseColumnId;
		if ( ! $this->value )
		{
			$this->value = array();
		}
		else
		{
			$this->value = ( is_string( $this->value ) ) ? explode( ',', $this->value ) : $this->value;
		}

		if ( count( $this->value ) )
		{
			if ( is_array( $this->options['orderResults'] ) )
			{
				if ( isset( $itemClass::$databaseColumnMap[ $this->options['orderResults'][0] ] ) )
				{
					$order = $itemClass::$databaseColumnMap[ $this->options['orderResults'][0] ] . ' ' . $this->options['orderResults'][1];
				}
			}

			$where = array( \IPS\Db::i()->in( $itemClass::$databaseTable . '.' . $itemClass::$databasePrefix . $itemClass::$databaseColumnId, $this->value ) );
			foreach( $itemClass::getItemsWithPermission( array( $where ), $order, NULL, $this->options['permissionCheck'] ) as $item )
			{
				$items[ $item->$idField ] = $item;
			}
			
			if ( !empty( $return ) )
			{
				$this->value = ( $this->options['maxItems'] AND $this->options['maxItems'] == 1 ) ? $items : array_pop( $items );
			}
			else
			{
				$this->value = NULL;
			}
		}

		return $this->value;
	}

 

This might break current implementations of the Item helper around that already work expecting always an array though. But if you can at least consider this change for 4.2 it would be nice to see it included.

[bfarber]

Your suggestions make sense - I've raised internally for further discussion. As you've pointed out, we'd need to be careful so-as not to cause issues for existing calls to the method.