Completed
Pull Request — master (#75)
by Jakub
10:54
created

admin_controller   B

Complexity

Total Complexity 48

Size/Duplication

Total Lines 499
Duplicated Lines 1.6 %

Coupling/Cohesion

Components 1
Dependencies 4

Test Coverage

Coverage 93.1%

Importance

Changes 0
Metric Value
wmc 48
lcom 1
cbo 4
dl 8
loc 499
ccs 216
cts 232
cp 0.931
rs 8.4864
c 0
b 0
f 0

20 Methods

Rating   Name   Duplication   Size   Complexity  
A __construct() 0 23 2
A set_page_url() 0 4 1
A get_page_title() 0 4 1
B mode_settings() 8 37 4
A mode_manage() 0 14 2
B action_add() 0 25 2
B action_edit() 0 34 3
A action_enable() 0 4 1
A action_disable() 0 4 1
B action_delete() 0 40 5
B list_ads() 0 36 6
A get_submitted_action() 0 13 3
B ad_enable() 0 28 6
A preview() 0 4 1
A upload_banner() 0 4 1
A analyse_ad_code() 0 4 1
A submit_add() 0 12 2
A submit_edit() 0 20 4
A success() 0 4 1
A error() 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:

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.

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\ads\controller;
12
13
use phpbb\ads\controller\admin_input as input;
14
15
/**
16
* Admin controller
17
*/
18
class admin_controller
19
{
20
	/** @var array|bool Form data */
21
	protected $data = false;
22
23
	/** @var \phpbb\template\template */
24
	protected $template;
25
26
	/** @var \phpbb\language\language */
27
	protected $language;
28
29
	/** @var \phpbb\request\request */
30
	protected $request;
31
32
	/** @var \phpbb\ads\ad\manager */
33
	protected $manager;
34
35
	/** @var \phpbb\config\db_text */
36
	protected $config_text;
37
38
	/** @var \phpbb\config\config */
39
	protected $config;
40
41
	/** @var \phpbb\group\helper */
42
	protected $group_helper;
43
44
	/** @var \phpbb\ads\controller\admin_input */
45
	protected $input;
46
47
	/** @var \phpbb\ads\controller\admin_helper */
48
	protected $helper;
49
50
	/** @var \phpbb\ads\analyser\manager */
51
	protected $analyser;
52
53
	/** @var string Custom form action */
54
	protected $u_action;
55
56
	/**
57
	 * Constructor
58
	 *
59
	 * @param \phpbb\template\template           $template     Template object
60
	 * @param \phpbb\language\language           $language     Language object
61
	 * @param \phpbb\request\request             $request      Request object
62
	 * @param \phpbb\ads\ad\manager              $manager      Advertisement manager object
63
	 * @param \phpbb\config\db_text              $config_text  Config text object
64
	 * @param \phpbb\config\config               $config       Config object
65
	 * @param \phpbb\group\helper                $group_helper Group helper object
66
	 * @param \phpbb\ads\controller\admin_input  $input        Admin input object
67
	 * @param \phpbb\ads\controller\admin_helper $helper       Admin helper object
68
	 * @param \phpbb\ads\analyser\manager        $analyser     Ad code analyser object
69
	 * @param string                             $root_path    phpBB root path
70
	 * @param string                             $php_ext      PHP extension
71
	 */
72 33
	public function __construct(\phpbb\template\template $template, \phpbb\language\language $language, \phpbb\request\request $request, \phpbb\ads\ad\manager $manager, \phpbb\config\db_text $config_text, \phpbb\config\config $config, \phpbb\group\helper $group_helper, \phpbb\ads\controller\admin_input $input, \phpbb\ads\controller\admin_helper $helper, \phpbb\ads\analyser\manager $analyser, $root_path, $php_ext)
73
	{
74 33
		$this->template = $template;
75 33
		$this->language = $language;
76 33
		$this->request = $request;
77 33
		$this->manager = $manager;
78 33
		$this->config_text = $config_text;
79 33
		$this->config = $config;
80 33
		$this->group_helper = $group_helper;
81 33
		$this->input = $input;
82 33
		$this->helper = $helper;
83 33
		$this->analyser = $analyser;
84
85 33
		if (!function_exists('user_get_id_name'))
86 33
		{
87
			include $root_path . 'includes/functions_user.' . $php_ext;
88
		}
89
90 33
		$this->language->add_lang('posting'); // Used by banner_upload() file errors
91 33
		$this->language->add_lang('acp', 'phpbb/ads');
92
93 33
		$this->template->assign_var('S_PHPBB_ADS', true);
94 33
	}
95
96
	/**
97
	 * Set page url
98
	 *
99
	 * @param	string	$u_action	Custom form action
100
	 * @return	void
101
	 */
102 27
	public function set_page_url($u_action)
103
	{
104 27
		$this->u_action = $u_action;
105 27
	}
106
107
	/**
108
	 * Get ACP page title for Ads module
109
	 *
110
	 * @return	string	Language string for Ads ACP module
111
	 */
112 1
	public function get_page_title()
113
	{
114 1
		return $this->language->lang('ACP_PHPBB_ADS_TITLE');
115
	}
116
117
	/**
118
	 * Process user request for settings mode
119
	 *
120
	 * @return	void
121
	 */
122 3
	public function mode_settings()
123
	{
124 3
		add_form_key('phpbb/ads/settings');
125 3
		if ($this->request->is_set_post('submit'))
126 3
		{
127
			// Validate form key
128 2
			if (check_form_key('phpbb/ads/settings'))
129 2
			{
130 1
				$this->config->set('phpbb_ads_adblocker_message', $this->request->variable('adblocker_message', 0));
131 1
				$this->config->set('phpbb_ads_enable_views', $this->request->variable('enable_views', 0));
132 1
				$this->config->set('phpbb_ads_enable_clicks', $this->request->variable('enable_clicks', 0));
133 1
				$this->config_text->set('phpbb_ads_hide_groups', json_encode($this->request->variable('hide_groups', array(0))));
134
135 1
				$this->success('ACP_AD_SETTINGS_SAVED');
136
			}
137
138 1
			$this->error('FORM_INVALID');
139
		}
140
141 1
		$hide_groups = json_decode($this->config_text->get('phpbb_ads_hide_groups'), true);
142 1
		$groups = $this->manager->load_groups();
143 1 View Code Duplication
		foreach ($groups as $group)
144
		{
145 1
			$this->template->assign_block_vars('groups', array(
146 1
				'ID'         => $group['group_id'],
147 1
				'NAME'       => $this->group_helper->get_name($group['group_name']),
148 1
				'S_SELECTED' => in_array($group['group_id'], $hide_groups),
149 1
			));
150 1
		}
151
152 1
		$this->template->assign_vars(array(
153 1
			'U_ACTION'          => $this->u_action,
154 1
			'ADBLOCKER_MESSAGE' => $this->config['phpbb_ads_adblocker_message'],
155 1
			'ENABLE_VIEWS'      => $this->config['phpbb_ads_enable_views'],
156 1
			'ENABLE_CLICKS'     => $this->config['phpbb_ads_enable_clicks'],
157 1
		));
158 1
	}
159
160
	/**
161
	 * Process user request for manage mode
162
	 *
163
	 * @return	void
164
	 */
165 29
	public function mode_manage()
166
	{
167
		// Trigger specific action
168 29
		$action = $this->request->variable('action', '');
169 29
		if (in_array($action, array('add', 'edit', 'enable', 'disable', 'delete')))
170 29
		{
171 27
			$this->{'action_' . $action}();
172 15
		}
173
		else
174
		{
175
			// Otherwise default to this
176 2
			$this->list_ads();
177
		}
178 17
	}
179
180
	/**
181
	 * Add an advertisement
182
	 *
183
	 * @return	void
184
	 */
185 12
	protected function action_add()
186
	{
187 6
		add_form_key('phpbb/ads/add');
188
189 6
		$action = $this->get_submitted_action();
190 6
		if ($action !== false)
191 6
		{
192 5
			$this->data = $this->input->get_form_data('phpbb/ads/add');
193 5
			$this->{$action}();
194 4
			$this->helper->assign_data($this->data, $this->input->get_errors());
195 4
		}
196 12
		else
197
		{
198 1
			$this->helper->assign_locations();
199
		}
200
201
		// Set output vars for display in the template
202 5
		$this->template->assign_vars(array(
203 5
			'S_ADD_AD'				=> true,
204 5
			'U_BACK'				=> $this->u_action,
205 5
			'U_ACTION'				=> "{$this->u_action}&amp;action=add",
206 5
			'PICKER_DATE_FORMAT'	=> input::DATE_FORMAT,
207 5
			'U_FIND_USERNAME'		=> $this->helper->get_find_username_link(),
208 5
		));
209 5
	}
210
211
	/**
212
	 * Edit an advertisement
213
	 *
214
	 * @return	void
215
	 */
216 7
	protected function action_edit()
217
	{
218 7
		$ad_id = $this->request->variable('id', 0);
219 7
		add_form_key('phpbb/ads/edit/' . $ad_id);
220
221 7
		$action = $this->get_submitted_action();
222 7
		if ($action !== false)
223 7
		{
224 5
			$this->data = $this->input->get_form_data('phpbb/ads/edit/' . $ad_id);
225 5
			$this->{$action}();
226 3
		}
227
		else
228
		{
229 2
			$this->data = $this->manager->get_ad($ad_id);
230 2
			if ($this->data === false)
231 2
			{
232 1
				$this->error('ACP_AD_DOES_NOT_EXIST');
233
			}
234
235
			// Load ad template locations
236 1
			$this->data['ad_locations'] = $this->manager->get_ad_locations($ad_id);
237
		}
238
239
		// Set output vars for display in the template
240 4
		$this->template->assign_vars(array(
241 4
			'S_EDIT_AD'				=> true,
242 4
			'EDIT_ID'				=> $ad_id,
243 4
			'U_BACK'				=> $this->u_action,
244 4
			'U_ACTION'				=> "{$this->u_action}&amp;action=edit&amp;id=$ad_id",
245 4
			'PICKER_DATE_FORMAT'	=> input::DATE_FORMAT,
246 4
			'U_FIND_USERNAME'		=> $this->helper->get_find_username_link(),
247 4
		));
248 4
		$this->helper->assign_data($this->data, $this->input->get_errors());
0 ignored issues
show
Bug introduced by
It seems like $this->data can also be of type boolean; however, phpbb\ads\controller\admin_helper::assign_data() does only seem to accept array, maybe add an additional type check?

If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check:

/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}

If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue.

Loading history...
249 4
	}
250
251
	/**
252
	 * Enable an advertisement
253
	 *
254
	 * @return	void
255
	 */
256 4
	protected function action_enable()
257
	{
258 4
		$this->ad_enable(true);
259 1
	}
260
261
	/**
262
	 * Disable an advertisement
263
	 *
264
	 * @return	void
265
	 */
266 4
	protected function action_disable()
267
	{
268 4
		$this->ad_enable(false);
269 1
	}
270
271
	/**
272
	 * Delete an advertisement
273
	 *
274
	 * @return	void
275
	 */
276 3
	protected function action_delete()
277
	{
278 3
		$ad_id = $this->request->variable('id', 0);
279
		if ($ad_id)
280 3
		{
281 3
			if (confirm_box(true))
282 3
			{
283
				// Get ad data so that we can log ad name
284 2
				$ad_data = $this->manager->get_ad($ad_id);
285
286
				// Delete ad and it's template locations
287 2
				$this->manager->delete_ad_locations($ad_id);
288 2
				$success = $this->manager->delete_ad($ad_id);
289
290
				// Only notify user on error or if not ajax
291 2
				if (!$success)
292 2
				{
293 1
					$this->error('ACP_AD_DELETE_ERRORED');
294
				}
295
				else
296
				{
297 1
					$this->helper->log('DELETE', $ad_data['ad_name']);
298
299 1
					if (!$this->request->is_ajax())
300 1
					{
301 1
						$this->success('ACP_AD_DELETE_SUCCESS');
302
					}
303
				}
304
			}
305
			else
306
			{
307 1
				confirm_box(false, $this->language->lang('CONFIRM_OPERATION'), build_hidden_fields(array(
308 1
					'id'     => $ad_id,
309 1
					'i'      => $this->request->variable('i', ''),
310 1
					'mode'   => $this->request->variable('mode', ''),
311 1
					'action' => 'delete',
312 1
				)));
313
			}
314 1
		}
315 1
	}
316
317
	/**
318
	 * Display the list of all ads
319
	 *
320
	 * @return	void
321
	 */
322 1
	protected function list_ads()
323
	{
324 1
		foreach ($this->manager->get_all_ads() as $row)
325
		{
326 1
			$ad_enabled = (int) $row['ad_enabled'];
327 1
			$ad_expired = $this->helper->is_expired($row);
328
329 1
			if ($ad_expired && $ad_enabled)
330 1
			{
331 1
				$ad_enabled = 0;
332 1
				$this->manager->update_ad($row['ad_id'], array('ad_enabled' => 0));
333 1
			}
334
335 1
			$this->template->assign_block_vars($ad_expired ? 'expired' : 'ads', array(
336 1
				'NAME'         => $row['ad_name'],
337 1
				'PRIORITY'     => $row['ad_priority'],
338 1
				'END_DATE'     => $this->helper->prepare_end_date($row['ad_end_date']),
339 1
				'VIEWS'        => $row['ad_views'],
340 1
				'CLICKS'       => $row['ad_clicks'],
341 1
				'VIEWS_LIMIT'  => $row['ad_views_limit'],
342 1
				'CLICKS_LIMIT' => $row['ad_clicks_limit'],
343 1
				'S_EXPIRED'    => $ad_expired,
344 1
				'S_ENABLED'    => $ad_enabled,
345 1
				'U_ENABLE'     => $this->u_action . '&amp;action=' . ($ad_enabled ? 'disable' : 'enable') . '&amp;id=' . $row['ad_id'],
346 1
				'U_EDIT'       => $this->u_action . '&amp;action=edit&amp;id=' . $row['ad_id'],
347 1
				'U_DELETE'     => $this->u_action . '&amp;action=delete&amp;id=' . $row['ad_id'],
348 1
			));
349 1
		}
350
351
		// Set output vars for display in the template
352 1
		$this->template->assign_vars(array(
353 1
			'U_ACTION_ADD'     => $this->u_action . '&amp;action=add',
354 1
			'S_VIEWS_ENABLED'  => $this->config['phpbb_ads_enable_views'],
355 1
			'S_CLICKS_ENABLED' => $this->config['phpbb_ads_enable_clicks'],
356 1
		));
357 1
	}
358
359
	/**
360
	 * Get what action user wants to do with the form.
361
	 * Possible options are:
362
	 *  - preview ad code
363
	 *  - upload banner to display in an ad code
364
	 *  - analyse ad code
365
	 *  - submit form (either add or edit an ad)
366
	 *
367
	 * @return	mixed	Action name or false when no action was submitted
368
	 */
369 13
	protected function get_submitted_action()
370
	{
371 13
		$actions = array('preview', 'upload_banner', 'analyse_ad_code', 'submit_add', 'submit_edit');
372 13
		foreach ($actions as $action)
373
		{
374 13
			if ($this->request->is_set_post($action))
375 13
			{
376 10
				return $action;
377
			}
378 11
		}
379
380 3
		return false;
381
	}
382
383
	/**
384
	 * Enable/disable an advertisement
385
	 *
386
	 * @param	bool	$enable	Enable or disable the advertisement?
387
	 * @return	void
388
	 */
389 6
	protected function ad_enable($enable)
390
	{
391 6
		$ad_id = $this->request->variable('id', 0);
392
393 6
		$success = $this->manager->update_ad($ad_id, array(
394 6
			'ad_enabled' => (int) $enable,
395 6
		));
396
397
		// If AJAX was used, show user a result message
398 6
		if ($this->request->is_ajax())
399 6
		{
400 2
			$json_response = new \phpbb\json_response;
401 2
			$json_response->send(array(
402 2
				'text'  => $this->language->lang($enable ? 'ENABLED' : 'DISABLED'),
403 2
				'title' => $this->language->lang('AD_ENABLE_TITLE', (int) $enable),
404 2
			));
405
		}
406
407
		// Otherwise, show traditional infobox
408
		if ($success)
409 4
		{
410 2
			$this->success($enable ? 'ACP_AD_ENABLE_SUCCESS' : 'ACP_AD_DISABLE_SUCCESS');
411
		}
412
		else
413
		{
414 2
			$this->error($enable ? 'ACP_AD_ENABLE_ERRORED' : 'ACP_AD_DISABLE_ERRORED');
415
		}
416
	}
417
418
	/**
419
	 * Submit action "preview".
420
	 * Prepare advertisement preview.
421
	 *
422
	 * @return	void
423
	 */
424 2
	protected function preview()
425
	{
426 2
		$this->template->assign_var('PREVIEW', htmlspecialchars_decode($this->data['ad_code']));
427 2
	}
428
429
	/**
430
	 * Submit action "upload_banner".
431
	 * Upload banner and append it to the ad code.
432
	 *
433
	 * @return	void
434
	 */
435 1
	protected function upload_banner()
436
	{
437 1
		$this->data['ad_code'] = $this->input->banner_upload($this->data['ad_code']);
438 1
	}
439
440
	/**
441
	 * Submit action "analyse_ad_code".
442
	 * Upload banner and append it to the ad code.
443
	 *
444
	 * @return	void
445
	 */
446 1
	protected function analyse_ad_code()
447
	{
448 1
		$this->analyser->run($this->data['ad_code']);
449 1
	}
450
451
	/**
452
	 * Submit action "submit_add".
453
	 * Add new ad.
454
	 *
455
	 * @return	void
456
	 */
457 2
	protected function submit_add()
458
	{
459 2
		if (!$this->input->has_errors())
460 2
		{
461 1
			$ad_id = $this->manager->insert_ad($this->data);
0 ignored issues
show
Bug introduced by
It seems like $this->data can also be of type boolean; however, phpbb\ads\ad\manager::insert_ad() does only seem to accept array, maybe add an additional type check?

If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check:

/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}

If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue.

Loading history...
462 1
			$this->manager->insert_ad_locations($ad_id, $this->data['ad_locations']);
463
464 1
			$this->helper->log('ADD', $this->data['ad_name']);
465
466 1
			$this->success('ACP_AD_ADD_SUCCESS');
467
		}
468 1
	}
