[ticket/15289] Update acp storage

PHPBB3-15289

phpBB/adm/style/acp_storage.html

@@ -16,7 +16,7 @@
 				<dd>
 					<select id="{{ storage.get_name }}" name="{{ storage.get_name }}[provider]" data-togglable-settings="true">
 						{% for provider in PROVIDERS if provider.is_available %}
-							<option value="{{ provider.get_class }}"{{ attribute(config, 'storage\\' ~ storage.get_name ~ '\\provider') ==  provider.get_class ? ' selected' : '' }} data-toggle-setting="#{{ storage.get_name }}_{{ provider.get_name }}_settings">
+							<option value="{{ get_class(provider) }}"{{ attribute(config, 'storage\\' ~ storage.get_name ~ '\\provider') ==  get_class(provider) ? ' selected' : '' }} data-toggle-setting="#{{ storage.get_name }}_{{ provider.get_name }}_settings">
 								{{ lang('STORAGE_ADAPTER_' ~ provider.get_name | upper ~ '_NAME') }}
 							</option>
 						{% endfor %}

phpBB/includes/acp/acp_storage.php

@@ -85,7 +85,6 @@ class acp_storage
 		}
 	}
 
-	// TODO: Validate data
 	public function overview($id, $mode)
 	{
 		$form_name = 'acp_storage';
@@ -99,23 +98,29 @@ class acp_storage
 
 		if ($this->request->is_set_post('submit'))
 		{
+			$modified_storages = [];
+			$messages = [];
+
 			foreach ($this->storage_collection as $storage)
 			{
+				$storage_name = $storage->get_name();
+
+				$options = $this->get_provider_options($this->get_current_provider($storage_name));
+
 				$modified = false;
-				$provider = $this->provider_collection->get_by_class($this->config['storage\\' . $storage->get_name() . '\\provider']);
 
 				// Check if provider have been modified
-				if ($this->request->variable([$storage->get_name(), 'provider'], '') != $this->config['storage\\' . $storage->get_name() . '\\provider'])
+				if ($this->get_new_provider($storage_name) != $this->get_current_provider($storage_name))
 				{
 					$modified = true;
 				}
 
 				// Check if options have been modified
-				if(!$modified)
+				if (!$modified)
 				{
-					foreach($provider->get_options() as $option => $params)
+					foreach (array_keys($options) as $def)
 					{
-						if ($this->request->variable([$storage->get_name(), $option], '') != $this->config['storage\\' . $storage->get_name() . '\\provider'])
+						if ($this->get_new_def($storage_name, $def) != $this->get_current_def($storage_name, $def))
 						{
 							$modified = true;
 							break;
@@ -123,33 +128,33 @@ class acp_storage
 					}
 				}
 
-				// Update storage
-				if($modified)
+				// If the storage have been modified, validate options
+				if ($modified)
 				{
-					// TODO: Allow to move data to the new storage automatically
-
-					// TODO: Validate data
-
-					// Remove old straoge config
-					foreach (array_keys($provider->get_options()) as $def)
-					{
-						$this->config->delete('storage\\' . $storage->get_name() . '\\config\\' . $def);
-					}
-
-					// Update provider
-					$this->config->set('storage\\' . $storage->get_name() . '\\provider', $this->request->variable([$storage->get_name(), 'provider'], ''));
-
-					// Set new storage config
-					$new_provider = $this->provider_collection->get_by_class($this->config['storage\\' . $storage->get_name() . '\\provider']);
-
-					foreach (array_keys($new_provider->get_options()) as $def)
-					{
-						$this->config->set('storage\\' . $storage->get_name() . '\\config\\' . $def, $this->request->variable([$storage->get_name(), $def], ''));
-					}
+					$modified_storages[] = $storage_name;
+					$this->validate_data($storage_name, $messages);
 				}
 			}
 
-			// Updated succesfuly
+			if (count($modified_storages))
+			{
+				if (!count($messages))
+				{
+					foreach ($modified_storages as $storage_name)
+					{
+						$this->update_storage_config($storage_name);
+					}
+
+					trigger_error($this->lang->lang('STORAGE_UPDATE_SUCCESSFUL') . adm_back_link($this->u_action), E_USER_NOTICE);
+				}
+				else
+				{
+					trigger_error(implode('<br />', $messages) . adm_back_link($this->u_action), E_USER_WARNING);
+				}
+			}
+
+			// If there is no errors
+			trigger_error($this->lang->lang('STORAGE_NO_CHANGES') . adm_back_link($this->u_action), E_USER_WARNING);
 		}
 
 		$this->template->assign_vars(array(
@@ -157,4 +162,106 @@ class acp_storage
 			'PROVIDERS' => $this->provider_collection,
 		));
 	}
+
+	protected function get_current_provider($storage_name)
+	{
+		return $this->config['storage\\' . $storage_name . '\\provider'];
+	}
+
+	protected function get_new_provider($storage_name)
+	{
+		return $this->request->variable([$storage_name, 'provider'], '');
+	}
+
+	protected function get_provider_options($provider)
+	{
+		return $this->provider_collection->get_by_class($provider)->get_options();
+	}
+
+	protected function get_current_def($storage_name, $def)
+	{
+		return $this->config['storage\\' . $storage_name . '\\config\\' . $def];
+	}
+
+	protected function get_new_def($storage_name, $def)
+	{
+		return $this->request->variable([$storage_name, $def], '');
+	}
+
+	protected function validate_data($storage_name, &$messages)
+	{
+		$storage_title = $this->lang->lang('STORAGE_' . strtoupper($storage_name) . '_TITLE');
+
+		// Check if provider exists
+		try
+		{
+			$new_provider = $this->provider_collection->get_by_class($this->get_new_provider($storage_name));
+		}
+		catch (\Exception $e)
+		{
+			$messages[] = $this->lang->lang('STORAGE_PROVIDER_NOT_EXISTS', $storage_title);
+			return;
+		}
+
+		// Check if provider is available
+		if (!$new_provider->is_available())
+		{
+			$messages[] = $this->lang->lang('STORAGE_PROVIDER_NOT_AVAILABLE', $storage_title);
+			return;
+		}
+
+		// Check options
+		$new_options = $this->get_provider_options($this->get_new_provider($storage_name));
+
+		foreach($new_options as $def_k => $def_v)
+		{
+			$value = $this->get_new_def($storage_name, $def_k);
+
+			switch ($def_v['type'])
+			{
+				case 'email':
+					if(!filter_var($value, FILTER_VALIDATE_EMAIL))
+					{
+						$messages[] = $this->lang->lang('STORAGE_FORM_TYPE_EMAIL_INCORRECT_FORMAT');
+					}
+				case 'text':
+				case 'password':
+					$maxlength = isset($def_v['maxlength']) ? $def_v['maxlength'] : 255;
+					if(strlen($value) > $maxlength)
+					{
+						$messages[] = $this->lang->lang('STORAGE_FORM_TYPE_TEXT_TOO_LONG');
+					}
+					break;
+				case 'radio':
+				case 'select':
+					if (!in_array($value, array_values($def_v['options'])))
+					{
+						$messages[] = $this->lang->lang('STORAGE_FORM_SELECT_NOT_AVAILABLE');
+					}
+					break;
+			}
+		}
+	}
+
+	protected function update_storage_config($storage_name)
+	{
+		$current_options = $this->get_provider_options($this->get_current_provider($storage_name));
+
+		// Remove old storage config
+		foreach (array_keys($current_options) as $def)
+		{
+			$this->config->delete('storage\\' . $storage_name . '\\config\\' . $def);
+		}
+
+		// Update provider
+		$this->config->set('storage\\' . $storage_name . '\\provider', $this->get_new_provider($storage_name));
+
+		// Set new storage config
+		$new_options = $this->get_provider_options($this->get_new_provider($storage_name));
+
+		foreach (array_keys($new_options) as $def)
+		{
+			$this->config->set('storage\\' . $storage_name . '\\config\\' . $def, $this->get_new_def($storage_name, $def));
+		}
+	}
 }

