admin_controller - Code Metrics - Inspection of "Hide advertisement for specific group" - phpbb-extensions/ad-management - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — master (#16)

by Jakub

created 2017-06-10 19:25 UTC

admin_controller C

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	563
Duplicated Lines	5.15 %

Coupling/Cohesion

Components	1
Dependencies	2

Test Coverage

Coverage

91.1%

Importance

Changes	35
Bugs	0	Features	1

Metric	Value
wmc	62
lcom	1
cbo	2
dl	29
loc	563
ccs	215
cts	236
cp	0.911
rs	5.9493
c	35
b	0
f	1

21 Methods

Rating	Name	Duplication	Size	Complexity
A	__construct()	0	14	1
A	mode_manage()	0	14	2
B	mode_settings()	0	34	5
A	set_page_url()	0	4	1
A	get_page_title()	0	4	1
B	action_add()	13	39	5
B	action_edit()	16	55	7
A	action_enable()	0	4	1
A	action_disable()	0	4	1
B	action_delete()	0	40	5
C	list_ads()	0	27	7
A	setup()	0	6	1
B	ad_enable()	0	28	6
C	get_form_data()	0	48	7
A	assign_form_data()	0	13	2
A	prepare_end_date()	0	13	3
A	assign_locations()	0	12	3
A	ad_preview()	0	4	1
A	success()	0	4	1
A	error()	0	4	1
A	log()	0	4	1

How to fix Duplicated Code Complexity

Duplicated Code

Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.

Common duplication problems, and corresponding solutions are:

If you have the same expression in different places: Extract expression to a method
If you have the same method in different sub-classes: Extract method, and pull up field to the parent class
If you have the same code in unrelated classes: Consider extracting the code to a new class, and injecting that class

Complex Class

Tip: Before tackling complexity, make sure that you eliminate any duplication first. This often can reduce the size of classes significantly.

Complex classes like admin_controller often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.

Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.

While breaking up the class, it is a good idea to analyze how other classes use admin_controller, and based on these observations, apply Extract Interface, too.

<?php
/**
 *
 * Advertisement management. An extension for the phpBB Forum Software package.
 *
 * @copyright (c) 2017 phpBB Limited <https://www.phpbb.com>
 * @license GNU General Public License, version 2 (GPL-2.0)
 *
 */

namespace phpbb\admanagement\controller;

/**
* Admin controller
*/
class admin_controller
{
	const MAX_NAME_LENGTH = 255;
	const DATE_FORMAT = 'Y-m-d';

	/** @var \phpbb\template\template */
	protected $template;

	/** @var \phpbb\user */
	protected $user;

	/** @var \phpbb\request\request */
	protected $request;

	/** @var \phpbb\admanagement\ad\manager */
	protected $manager;

	/** @var \phpbb\admanagement\location\manager */
	protected $location_manager;

	/** @var \phpbb\log\log */
	protected $log;

	/** @var \phpbb\config\config */
	protected $config;

	/** @var \phpbb\db\driver\driver_interface */
	protected $db;

	/** @var string php_ext */
	protected $php_ext;

	/** @var string ext_path */
	protected $ext_path;

	/** @var string groups_table */
	protected $groups_table;

	/** @var string Custom form action */
	protected $u_action;

	/** @var array Form validation errors */
	protected $errors = array();

	/**
	* Constructor
	*
	* @param \phpbb\template\template				$template			Template object
	* @param \phpbb\user							$user				User object
	* @param \phpbb\request\request					$request			Request object
	* @param \phpbb\admanagement\ad\manager			$manager			Advertisement manager object
	* @param \phpbb\admanagement\location\manager	$location_manager	Template location manager object
	* @param \phpbb\log\log							$log				The phpBB log system
	* @param \phpbb\config\config					$config				Config object
	* @param \phpbb\db\driver\driver_interface		$db					Database object
	* @param string									$php_ext			PHP extension
	* @param string									$ext_path			Path to this extension
	* @param string									$groups_table		Groups table
	*/
	public function __construct(\phpbb\template\template $template, \phpbb\user $user, \phpbb\request\request $request, \phpbb\admanagement\ad\manager $manager, \phpbb\admanagement\location\manager $location_manager, \phpbb\log\log $log, \phpbb\config\config $config, \phpbb\db\driver\driver_interface $db, $php_ext, $ext_path, $groups_table)
	{
		$this->template = $template;
		$this->user = $user;
		$this->request = $request;
		$this->manager = $manager;
		$this->location_manager = $location_manager;
		$this->log = $log;
		$this->config = $config;
		$this->db = $db;
		$this->php_ext = $php_ext;
		$this->ext_path = $ext_path;
		$this->groups_table = $groups_table;
	}

	/**
	* Process user request for manage mode
	*
	* @return void
	*/
	public function mode_manage()
	{
		$this->setup();

		// Trigger specific action
		$action = $this->request->variable('action', '');
		if (in_array($action, array('add', 'edit', 'enable', 'disable', 'delete')))
		{
			$this->{'action_' . $action}();
		}

		// Otherwise default to this
		$this->list_ads();
	}

	/**
	* Process user request for settings mode
	*
	* @return void
	*/
	public function mode_settings()
	{
		$this->setup();
		$this->user->add_lang('common');

		add_form_key('phpbb/admanagement/settings');
		if ($this->request->is_set_post('submit'))
		{
			// Validate form key
			if (!check_form_key('phpbb/admanagement/settings'))
			{
				$errors[] = $this->user->lang('FORM_INVALID');
foreach ($collection as $item) {
    $myArray['foo'] = $item->getFoo();

    if ($item->hasBar()) {
        $myArray['bar'] = $item->getBar();
    }

    // do something with $myArray
}
			}

			$this->config->set('phpbb_admanagement_hide_groups', implode(',', $this->request->variable('hide_groups', array(0))));

			$this->success('ACP_AD_SETTINGS_SAVED');
		}

		$hide_groups = explode(',', $this->config['phpbb_admanagement_hide_groups']);
		$sql = 'SELECT group_id, group_name
			FROM ' . $this->groups_table;
		$result = $this->db->sql_query($sql);
		while ($row = $this->db->sql_fetchrow($result))
		{
			$group_name = (!empty($this->user->lang['G_' . $row['group_name']])) ? $this->user->lang('G_' . $row['group_name']) : $row['group_name'];

			$this->template->assign_block_vars('groups', array(
				'ID'			=> $row['group_id'],
				'NAME'			=> $group_name,
				'S_SELECTED'	=> in_array($row['group_id'], $hide_groups),
			));
		}
	}

	/**
	* Set page url
	*
	* @param string $u_action Custom form action
	* @return void
	*/
	public function set_page_url($u_action)
	{
		$this->u_action = $u_action;
	}

	/**
	* Get ACP page title for Ads module
	*
	* @return string	Language string for Ads ACP module
	*/
	public function get_page_title()
	{
		return $this->user->lang('ACP_ADMANAGEMENT_TITLE');
	}

	/**
	* Add an advertisement
	*
	* @return void
	*/
	public function action_add()
	{
		$preview = $this->request->is_set_post('preview');
		$submit = $this->request->is_set_post('submit');

		add_form_key('phpbb/admanagement/add');
		if ($preview || $submit)
		{
			$data = $this->get_form_data('phpbb/admanagement/add');

			if ($preview)

			{
				$this->ad_preview($data['ad_code']);
			}
			else if (empty($this->errors))
			{
				$ad_id = $this->manager->insert_ad($data);
				$this->manager->insert_ad_locations($ad_id, $data['ad_locations']);

				$this->log('ADD', $data['ad_name']);

				$this->success('ACP_AD_ADD_SUCCESS');
			}

			$this->assign_locations($data);
			$this->assign_form_data($data);
		}
		else
		{
			$this->assign_locations();
		}

		// Set output vars for display in the template
		$this->template->assign_vars(array(
			'S_ADD_AD'				=> true,
			'U_BACK'				=> $this->u_action,
			'PICKER_DATE_FORMAT'	=> self::DATE_FORMAT,
		));
	}

	/**
	* Edit an advertisement
	*
	* @return void
	*/
	public function action_edit()
	{
		$ad_id = $this->request->variable('id', 0);
		$preview = $this->request->is_set_post('preview');
		$submit = $this->request->is_set_post('submit');

		add_form_key('phpbb/admanagement/edit/' . $ad_id);
		if ($preview || $submit)
		{
			$data = $this->get_form_data('phpbb/admanagement/edit/' . $ad_id);

			if ($preview)
			{
				$this->ad_preview($data['ad_code']);
			}
			else if (empty($this->errors))

			{
				$success = $this->manager->update_ad($ad_id, $data);

				if ($success)
				{
					// Only insert new ad locations to DB when ad exists
					$this->manager->delete_ad_locations($ad_id);
					$this->manager->insert_ad_locations($ad_id, $data['ad_locations']);

					$this->log('EDIT', $data['ad_name']);

					$this->success('ACP_AD_EDIT_SUCCESS');
				}
				$this->error('ACP_AD_DOES_NOT_EXIST');
			}
		}
		else
		{
			// Load ad data
			$data = $this->manager->get_ad($ad_id);
			if (empty($data))
			{
				$this->error('ACP_AD_DOES_NOT_EXIST');
			}

			// Load ad template locations
			$data['ad_locations'] = $this->manager->get_ad_locations($ad_id);
		}

		// Set output vars for display in the template
		$this->template->assign_vars(array(
			'S_EDIT_AD'				=> true,
			'EDIT_ID'				=> $ad_id,
			'U_BACK'				=> $this->u_action,
			'PICKER_DATE_FORMAT'	=> self::DATE_FORMAT,
		));
		$this->assign_locations($data);
		$this->assign_form_data($data);
	}

	/**
	* Enable an advertisement
	*
	* @return void
	*/
	public function action_enable()
	{
		$this->ad_enable(true);
	}

	/**
	* Disable an advertisement
	*
	* @return void
	*/
	public function action_disable()
	{
		$this->ad_enable(false);
	}

	/**
	* Delete an advertisement
	*
	* @return void
	*/
	public function action_delete()
	{
		$ad_id = $this->request->variable('id', 0);
		if ($ad_id)
		{
			if (confirm_box(true))
			{
				// Get ad data so that we can log ad name
				$ad_data = $this->manager->get_ad($ad_id);

				// Delete ad and it's template locations
				$this->manager->delete_ad_locations($ad_id);
				$success = $this->manager->delete_ad($ad_id);

				// Only notify user on error or if not ajax
				if (!$success)
				{
					$this->error('ACP_AD_DELETE_ERRORED');
				}
				else
				{
					$this->log('DELETE', $ad_data['ad_name']);

					if (!$this->request->is_ajax())
					{
						$this->success('ACP_AD_DELETE_SUCCESS');
					}
				}
			}
			else
			{
				confirm_box(false, $this->user->lang('CONFIRM_OPERATION'), build_hidden_fields(array(
					'id'		=> $ad_id,
					'i'			=> $this->request->variable('i', ''),
					'mode'		=> $this->request->variable('mode', ''),
					'action'	=> 'delete'
				)));
			}
		}
	}

	/**
	* Display the ads
	*
	* @return void
	*/
	public function list_ads()
	{
		foreach ($this->manager->get_all_ads() as $row)
		{
			$ad_enabled = (int) $row['ad_enabled'];
			$ad_end_date = (int) $row['ad_end_date'];
			$ad_expired = $ad_end_date > 0 && $ad_end_date < time();
			if ($ad_expired && $ad_enabled)
			{
				$ad_enabled = 0;
				$this->manager->update_ad($row['ad_id'], array('ad_enabled' => 0));
			}

			$this->template->assign_block_vars('ads', array(
				'NAME'					=> $row['ad_name'],
				'END_DATE'				=> $ad_end_date ? $this->user->format_date($ad_end_date, self::DATE_FORMAT) : '',
				'S_END_DATE_EXPIRED'	=> $ad_expired,
				'S_ENABLED'				=> $ad_enabled,
				'U_ENABLE'				=> $this->u_action . '&amp;action=' . ($ad_enabled ? 'disable' : 'enable') . '&amp;id=' . $row['ad_id'],
				'U_EDIT'				=> $this->u_action . '&amp;action=edit&amp;id=' . $row['ad_id'],
				'U_DELETE'				=> $this->u_action . '&amp;action=delete&amp;id=' . $row['ad_id'],
			));
		}

		// Set output vars for display in the template
		$this->template->assign_var('U_ACTION_ADD', $this->u_action . '&amp;action=add');
	}

	/**
	* Perform general tasks
	*
	* @return void
	*/
	protected function setup()
	{
		$this->user->add_lang_ext('phpbb/admanagement', 'acp');

		$this->template->assign_var('S_PHPBB_ADMANAGEMENT', true);
	}

	/**
	* Enable/disable an advertisement
	*
	* @param	bool	$enable	Enable or disable the advertisement?
	* @return void
	*/
	protected function ad_enable($enable)
	{
		$ad_id = $this->request->variable('id', 0);

		$success = $this->manager->update_ad($ad_id, array(
			'ad_enabled'	=> (int) $enable,
		));

		// If AJAX was used, show user a result message
		if ($this->request->is_ajax())
		{
			$json_response = new \phpbb\json_response;
			$json_response->send(array(
				'text'	=> $this->user->lang($enable ? 'ENABLED' : 'DISABLED'),
				'title'	=> $this->user->lang('AD_ENABLE_TITLE', (int) $enable),
			));
		}

		// Otherwise, show traditional infobox
		if ($success)
		{
			$this->success($enable ? 'ACP_AD_ENABLE_SUCCESS' : 'ACP_AD_DISABLE_SUCCESS');
		}
		else
		{
			$this->error($enable ? 'ACP_AD_ENABLE_ERRORED' : 'ACP_AD_DISABLE_ERRORED');
		}
	}

	/**
	* Get admin form data.
	*
	* @param	string	$form_name	The form name.
	* @return	array	Form data
	*/
	protected function get_form_data($form_name)
	{
		$data = array(
			'ad_name'		=> $this->request->variable('ad_name', '', true),
			'ad_note'		=> $this->request->variable('ad_note', '', true),
			'ad_code'		=> $this->request->variable('ad_code', '', true),
			'ad_enabled'	=> $this->request->variable('ad_enabled', 0),
			'ad_locations'	=> $this->request->variable('ad_locations', array('')),
			'ad_end_date'	=> $this->request->variable('ad_end_date', ''),
		);

		// Validate form key
		if (!check_form_key($form_name))
		{
			$this->errors[] = $this->user->lang('FORM_INVALID');
		}

		// Validate ad name
		if ($data['ad_name'] === '')
		{
			$this->errors[] = $this->user->lang('AD_NAME_REQUIRED');
		}
		if (truncate_string($data['ad_name'], self::MAX_NAME_LENGTH) !== $data['ad_name'])
		{
			$this->errors[] = $this->user->lang('AD_NAME_TOO_LONG', self::MAX_NAME_LENGTH);
		}

		// Validate ad end date
		if (preg_match('#^\d{4}\-\d{2}\-\d{2}$#', $data['ad_end_date']))
		{
			$data['ad_end_date'] = (int) $this->user->get_timestamp_from_format(self::DATE_FORMAT, $data['ad_end_date']);

			if ($data['ad_end_date'] < time())
			{
				$this->errors[] = $this->user->lang('AD_END_DATE_INVALID');
			}
		}
		else if ($data['ad_end_date'] !== '')
		{
			$this->errors[] = $this->user->lang('AD_END_DATE_INVALID');
		}
		else
		{
			$data['ad_end_date'] = 0;
		}

		return $data;
	}

	/**
	* Assign form data to the template.
	*
	* @param	array	$data	The form data.
	* @return void
	*/
	protected function assign_form_data($data)
	{
		$this->template->assign_vars(array(
			'S_ERROR'		=> (bool) count($this->errors),
			'ERROR_MSG'		=> count($this->errors) ? implode('<br />', $this->errors) : '',

			'AD_NAME'		=> $data['ad_name'],
			'AD_NOTE'		=> $data['ad_note'],
			'AD_CODE'		=> $data['ad_code'],
			'AD_ENABLED'	=> $data['ad_enabled'],
			'AD_END_DATE'	=> $this->prepare_end_date($data['ad_end_date']),
		));
	}
	/**
	* Prepare end date for display
	*
	* @param	mixed	$end_date	End date.
	* @return	string	End date prepared for display.
	*/
	protected function prepare_end_date($end_date)
	{
		if (empty($end_date))
		{
			return '';
		}
		else if (is_numeric($end_date))
		{
			return $this->user->format_date($end_date, self::DATE_FORMAT);
		}

		return $end_date;
class Author {
    private $name;

    public function __construct($name) {
        $this->name = $name;
    }

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}
	}

	/**
	* Assign template locations data to the template.
	*
	* @param	mixed	$data	The form data or nothing.
	* @return	void
	*/
	protected function assign_locations($data = false)
	{
		foreach ($this->location_manager->get_all_locations() as $location_id => $location_data)
		{
			$this->template->assign_block_vars('ad_locations', array(
				'LOCATION_ID'	=> $location_id,
				'LOCATION_DESC'	=> $location_data['desc'],
				'LOCATION_NAME'	=> $location_data['name'],
				'S_SELECTED'	=> $data ? in_array($location_id, $data['ad_locations']) : false,
			));
		}
	}

	/**
	* Prepare advertisement preview
	*
	* @param	string	$code	Ad code to preview
	* @return	void
	*/
	protected function ad_preview($code)
	{
		$this->template->assign_var('PREVIEW', htmlspecialchars_decode($code));
	}

	/**
	* Print success message.
	*
	* It takes arguments in the form of a language key, followed by language substitution values.
	*/
	protected function success()
	{
		trigger_error(call_user_func_array(array($this->user, 'lang'), func_get_args()) . adm_back_link($this->u_action));
	}

	/**
	* Print error message.
	*
	* It takes arguments in the form of a language key, followed by language substitution values.
	*/
	protected function error()
	{
		trigger_error(call_user_func_array(array($this->user, 'lang'), func_get_args()) . adm_back_link($this->u_action), E_USER_WARNING);
	}

	/**
	* Log action
	*
	* @param	string	$action		Performed action in uppercase
	* @param	string	$ad_name	Advertisement name
	* @return	void
	*/
	protected function log($action, $ad_name)
	{
		$this->log->add('admin', $this->user->data['user_id'], $this->user->ip, 'ACP_ADMANAGEMENT_' . $action . '_LOG', time(), array($ad_name));
	}
}


1			<?php
2			/**
3			*
4			* Advertisement management. An extension for the phpBB Forum Software package.
5			*
6			* @copyright (c) 2017 phpBB Limited <https://www.phpbb.com>
7			* @license GNU General Public License, version 2 (GPL-2.0)
8			*
9			*/
10
11			namespace phpbb\admanagement\controller;
12
13			/**
14			* Admin controller
15			*/
16			class admin_controller
17			{
18			const MAX_NAME_LENGTH = 255;
19			const DATE_FORMAT = 'Y-m-d';
20
21			/** @var \phpbb\template\template */
22			protected $template;
23
24			/** @var \phpbb\user */
25			protected $user;
26
27			/** @var \phpbb\request\request */
28			protected $request;
29
30			/** @var \phpbb\admanagement\ad\manager */
31			protected $manager;
32
33			/** @var \phpbb\admanagement\location\manager */
34			protected $location_manager;
35
36			/** @var \phpbb\log\log */
37			protected $log;
38
39			/** @var \phpbb\config\config */
40			protected $config;
41
42			/** @var \phpbb\db\driver\driver_interface */
43			protected $db;
44
45			/** @var string php_ext */
46			protected $php_ext;
47
48			/** @var string ext_path */
49			protected $ext_path;
50
51			/** @var string groups_table */
52			protected $groups_table;
53
54			/** @var string Custom form action */
55			protected $u_action;
56
57			/** @var array Form validation errors */
58			protected $errors = array();
59
60			/**
61			* Constructor
62			*
63	31		* @param \phpbb\template\template $template Template object
64			* @param \phpbb\user $user User object
65	31		* @param \phpbb\request\request $request Request object
66	31		* @param \phpbb\admanagement\ad\manager $manager Advertisement manager object
67	31		* @param \phpbb\admanagement\location\manager $location_manager Template location manager object
68	31		* @param \phpbb\log\log $log The phpBB log system
69	31		* @param \phpbb\config\config $config Config object
70	31		* @param \phpbb\db\driver\driver_interface $db Database object
71	31		* @param string $php_ext PHP extension
72	31		* @param string $ext_path Path to this extension
73	31		* @param string $groups_table Groups table
74			*/
75			public function __construct(\phpbb\template\template $template, \phpbb\user $user, \phpbb\request\request $request, \phpbb\admanagement\ad\manager $manager, \phpbb\admanagement\location\manager $location_manager, \phpbb\log\log $log, \phpbb\config\config $config, \phpbb\db\driver\driver_interface $db, $php_ext, $ext_path, $groups_table)
76			{
77			$this->template = $template;
78			$this->user = $user;
79			$this->request = $request;
80	6		$this->manager = $manager;
81			$this->location_manager = $location_manager;
82	6		$this->log = $log;
83			$this->config = $config;
84	6		$this->db = $db;
85			$this->php_ext = $php_ext;
86			$this->ext_path = $ext_path;
87	6		$this->groups_table = $groups_table;
88	6		}
89	6
90	5		/**
91	5		* Process user request for manage mode
92			*
93			* @return void
94	6		*/
95	6		public function mode_manage()
96			{
97			$this->setup();
98
99			// Trigger specific action
100			$action = $this->request->variable('action', '');
101			if (in_array($action, array('add', 'edit', 'enable', 'disable', 'delete')))
102			{
103	25		$this->{'action_' . $action}();
104			}
105	25
106	25		// Otherwise default to this
107			$this->list_ads();
108			}
109
110			/**
111			* Process user request for settings mode
112			*
113	1		* @return void
114			*/
115	1		public function mode_settings()
116			{
117			$this->setup();
118			$this->user->add_lang('common');
119
120			add_form_key('phpbb/admanagement/settings');
121			if ($this->request->is_set_post('submit'))
122			{
123	7		// Validate form key
124			if (!check_form_key('phpbb/admanagement/settings'))
125	7		{
126	7		$errors[] = $this->user->lang('FORM_INVALID');
			0 ignored issues – show Coding Style Comprehensibility introduced 2017-06-10 19:41 UTC by Report Bug Copy Issue Report `$errors` was never initialized. Although not strictly required by PHP, it is generally a good practice to add `$errors = array();` before regardless. Adding an explicit array definition is generally preferable to implicit array definition as it guarantees a stable state of the code. Let’s take a look at an example: foreach ($collection as $item) { $myArray['foo'] = $item->getFoo(); if ($item->hasBar()) { $myArray['bar'] = $item->getBar(); } // do something with $myArray } As you can see in this example, the array `$myArray` is initialized the first time when the foreach loop is entered. You can also see that the value of the `bar` key is only written conditionally; thus, its value might result from a previous iteration. This might or might not be intended. To make your intention clear, your code more readible and to avoid accidental bugs, we recommend to add an explicit initialization `$myArray = array()` either outside or inside the foreach loop. Loading history...
127			}
128	7
129	7		$this->config->set('phpbb_admanagement_hide_groups', implode(',', $this->request->variable('hide_groups', array(0))));
130	7
131	6		$this->success('ACP_AD_SETTINGS_SAVED');
132			}
133
134	6		$hide_groups = explode(',', $this->config['phpbb_admanagement_hide_groups']);
135	1		$sql = 'SELECT group_id, group_name
136	1		FROM ' . $this->groups_table;
137			$result = $this->db->sql_query($sql);
138	5		while ($row = $this->db->sql_fetchrow($result))
139	1		{
140	1		$group_name = (!empty($this->user->lang['G_' . $row['group_name']])) ? $this->user->lang('G_' . $row['group_name']) : $row['group_name'];
141
142	1		$this->template->assign_block_vars('groups', array(
143			'ID' => $row['group_id'],
144	1		'NAME' => $group_name,
145			'S_SELECTED' => in_array($row['group_id'], $hide_groups),
146			));
147	5		}
148	5		}
149	5
150			/**
151			* Set page url
152	1		*
153			* @param string $u_action Custom form action
154			* @return void
155			*/
156	6		public function set_page_url($u_action)
157	6		{
158	6		$this->u_action = $u_action;
159	6		}
160	6
161	6		/**
162			* Get ACP page title for Ads module
163			*
164			* @return string Language string for Ads ACP module
165			*/
166			public function get_page_title()
167			{
168	10		return $this->user->lang('ACP_ADMANAGEMENT_TITLE');
169			}
170	9
171	9		/**
172	9		* Add an advertisement
173			*
174	9		* @return void
175	9		*/
176	9		public function action_add()
177	7		{
178			$preview = $this->request->is_set_post('preview');
179			$submit = $this->request->is_set_post('submit');
180	7
181	1		add_form_key('phpbb/admanagement/add');
182	1		if ($preview \|\| $submit)
183			{
184	6		$data = $this->get_form_data('phpbb/admanagement/add');
185	2
186		View Code Duplication	if ($preview)
			1 ignored issue – show Duplication introduced 2017-06-01 14:16 UTC by Report Bug Copy Issue Report This code seems to be duplicated across your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
187			{
188	2		$this->ad_preview($data['ad_code']);
189			}
190	1		else if (empty($this->errors))
191	1		{
192			$ad_id = $this->manager->insert_ad($data);
193	1		$this->manager->insert_ad_locations($ad_id, $data['ad_locations']);
194
195	1		$this->log('ADD', $data['ad_name']);
196	10
197	1		$this->success('ACP_AD_ADD_SUCCESS');
198			}
199	5
200			$this->assign_locations($data);
201			$this->assign_form_data($data);
202			}
203	2		else
204	2		{
205	2		$this->assign_locations();
206	1		}
207
208			// Set output vars for display in the template
209			$this->template->assign_vars(array(
210	1		'S_ADD_AD' => true,
211			'U_BACK' => $this->u_action,
212			'PICKER_DATE_FORMAT' => self::DATE_FORMAT,
213			));
214	6		}
215	6
216	6		/**
217	6		* Edit an advertisement
218	6		*
219	6		* @return void
220	6		*/
221	6		public function action_edit()
222	6		{
223			$ad_id = $this->request->variable('id', 0);
224			$preview = $this->request->is_set_post('preview');
225			$submit = $this->request->is_set_post('submit');
226
227			add_form_key('phpbb/admanagement/edit/' . $ad_id);
228			if ($preview \|\| $submit)
229	3		{
230			$data = $this->get_form_data('phpbb/admanagement/edit/' . $ad_id);
231	3
232	1		if ($preview)
233			{
234			$this->ad_preview($data['ad_code']);
235			}
236		View Code Duplication	else if (empty($this->errors))
			1 ignored issue – show Duplication introduced 2017-06-01 14:16 UTC by Report Bug Copy Issue Report This code seems to be duplicated across your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
237			{
238			$success = $this->manager->update_ad($ad_id, $data);
239	3
240			if ($success)
241	3		{
242	1		// Only insert new ad locations to DB when ad exists
243			$this->manager->delete_ad_locations($ad_id);
244			$this->manager->insert_ad_locations($ad_id, $data['ad_locations']);
245
246			$this->log('EDIT', $data['ad_name']);
247
248			$this->success('ACP_AD_EDIT_SUCCESS');
249	3		}
250			$this->error('ACP_AD_DOES_NOT_EXIST');
251	3		}
252			}
253	3		else
254	3		{
255	3		// Load ad data
256			$data = $this->manager->get_ad($ad_id);
257	2		if (empty($data))
258			{
259			$this->error('ACP_AD_DOES_NOT_EXIST');
260	2		}
261	2
262			// Load ad template locations
263			$data['ad_locations'] = $this->manager->get_ad_locations($ad_id);
264	2		}
265	2
266	1		// Set output vars for display in the template
267			$this->template->assign_vars(array(
268			'S_EDIT_AD' => true,
269			'EDIT_ID' => $ad_id,
270	1		'U_BACK' => $this->u_action,
271			'PICKER_DATE_FORMAT' => self::DATE_FORMAT,
272	1		));
273	1		$this->assign_locations($data);
274	1		$this->assign_form_data($data);
275			}
276
277			/**
278			* Enable an advertisement
279			*
280	1		* @return void
281	1		*/
282	1		public function action_enable()
283	1		{
284			$this->ad_enable(true);
285	1		}
286
287	1		/**
288	1		* Disable an advertisement
289			*
290			* @return void
291			*/
292			public function action_disable()
293			{
294			$this->ad_enable(false);
295	1		}
296
297	1		/**
298			* Delete an advertisement
299	1		*
300	1		* @return void
301	1		*/
302	1		public function action_delete()
303	1		{
304	1		$ad_id = $this->request->variable('id', 0);
305	1		if ($ad_id)
306	1		{
307			if (confirm_box(true))
308	1		{
309	1		// Get ad data so that we can log ad name
310	1		$ad_data = $this->manager->get_ad($ad_id);
311	1
312	1		// Delete ad and it's template locations
313	1		$this->manager->delete_ad_locations($ad_id);
314	1		$success = $this->manager->delete_ad($ad_id);
315	1
316	1		// Only notify user on error or if not ajax
317	1		if (!$success)
318			{
319			$this->error('ACP_AD_DELETE_ERRORED');
320	1		}
321	1		else
322			{
323			$this->log('DELETE', $ad_data['ad_name']);
324
325			if (!$this->request->is_ajax())
326			{
327			$this->success('ACP_AD_DELETE_SUCCESS');
328			}
329	4		}
330			}
331	4		else
332			{
333	4		confirm_box(false, $this->user->lang('CONFIRM_OPERATION'), build_hidden_fields(array(
334	4		'id' => $ad_id,
335	4		'i' => $this->request->variable('i', ''),
336			'mode' => $this->request->variable('mode', ''),
337			'action' => 'delete'
338	4		)));
339	4		}
340			}
341			}
342
343			/**
344			* Display the ads
345			*
346			* @return void
347			*/
348			public function list_ads()
349	4		{
350	2		foreach ($this->manager->get_all_ads() as $row)
351			{
352			$ad_enabled = (int) $row['ad_enabled'];
353			$ad_end_date = (int) $row['ad_end_date'];
354	2		$ad_expired = $ad_end_date > 0 && $ad_end_date < time();
355			if ($ad_expired && $ad_enabled)
356			{
357			$ad_enabled = 0;
358			$this->manager->update_ad($row['ad_id'], array('ad_enabled' => 0));
359			}
360
361			$this->template->assign_block_vars('ads', array(
362			'NAME' => $row['ad_name'],
363			'END_DATE' => $ad_end_date ? $this->user->format_date($ad_end_date, self::DATE_FORMAT) : '',
364	13		'S_END_DATE_EXPIRED' => $ad_expired,
365			'S_ENABLED' => $ad_enabled,
366			'U_ENABLE' => $this->u_action . '&action=' . ($ad_enabled ? 'disable' : 'enable') . '&id=' . $row['ad_id'],
367	13		'U_EDIT' => $this->u_action . '&action=edit&id=' . $row['ad_id'],
368	13		'U_DELETE' => $this->u_action . '&action=delete&id=' . $row['ad_id'],
369	13		));
370	13		}
371	13
372	13		// Set output vars for display in the template
373	13		$this->template->assign_var('U_ACTION_ADD', $this->u_action . '&action=add');
374			}
375
376	13		/**
377	13		* Perform general tasks
378	2		*
379	2		* @return void
380			*/
381			protected function setup()
382	13		{
383	13		$this->user->add_lang_ext('phpbb/admanagement', 'acp');
384	2
385	2		$this->template->assign_var('S_PHPBB_ADMANAGEMENT', true);
386	13		}
387	13
388	2		/**
389	2		* Enable/disable an advertisement
390			*
391			* @param bool $enable Enable or disable the advertisement?
392	13		* @return void
393	13		*/
394	4		protected function ad_enable($enable)
395			{
396	4		$ad_id = $this->request->variable('id', 0);
397	4
398	2		$success = $this->manager->update_ad($ad_id, array(
399	2		'ad_enabled' => (int) $enable,
400	4		));
401	9
402	9		// If AJAX was used, show user a result message
403			if ($this->request->is_ajax())
404			{
405			$json_response = new \phpbb\json_response;
406			$json_response->send(array(
407	9		'text' => $this->user->lang($enable ? 'ENABLED' : 'DISABLED'),
408			'title' => $this->user->lang('AD_ENABLE_TITLE', (int) $enable),
409			));
410	13		}
411
412			// Otherwise, show traditional infobox
413			if ($success)
414			{
415			$this->success($enable ? 'ACP_AD_ENABLE_SUCCESS' : 'ACP_AD_DISABLE_SUCCESS');
416			}
417			else
418			{
419	11		$this->error($enable ? 'ACP_AD_ENABLE_ERRORED' : 'ACP_AD_DISABLE_ERRORED');
420			}
421	11		}
422	11
423	11		/**
424			* Get admin form data.
425	11		*
426	11		* @param string $form_name The form name.
427	11		* @return array Form data
428	11		*/
429	11		protected function get_form_data($form_name)
430	11		{
431	11		$data = array(
432			'ad_name' => $this->request->variable('ad_name', '', true),
433			'ad_note' => $this->request->variable('ad_note', '', true),
434			'ad_code' => $this->request->variable('ad_code', '', true),
435			'ad_enabled' => $this->request->variable('ad_enabled', 0),
436			'ad_locations' => $this->request->variable('ad_locations', array('')),
437			'ad_end_date' => $this->request->variable('ad_end_date', ''),
438	11		);
439
440	11		// Validate form key
441	11		if (!check_form_key($form_name))
442	8		{
443			$this->errors[] = $this->user->lang('FORM_INVALID');
444	3		}
445	3
446	3		// Validate ad name
447			if ($data['ad_name'] === '')
448			{
449			$this->errors[] = $this->user->lang('AD_NAME_REQUIRED');
450			}
451			if (truncate_string($data['ad_name'], self::MAX_NAME_LENGTH) !== $data['ad_name'])
452			{
453			$this->errors[] = $this->user->lang('AD_NAME_TOO_LONG', self::MAX_NAME_LENGTH);
454			}
455
456			// Validate ad end date
457			if (preg_match('#^\d{4}\-\d{2}\-\d{2}$#', $data['ad_end_date']))
458	12		{
459			$data['ad_end_date'] = (int) $this->user->get_timestamp_from_format(self::DATE_FORMAT, $data['ad_end_date']);
460	12
461			if ($data['ad_end_date'] < time())
462	12		{
463	12		$this->errors[] = $this->user->lang('AD_END_DATE_INVALID');
464	12		}
465	12		}
466	12		else if ($data['ad_end_date'] !== '')
467	12		{
468	12		$this->errors[] = $this->user->lang('AD_END_DATE_INVALID');
469	12		}
470			else
471			{
472			$data['ad_end_date'] = 0;
473			}
474
475			return $data;
476			}
477	2
478			/**
479	2		* Assign form data to the template.
480	2		*
481			* @param array $data The form data.
482			* @return void
483			*/
484			protected function assign_form_data($data)
485			{
486			$this->template->assign_vars(array(
487	5		'S_ERROR' => (bool) count($this->errors),
488			'ERROR_MSG' => count($this->errors) ? implode('<br />', $this->errors) : '',
489	5
490			'AD_NAME' => $data['ad_name'],
491			'AD_NOTE' => $data['ad_note'],
492			'AD_CODE' => $data['ad_code'],
493			'AD_ENABLED' => $data['ad_enabled'],
494			'AD_END_DATE' => $this->prepare_end_date($data['ad_end_date']),
495			));
496			}
497	5		/**
498			* Prepare end date for display
499	5		*
500			* @param mixed $end_date End date.
501			* @return string End date prepared for display.
502			*/
503			protected function prepare_end_date($end_date)
504			{
505			if (empty($end_date))
506			{
507			return '';
508			}
509	3		else if (is_numeric($end_date))
510			{
511	3		return $this->user->format_date($end_date, self::DATE_FORMAT);
512	3		}
513
514			return $end_date;
			0 ignored issues – show Bug Best Practice introduced 2017-06-09 10:04 UTC by Report Bug Copy Issue Report The return type of `return $end_date;` (`object\|string\|null\|array\|boolean`) is incompatible with the return type documented by `phpbb\admanagement\contr...oller::prepare_end_date` of type `string`. If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design. Let’s take a look at an example: class Author { private $name; public function __construct($name) { $this->name = $name; } public function getName() { return $this->name; } } abstract class Post { public function getAuthor() { return 'Johannes'; } } class BlogPost extends Post { public function getAuthor() { return new Author('Johannes'); } } class ForumPost extends Post { /* ... */ } function my_function(Post $post) { echo strtoupper($post->getAuthor()); } Our function `my_function` expects a `Post` object, and outputs the author of the post. The base class `Post` returns a simple string and outputting a simple string will work just fine. However, the child class `BlogPost` which is a sub-type of `Post` instead decided to return an `object`, and is therefore violating the SOLID principles. If a `BlogPost` were passed to `my_function`, PHP would not complain, but ultimately fail when executing the `strtoupper` call in its body. Loading history...
515			}
516
517			/**
518			* Assign template locations data to the template.
519			*
520			* @param mixed $data The form data or nothing.
521			* @return void
522			*/
523			protected function assign_locations($data = false)
524			{
525			foreach ($this->location_manager->get_all_locations() as $location_id => $location_data)
526			{
527			$this->template->assign_block_vars('ad_locations', array(
528			'LOCATION_ID' => $location_id,
529			'LOCATION_DESC' => $location_data['desc'],
530			'LOCATION_NAME' => $location_data['name'],
531			'S_SELECTED' => $data ? in_array($location_id, $data['ad_locations']) : false,
532			));
533			}
534			}
535
536			/**
537			* Prepare advertisement preview
538			*
539			* @param string $code Ad code to preview
540			* @return void
541			*/
542			protected function ad_preview($code)
543			{
544			$this->template->assign_var('PREVIEW', htmlspecialchars_decode($code));
545			}
546
547			/**
548			* Print success message.
549			*
550			* It takes arguments in the form of a language key, followed by language substitution values.
551			*/
552			protected function success()
553			{
554			trigger_error(call_user_func_array(array($this->user, 'lang'), func_get_args()) . adm_back_link($this->u_action));
555			}
556
557			/**
558			* Print error message.
559			*
560			* It takes arguments in the form of a language key, followed by language substitution values.
561			*/
562			protected function error()
563			{
564			trigger_error(call_user_func_array(array($this->user, 'lang'), func_get_args()) . adm_back_link($this->u_action), E_USER_WARNING);
565			}
566
567			/**
568			* Log action
569			*
570			* @param string $action Performed action in uppercase
571			* @param string $ad_name Advertisement name
572			* @return void
573			*/
574			protected function log($action, $ad_name)
575			{
576			$this->log->add('admin', $this->user->data['user_id'], $this->user->ip, 'ACP_ADMANAGEMENT_' . $action . '_LOG', time(), array($ad_name));
577			}
578			}
579

phpbb-extensions / ad-management