469
470
	/**
471
	 * Submit action "submit_edit".
472
	 * Edit ad.
473
	 *
474
	 * @return	void
475
	 */
476 4
	protected function submit_edit()
477
	{
478 4
		$ad_id = $this->request->variable('id', 0);
479 4
		if ($ad_id && !$this->input->has_errors())
480 4
		{
481 2
			$success = $this->manager->update_ad($ad_id, $this->data);
0 ignored issues
show
Bug introduced by
It seems like $this->data can also be of type boolean; however, phpbb\ads\ad\manager::update_ad() does only seem to accept array, maybe add an additional type check?

If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check:

/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}

If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue.

Loading history...
482
			if ($success)
483 2
			{
484
				// Only insert new ad locations to DB when ad exists
485 1
				$this->manager->delete_ad_locations($ad_id);
486 1
				$this->manager->insert_ad_locations($ad_id, $this->data['ad_locations']);
487
488 1
				$this->helper->log('EDIT', $this->data['ad_name']);
489
490 1
				$this->success('ACP_AD_EDIT_SUCCESS');
491
			}
492
493 1
			$this->error('ACP_AD_DOES_NOT_EXIST');
494
		}
495 2
	}
496
497
	/**
498
	 * Print success message.
499
	 *
500
	 * @param	string	$msg	Message lang key
501
	 */
502 6
	protected function success($msg)
503
	{
504 6
		trigger_error($this->language->lang($msg) . adm_back_link($this->u_action));
505
	}
506
507
	/**
508
	 * Print error message.
509
	 *
510
	 * @param	string	$msg	Message lang key
511
	 */
512 6
	protected function error($msg)
513
	{
514 6
		trigger_error($this->language->lang($msg) . adm_back_link($this->u_action), E_USER_WARNING);
515
	}
516
}
517