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

admin_controller::__construct()   A

Complexity

Conditions 2
Paths 2

Size

Total Lines 23
Code Lines 16

Duplication

Lines 0
Ratio 0 %

Code Coverage

Tests 17
CRAP Score 2.0046

Importance

Changes 0
Metric Value
dl 0
loc 23
ccs 17
cts 19
cp 0.8947
rs 9.0856
c 0
b 0
f 0
cc 2
eloc 16
nc 2
nop 12
crap 2.0046

How to fix   Many Parameters   

Many Parameters

Methods with many parameters are not only hard to understand, but their parameters also often become inconsistent when you need more, or different data.

There are several approaches to avoid long parameter lists:

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