[feature/avatars] Refactor avatar's handle_form

Since it was performing two distinct operations, refactor handle_form
to separate functions that prepare and process forms.

PHPBB3-10018

phpBB/includes/avatar/driver.php

@@ -102,7 +102,15 @@ abstract class phpbb_avatar_driver
 	/**
 	* @TODO
 	**/
-	public function handle_form($template, $user_row, &$error, $submitted = false)
+	public function prepare_form($template, $user_row, &$error)
+	{
+		return false;
+	}
+
+	/**
+	* @TODO
+	**/
+	public function process_form($template, $user_row, &$error)
 	{
 		return false;
 	}

phpBB/includes/avatar/driver/local.php

@@ -47,7 +47,65 @@ class phpbb_avatar_driver_local extends phpbb_avatar_driver
 	/**
 	* @inheritdoc
 	*/
-	public function handle_form($template, $user_row, &$error, $submitted = false)
+	public function prepare_form($template, $user_row, &$error)
+	{
+		$avatar_list = $this->get_avatar_list();
+		$category = request_var('av_local_cat', '');
+		
+		$categories = array_keys($avatar_list);
+
+		foreach ($categories as $cat)
+		{
+			if (!empty($avatar_list[$cat]))
+			{
+				$template->assign_block_vars('av_local_cats', array(
+					'NAME' => $cat,
+					'SELECTED' => ($cat == $category),
+				));
+			}
+		}
+
+		if (!empty($avatar_list[$category]))
+		{
+			foreach ($avatar_list[$category] as $img => $data)
+			{
+				$template->assign_block_vars('av_local_imgs', array(
+					'AVATAR_IMAGE'  => $path . '/' . $data['file'],
+					'AVATAR_NAME' => $data['name'],
+					'AVATAR_FILE' => $data['filename'],
+				));
+			}
+		}
+
+		return true;
+	}
+	
+	/**
+	* @inheritdoc
+	*/
+	public function process_form($template, $user_row, &$error)
+	{
+		$avatar_list = $this->get_avatar_list();
+		$category = request_var('av_local_cat', '');
+		
+		$file = request_var('av_local_file', '');
+		if (!isset($avatar_list[$category][urldecode($file)]))
+		{
+			$error[] = 'AVATAR_URL_NOT_FOUND';
+			return false;
+		}
+
+		return array(
+			'user_avatar' => $category . '/' . $file,
+			'user_avatar_width' => $avatar_list[$category][urldecode($file)]['width'],
+			'user_avatar_height' => $avatar_list[$category][urldecode($file)]['height'],
+		);
+	}
+
+	/**
+	* @TODO
+	*/
+	private function get_avatar_list()
 	{
 		$avatar_list = ($this->cache == null) ? false : $this->cache->get('av_local_list');
 
@@ -102,49 +160,6 @@ class phpbb_avatar_driver_local extends phpbb_avatar_driver
 			}
 		}
 
-		$category = request_var('av_local_cat', '');
+		return $avatar_list;
-		
-		if ($submitted) {
-			$file = request_var('av_local_file', '');
-			if (!isset($avatar_list[$category][urldecode($file)]))
-			{
-				$error[] = 'AVATAR_URL_NOT_FOUND';
-				return false;
-			}
-
-			return array(
-				'user_avatar' => $category . '/' . $file,
-				'user_avatar_width' => $avatar_list[$category][urldecode($file)]['width'],
-				'user_avatar_height' => $avatar_list[$category][urldecode($file)]['height'],
-			);
-		}
-
-
-		$categories = array_keys($avatar_list);
-
-		foreach ($categories as $cat)
-		{
-			if (!empty($avatar_list[$cat]))
-			{
-				$template->assign_block_vars('av_local_cats', array(
-					'NAME' => $cat,
-					'SELECTED' => ($cat == $category),
-				));
-			}
-		}
-
-		if (!empty($avatar_list[$category]))
-		{
-			foreach ($avatar_list[$category] as $img => $data)
-			{
-				$template->assign_block_vars('av_local_imgs', array(
-					'AVATAR_IMAGE'  => $path . '/' . $data['file'],
-					'AVATAR_NAME' => $data['name'],
-					'AVATAR_FILE' => $data['filename'],
-				));
-			}
-		}
-
-		return true;
 	}
 }

phpBB/includes/avatar/driver/remote.php

@@ -47,9 +47,21 @@ class phpbb_avatar_driver_remote extends phpbb_avatar_driver
 	/**
 	* @inheritdoc
 	*/
