Can you update the hasReviewed() function to return the row?

[teraßyte]

Right now if you check if a user has already reviewed an item you only return a count:

	public function hasReviewed( $member=NULL )
	{
		/* Check cache */
		if( $this->_hasReviewed !== NULL )
		{
			return $this->_hasReviewed;
		}

		$member = $member ?: \IPS\Member::loggedIn();
	
		$reviewClass = static::$reviewClass;
		$idColumn = static::$databaseColumnId;
	
		$this->_hasReviewed	= \IPS\Db::i()->select( 'COUNT(*)', $reviewClass::$databaseTable, array(
				array( $reviewClass::$databasePrefix . $reviewClass::$databaseColumnMap['author'] . '=?', $member->member_id ),
				array( $reviewClass::$databasePrefix . $reviewClass::$databaseColumnMap['item'] . '=?', $this->$idColumn )
		) )->first();
		return $this->_hasReviewed;
	}

 

In one of my applications I need to check if a user has already reviewed an item, and if it did I need to display the review rating. With the current code however I'd be forced to run another query to retrieve the data (I have overloaded it thought), there is really no need to use COUNT(*) there since an user can only make 1 review anyway.

If you select and return the whole row I won't have to overload anymore the default function, you would also need to update the function canReview() as well if you make the change. Here's what I changed my functions to:

	public function hasReviewed( $member=NULL )
	{
		$member = $member ?: \IPS\Member::loggedIn();
		
		/* Check cache */
		if( isset($this->_hasReviewed[ $member->member_id ]) )
		{
			return $this->_hasReviewed[ $member->member_id ];
		}
		
		# Nothing? Cache it then
		$reviewClass = static::$reviewClass;
		$idColumn = static::$databaseColumnId;
		
		try
		{
			$this->_hasReviewed[ $member->member_id ] = \IPS\Db::i()->select( '*', $reviewClass::$databaseTable, array(
				array( $reviewClass::$databasePrefix . $reviewClass::$databaseColumnMap['author'] . '=?', $member->member_id ),
				array( $reviewClass::$databasePrefix . $reviewClass::$databaseColumnMap['item'] . '=?', $this->$idColumn )
			) )->first();
		}
		catch ( \UnderflowException $e )
		{
			$this->_hasReviewed[ $member->member_id ] = array();
		}
		
		return $this->_hasReviewed[ $member->member_id ];
	}
	
	public function canReview( $member=NULL )
	{
		$member = $member ?: \IPS\Member::loggedIn();
		if ( !$member->member_id or $member->restrict_post or ( $this instanceof \IPS\Content\Lockable and $this->locked() and ( !$member->member_id or !static::modPermission( 'reply_to_locked', $member, $this->container() ) ) ) )
		{
			return FALSE;
		}
		
		if ( $this->canCommentReview( 'review', $member ) )
		{
			$memberReview = $this->hasReviewed( $member );
			
			if ( empty($memberReview) )
			{
				return TRUE;
			}
		}
		
		return FALSE;
	}

 

 

There is also a caching bug in the original function above (fixed in mine), but I'll make a bug report about that now. The result returned is always the same after the first call even if you check a different member.

EDIT

Here's the bug report:

 

[bfarber]

The method actually indicates it will return a boolean value, rather than an integer (which is what it is doing now).

I'm a little hesitant blindly making this change (although I agree a project search indicates it's not a problem presently), because we could, for instance, implement the ability to review multiple times (say, for instance, you buy a product multiple times or you buy a green product and a red product). I would stick with your overloaded methods for now, or just use the reviews() method that already exists (passing in a custom where clause).

[teraßyte]

I have gone ahead and made another little change to my function actually. I added a second parameter and still return a boolean on the normal call, when the second parameter is TRUE I pass back the whole array instead. That keeps it compatible with the current code.

 

But yeah, I'll keep using my overloaded function for now then.