phpBB/language/en/acp/storage.php

@@ -37,17 +37,29 @@ if (empty($lang) || !is_array($lang))
 // in a url you again do not need to specify an order e.g., 'Click %sHERE%s' is fine
 
 $lang = array_merge($lang, array(
+
+	// Template
 	'STORAGE_TITLE'					=> 'Storage Settings',
 	'STORAGE_TITLE_EXPLAIN'			=> 'Here you can change the storage.',
 	'STORAGE_SELECT'				=> 'Select storage',
 	'STORAGE_SELECT_DESC'			=> 'Select an storage from the list.',
 
+	// Storage ma,es
 	'STORAGE_ATTACHMENT_TITLE'		=> 'Attachments storage',
 	'STORAGE_AVATAR_TITLE'			=> 'Avatars storage',
 	'STORAGE_BACKUP_TITLE'			=> 'Backup storage',
 
+	// Local adapter
+	'STORAGE_ADAPTER_LOCAL_NAME'			=> 'Local',
 	'STORAGE_ADAPTER_LOCAL_OPTION_PATH'		=> 'Path',
 
-	'STORAGE_ADAPTER_LOCAL_NAME'	=> 'Local',
-
+	// Form validation
+	// Todo" more descriptive form errors showing storage name and field name
+	'STORAGE_UPDATE_SUCCESSFUL' 				=>	'All storages were successfuly updated.',
+	'STORAGE_NO_CHANGES'						=>	'No changes has been made.',
+	'STORAGE_PROVIDER_NOT_EXISTS'				=>	'Provider selected for %s dont exist.',
+	'STORAGE_PROVIDER_NOT_AVAILABLE'			=>	'Provider selected for %s is not available.',
+	'STORAGE_FORM_TYPE_EMAIL_INCORRECT_FORMAT'	=>	'Incorrect email',
+	'STORAGE_FORM_TYPE_TEXT_TOO_LONG'			=>	'Text is too long',
+	'STORAGE_FORM_TYPE_SELECT_NOT_AVAILABLE'	=>	'Selected value is not available',
 ));

phpBB/phpbb/template/twig/extension.php

@@ -86,6 +86,7 @@ class extension extends \Twig_Extension
 		return array(
 			new \Twig_SimpleFunction('lang', array($this, 'lang')),
 			new \Twig_SimpleFunction('lang_defined', array($this, 'lang_defined')),
+			new \Twig_SimpleFunction('get_class', array($this, 'get_class')),
 		);
 	}
 
@@ -185,12 +186,24 @@ class extension extends \Twig_Extension
 	}
 
 	/**
-	* Check if a language variable exists
-	*
-	* @return bool
-	*/
+	 * Check if a language variable exists
+	 *
+	 * @return bool
+	 */
 	public function lang_defined($key)
 	{
 		return call_user_func_array([$this->language, 'is_set'], [$key]);
 	}
+
+	/*
+	 * Returns the name of the class of an object
+	 *
+	 * @param object            $object         The object
+	 *
+	 * @return string
+	 */
+	public function get_class($object)
+	{
+		return get_class($object);
+	}
 }