-	public function handle_form($template, $user_row, &$error, $submitted = false)
+	public function prepare_form($template, $user_row, &$error)
 	{
-		if ($submitted)
+		$template->assign_vars(array(
+			'AV_REMOTE_WIDTH' => (($user_row['user_avatar_type'] == AVATAR_REMOTE || $user_row['user_avatar_type'] == 'remote') && $user_row['user_avatar_width']) ? $user_row['user_avatar_width'] : request_var('av_local_width', 0),
+			'AV_REMOTE_HEIGHT' => (($user_row['user_avatar_type'] == AVATAR_REMOTE || $user_row['user_avatar_type'] == 'remote') && $user_row['user_avatar_height']) ? $user_row['user_avatar_height'] : request_var('av_local_width', 0),
+			'AV_REMOTE_URL' => (($user_row['user_avatar_type'] == AVATAR_REMOTE || $user_row['user_avatar_type'] == 'remote') && $user_row['user_avatar']) ? $user_row['user_avatar'] : '',
+		));
+
+		return true;
+	}
+
+	/**
+	* @inheritdoc
+	*/
+	public function process_form($template, $user_row, &$error)
 	{
 		$url = request_var('av_remote_url', '');
 		$width = request_var('av_remote_width', 0);
@@ -140,22 +152,10 @@ class phpbb_avatar_driver_remote extends phpbb_avatar_driver
 			}
 		}
 
-			$result = array(
+		return array(
 			'user_avatar' => $url,
 			'user_avatar_width' => $width,
 			'user_avatar_height' => $height,
 		);
-
-			return $result;
-		}
-		else
-		{
-			$template->assign_vars(array(
-				'AV_REMOTE_WIDTH' => (($user_row['user_avatar_type'] == AVATAR_REMOTE || $user_row['user_avatar_type'] == 'remote') && $user_row['user_avatar_width']) ? $user_row['user_avatar_width'] : request_var('av_local_width', 0),
-				'AV_REMOTE_HEIGHT' => (($user_row['user_avatar_type'] == AVATAR_REMOTE || $user_row['user_avatar_type'] == 'remote') && $user_row['user_avatar_height']) ? $user_row['user_avatar_height'] : request_var('av_local_width', 0),
-				'AV_REMOTE_URL' => (($user_row['user_avatar_type'] == AVATAR_REMOTE || $user_row['user_avatar_type'] == 'remote') && $user_row['user_avatar']) ? $user_row['user_avatar'] : '',
-			));
-			return true;
-		}
 	}
 }

phpBB/includes/avatar/driver/upload.php

@@ -47,17 +47,31 @@ class phpbb_avatar_driver_upload extends phpbb_avatar_driver
 	/**
 	* @inheritdoc
 	*/
-	public function handle_form($template, $user_row, &$error, $submitted = false)
+	public function prepare_form($template, $user_row, &$error)
 	{
-		$can_upload = (file_exists($this->phpbb_root_path . $this->config['avatar_path']) && phpbb_is_writable($this->phpbb_root_path . $this->config['avatar_path']) && (@ini_get('file_uploads') || strtolower(@ini_get('file_uploads')) == 'on')) ? true : false;
+		if (!$this->can_upload())
-
-		if ($can_upload == false)
 		{
 			return false;
 		}
 
-		if ($submitted)
+		$template->assign_vars(array(
+			'S_UPLOAD_AVATAR_URL' => ($this->config['allow_avatar_remote_upload']) ? true : false,
+			'AV_UPLOAD_SIZE' => $this->config['avatar_filesize'],
+		));
+		
+		return true;
+	}
+
+	/**
+	* @inheritdoc
+	*/
+	public function process_form($template, $user_row, &$error)
 	{
+		if (!$this->can_upload())
+		{
+			return false;
+		}
+
 		include_once($this->phpbb_root_path . 'includes/functions_upload.' . $this->phpEx);
 
 		$upload = new fileupload('AVATAR_', array('jpg', 'jpeg', 'gif', 'png'), $this->config['avatar_filesize'], $this->config['avatar_min_width'], $this->config['avatar_min_height'], $this->config['avatar_max_width'], $this->config['avatar_max_height'], (isset($this->config['mime_triggers']) ? explode('|', $this->config['mime_triggers']) : false));
@@ -107,11 +121,11 @@ class phpbb_avatar_driver_upload extends phpbb_avatar_driver
 		);
 	}
 
-		$template->assign_vars(array(
+	/**
-			'S_UPLOAD_AVATAR_URL' => ($this->config['allow_avatar_remote_upload']) ? true : false,
+	* @TODO
-			'AV_UPLOAD_SIZE' => $this->config['avatar_filesize'],
+	*/
-		));
+	private function can_upload()
-		
+	{
-		return true;
+		return (file_exists($this->phpbb_root_path . $this->config['avatar_path']) && phpbb_is_writable($this->phpbb_root_path . $this->config['avatar_path']) && (@ini_get('file_uploads') || strtolower(@ini_get('file_uploads')) == 'on'));
 	}
 }

phpBB/includes/ucp/ucp_profile.php

@@ -568,7 +568,7 @@ class ucp_profile
 							{
 								if (check_form_key('ucp_avatar'))
 								{
-									$result = $avatar->handle_form($template, $user->data, $error, true);
+									$result = $avatar->process_form($template, $user->data, $error);
 								
 									if ($result && empty($error))
 									{
@@ -592,7 +592,7 @@ class ucp_profile
 								}
 							}
 
-							if ($avatar->handle_form($template, $user->data, $error)) {
+							if ($avatar->prepare_form($template, $user->data, $error)) {
 								$driver_u = strtoupper($driver);
 
 								$template->assign_block_vars('avatar_drivers', array(