Completed
Pull Request — master (#75)
by Jakub
11:37
created

admin_controller::__construct()   A

Complexity

Conditions 1
Paths 1

Size

Total Lines 15
Code Lines 13

Duplication

Lines 0
Ratio 0 %

Code Coverage

Tests 14
CRAP Score 1

Importance

Changes 0
Metric Value
c 0
b 0
f 0
dl 0
loc 15
ccs 14
cts 14
cp 1
rs 9.4285
cc 1
eloc 13
nc 1
nop 12
crap 1

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 \phpbb\template\template */
21
	protected $template;
22
23
	/** @var \phpbb\language\language */
24
	protected $language;
25
26
	/** @var \phpbb\request\request */
27
	protected $request;
28
29
	/** @var \phpbb\ads\ad\manager */
30
	protected $manager;
31
32
	/** @var \phpbb\config\db_text */
33
	protected $config_text;
34
35
	/** @var \phpbb\config\config */
36
	protected $config;
37
38
	/** @var \phpbb\group\helper */
39
	protected $group_helper;
40
41
	/** @var \phpbb\ads\controller\admin_input */
42
	protected $input;
43
44
	/** @var \phpbb\ads\controller\admin_helper */
45
	protected $helper;
46
47
	/** @var \phpbb\ads\analyser\manager */
48
	protected $analyser;
49
50
	/** @var string root_path */
51
	protected $root_path;
52
53
	/** @var string php_ext */
54
	protected $php_ext;
55
56
	/** @var string Custom form action */
57
	protected $u_action;
58
59
	/**
60
	 * Constructor
61
	 *
62
	 * @param \phpbb\template\template				$template		Template object
63
	 * @param \phpbb\language\language				$language		Language object
64
	 * @param \phpbb\request\request				$request		Request object
65
	 * @param \phpbb\ads\ad\manager					$manager		Advertisement manager object
66
	 * @param \phpbb\config\db_text					$config_text 	Config text object
67
	 * @param \phpbb\config\config					$config			Config object
68
	 * @param \phpbb\group\helper					$group_helper	Group helper object
69
	 * @param \phpbb\ads\controller\admin_input 	$input			Admin input object
70
	 * @param \phpbb\ads\controller\admin_helper	$helper			Admin helper object
71
	 * @param \phpbb\ads\analyser\manager			$analyser		Ad code analyser object
72
	 * @param string								$root_path		phpBB root path
73
	 * @param string								$php_ext		PHP extension
74
	 */
75 26
	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)
76
	{
77 26
		$this->template = $template;
78 26
		$this->language = $language;
79 26
		$this->request = $request;
80 26
		$this->manager = $manager;
81 26
		$this->config_text = $config_text;
82 26
		$this->config = $config;
83 26
		$this->group_helper = $group_helper;
84 26
		$this->input = $input;
85 26
		$this->helper = $helper;
86 26
		$this->analyser = $analyser;
87 26
		$this->root_path = $root_path;
88 26
		$this->php_ext = $php_ext;
89 26
	}
90
91
	/**
92
	 * Process user request for manage mode
93
	 *
94
	 * @return void
95
	 */
96 6
	public function mode_manage()
97
	{
98 6
		$this->setup();
99
100
		// Trigger specific action
101 6
		$action = $this->request->variable('action', '');
102 6
		if (in_array($action, array('add', 'edit', 'enable', 'disable', 'delete')))
103 6
		{
104 5
			$this->{'action_' . $action}();
105 5
		}
106
107
		// Otherwise default to this
108 6
		$this->list_ads();
109 6
	}
110
111
	/**
112
	 * Process user request for settings mode
113
	 *
114
	 * @return void
115
	 */
116 3
	public function mode_settings()
117
	{
118 3
		$this->setup();
119
120 3
		add_form_key('phpbb/ads/settings');
121 3
		if ($this->request->is_set_post('submit'))
122 3
		{
123
			// Validate form key
124 2
			if (check_form_key('phpbb/ads/settings'))
125 2
			{
126 1
				$this->config->set('phpbb_ads_adblocker_message', $this->request->variable('adblocker_message', 0));
127 1
				$this->config->set('phpbb_ads_enable_views', $this->request->variable('enable_views', 0));
128 1
				$this->config->set('phpbb_ads_enable_clicks', $this->request->variable('enable_clicks', 0));
129 1
				$this->config_text->set('phpbb_ads_hide_groups', json_encode($this->request->variable('hide_groups', array(0))));
130
131 1
				$this->success('ACP_AD_SETTINGS_SAVED');
132
			}
133
134 1
			$this->helper->assign_errors(array($this->language->lang('FORM_INVALID')));
135 1
		}
136
137 2
		$hide_groups = json_decode($this->config_text->get('phpbb_ads_hide_groups'), true);
138 2
		$groups = $this->manager->load_groups();
139 2 View Code Duplication
		foreach ($groups as $group)
140
		{
141 2
			$this->template->assign_block_vars('groups', array(
142 2
				'ID'         => $group['group_id'],
143 2
				'NAME'       => $this->group_helper->get_name($group['group_name']),
144 2
				'S_SELECTED' => in_array($group['group_id'], $hide_groups),
145 2
			));
146 2
		}
147
148 2
		$this->template->assign_vars(array(
149 2
			'U_ACTION'          => $this->u_action,
150 2
			'ADBLOCKER_MESSAGE' => $this->config['phpbb_ads_adblocker_message'],
151 2
			'ENABLE_VIEWS'      => $this->config['phpbb_ads_enable_views'],
152 2
			'ENABLE_CLICKS'     => $this->config['phpbb_ads_enable_clicks'],
153 2
		));
154 2
	}
155
156
	/**
157
	 * Set page url
158
	 *
159
	 * @param string $u_action Custom form action
160
	 * @return void
161
	 */
162 20
	public function set_page_url($u_action)
163
	{
164 20
		$this->u_action = $u_action;
165 20
	}
166
167
	/**
168
	 * Get ACP page title for Ads module
169
	 *
170
	 * @return string    Language string for Ads ACP module
171
	 */
172 1
	public function get_page_title()
173
	{
174 1
		return $this->language->lang('ACP_PHPBB_ADS_TITLE');
175
	}
176
177
	/**
178
	 * Add an advertisement
179
	 *
180
	 * @return void
181
	 */
182 1
	public function action_add()
183
	{
184 1
		$this->process_form('add', 0);
185
	}
186
187
	/**
188
	 * Edit an advertisement
189
	 *
190
	 * @return void
191
	 */
192 12
	public function action_edit()
193
	{
194 5
		$ad_id = $this->request->variable('id', 0);
195
196 12
		$this->process_form('edit', $ad_id);
197 2
	}
198
199
	/**
200
	 * Enable an advertisement
201
	 *
202
	 * @return void
203
	 */
204 4
	public function action_enable()
205
	{
206 4
		$this->ad_enable(true);
207 1
	}
208
209
	/**
210
	 * Disable an advertisement
211
	 *
212
	 * @return void
213
	 */
214 4
	public function action_disable()
215
	{
216 4
		$this->ad_enable(false);
217 1
	}
218
219
	/**
220
	 * Delete an advertisement
221
	 *
222
	 * @return void
223
	 */
224 3
	public function action_delete()
225
	{
226 3
		$ad_id = $this->request->variable('id', 0);
227
		if ($ad_id)
228 3
		{
229 3
			if (confirm_box(true))
230 3
			{
231
				// Get ad data so that we can log ad name
232 2
				$ad_data = $this->manager->get_ad($ad_id);
233
234
				// Delete ad and it's template locations
235 2
				$this->manager->delete_ad_locations($ad_id);
236 2
				$success = $this->manager->delete_ad($ad_id);
237
238
				// Only notify user on error or if not ajax
239 2
				if (!$success)
240 2
				{
241 1
					$this->error('ACP_AD_DELETE_ERRORED');
242
				}
243
				else
244
				{
245 1
					$this->helper->log('DELETE', $ad_data['ad_name']);
246
247 1
					if (!$this->request->is_ajax())
248 1
					{
249 1
						$this->success('ACP_AD_DELETE_SUCCESS');
250
					}
251
				}
252
			}
253
			else
254
			{
255 1
				confirm_box(false, $this->language->lang('CONFIRM_OPERATION'), build_hidden_fields(array(
256 1
					'id'     => $ad_id,
257 1
					'i'      => $this->request->variable('i', ''),
258 1
					'mode'   => $this->request->variable('mode', ''),
259
					'action' => 'delete'
260 1
				)));
261
			}
262 1
		}
263 1
	}
264
265
	/**
266
	 * Display the ads
267
	 *
268
	 * @return void
269
	 */
270 1
	public function list_ads()
271
	{
272 1
		foreach ($this->manager->get_all_ads() as $row)
273
		{
274 1
			$ad_enabled = (int) $row['ad_enabled'];
275 1
			$ad_end_date = (int) $row['ad_end_date'];
276 1
			$ad_expired = ($ad_end_date > 0 && $ad_end_date < time()) || ($row['ad_views_limit'] && $row['ad_views'] >= $row['ad_views_limit']) || ($row['ad_clicks_limit'] && $row['ad_clicks'] >= $row['ad_clicks_limit']);
277 1
			if ($ad_expired && $ad_enabled)
278 1
			{
279 1
				$ad_enabled = 0;
280 1
				$this->manager->update_ad($row['ad_id'], array('ad_enabled' => 0));
281 1
			}
282
283 1
			$this->template->assign_block_vars($ad_expired ? 'expired' : 'ads', array(
284 1
				'NAME'               => $row['ad_name'],
285 1
				'PRIORITY'			 => $row['ad_priority'],
286 1
				'END_DATE'           => $this->helper->prepare_end_date($ad_end_date),
287 1
				'VIEWS'              => $row['ad_views'],
288 1
				'CLICKS'             => $row['ad_clicks'],
289 1
				'VIEWS_LIMIT'        => $row['ad_views_limit'],
290 1
				'CLICKS_LIMIT'       => $row['ad_clicks_limit'],
291 1
				'S_EXPIRED' 		 => $ad_expired,
292 1
				'S_ENABLED'          => $ad_enabled,
293 1
				'U_ENABLE'           => $this->u_action . '&amp;action=' . ($ad_enabled ? 'disable' : 'enable') . '&amp;id=' . $row['ad_id'],
294 1
				'U_EDIT'             => $this->u_action . '&amp;action=edit&amp;id=' . $row['ad_id'],
295 1
				'U_DELETE'           => $this->u_action . '&amp;action=delete&amp;id=' . $row['ad_id'],
296 1
			));
297 1
		}
298
299
		// Set output vars for display in the template
300 1
		$this->template->assign_vars(array(
301 1
			'U_ACTION_ADD'     => $this->u_action . '&amp;action=add',
302 1
			'S_VIEWS_ENABLED'  => $this->config['phpbb_ads_enable_views'],
303 1
			'S_CLICKS_ENABLED' => $this->config['phpbb_ads_enable_clicks'],
304 1
		));
305 1
	}
306
307
	/**
308
	 * Perform general tasks
309
	 *
310
	 * @return void
311
	 */
312 9
	protected function setup()
313
	{
314 9
		if (!function_exists('user_get_id_name'))
315 9
		{
316
			include $this->root_path . 'includes/functions_user.' . $this->php_ext;
317
		}
318
319 9
		$this->language->add_lang('posting'); // Used by banner_upload() file errors
320 9
		$this->language->add_lang('acp', 'phpbb/ads');
321
322 9
		$this->template->assign_var('S_PHPBB_ADS', true);
323 9
	}
324
325
	/**
326
	 * Edit an advertisement
327
	 *
328
	 * @return void
329
	 */
330 6
	public function process_form($action, $ad_id)
331
	{
332 6
		$preview = $this->request->is_set_post('preview');
333 6
		$submit = $this->request->is_set_post('submit');
334 6
		$upload_banner = $this->request->is_set_post('upload_banner');
335 6
		$analyse_ad_code = $this->request->is_set_post('analyse_ad_code');
336
337 6
		add_form_key('phpbb/ads/' . $action . '/' . $ad_id);
338 6
		if ($preview || $submit || $upload_banner || $analyse_ad_code)
339 6
		{
340 5
			$data = $this->input->get_form_data('phpbb/ads/' . $action . '/' . $ad_id);
341
342
			if ($preview)
343 4
			{
344
				$this->ad_preview($data['ad_code']);
345
			}
346
			else if ($upload_banner)
347 4
			{
348
				$data['ad_code'] = $this->input->banner_upload($data['ad_code']);
349
			}
350
			else if ($analyse_ad_code)
351 4
			{
352
				$this->analyser->run($data['ad_code']);
353
			}
354 4
			else if (!$this->input->has_errors())
355 4
			{
356 2
				$this->{$action . '_ad'}($data, $ad_id);
357
			}
358
359 2
			if ($action == 'add')
360 2
			{
361
				$this->assign_data($data);
362
			}
363 2
		}
364
		else
365
		{
366 1
			$this->helper->assign_locations();
367
368 1
			if ($action == 'edit')
369 1
			{
370 1
				$data = $this->manager->get_ad($ad_id);
371 1
				if (empty($data))
372 1
				{
373 1
					$this->error('ACP_AD_DOES_NOT_EXIST');
374
				}
375
376
				// Load ad template locations
377
				$data['ad_locations'] = $this->manager->get_ad_locations($ad_id);
378
			}
379
		}
380
381
		// Set output vars for display in the template
382 2
		$this->template->assign_vars(array(
383 2
			'S_ADD_AD'           => $action == 'add',
384 2
			'S_EDIT_AD'          => $action == 'edit',
385 2
			'EDIT_ID'            => $ad_id,
386 2
			'U_BACK'             => $this->u_action,
387 2
			'U_ACTION'           => "{$this->u_action}&amp;action=$action" . ($action == 'edit' ? '&amp;id=' . $ad_id : ''),
388 2
			'PICKER_DATE_FORMAT' => input::DATE_FORMAT,
389 2
			'U_FIND_USERNAME'    => $this->helper->get_find_username_link(),
390 2
		));
391
392 2
		if ($action == 'edit')
393 2
		{
394 2
			$this->assign_data($data);
0 ignored issues
show
Bug introduced by
The variable $data does not seem to be defined for all execution paths leading up to this point.

If you define a variable conditionally, it can happen that it is not defined for all execution paths.

Let’s take a look at an example:

function myFunction($a) {
    switch ($a) {
        case 'foo':
            $x = 1;
            break;

        case 'bar':
            $x = 2;
            break;
    }

    // $x is potentially undefined here.
    echo $x;
}

In the above example, the variable $x is defined if you pass “foo” or “bar” as argument for $a. However, since the switch statement has no default case statement, if you pass any other value, the variable $x would be undefined.

Available Fixes

  1. Check for existence of the variable explicitly:

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
        }
    
        if (isset($x)) { // Make sure it's always set.
            echo $x;
        }
    }
    
  2. Define a default value for the variable:

    function myFunction($a) {
        $x = ''; // Set a default which gets overridden for certain paths.
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
        }
    
        echo $x;
    }
    
  3. Add a value for the missing path:

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
    
            // We add support for the missing case.
            default:
                $x = '';
                break;
        }
    
        echo $x;
    }
    
Loading history...
395 2
		}
396 2
	}
397
398
	protected function add_ad($data)
399
	{
400
		$ad_id = $this->manager->insert_ad($data);
401
		$this->manager->insert_ad_locations($ad_id, $data['ad_locations']);
402
403
		$this->helper->log('ADD', $data['ad_name']);
404
405
		$this->success('ACP_AD_ADD_SUCCESS');
406
	}
407
408 2
	protected function edit_ad($data, $ad_id)
409
	{
410 2
		$success = $this->manager->update_ad($ad_id, $data);
411
412
		if ($success)
413 2
		{
414
			// Only insert new ad locations to DB when ad exists
415 1
			$this->manager->delete_ad_locations($ad_id);
416 1
			$this->manager->insert_ad_locations($ad_id, $data['ad_locations']);
417
418 1
			$this->helper->log('EDIT', $data['ad_name']);
419
420 1
			$this->success('ACP_AD_EDIT_SUCCESS');
421
		}
422
423 1
		$this->error('ACP_AD_DOES_NOT_EXIST');
424
	}
425
426 2
	protected function assign_data($data)
427
	{
428 2
		$this->helper->assign_locations($data['ad_locations']);
429 2
		$this->helper->assign_form_data($data);
430 2
		$this->helper->assign_errors($this->input->get_errors());
431 2
	}
432
433
	/**
434
	 * Enable/disable an advertisement
435
	 *
436
	 * @param    bool $enable Enable or disable the advertisement?
437
	 * @return void
438
	 */
439 6
	protected function ad_enable($enable)
440
	{
441 6
		$ad_id = $this->request->variable('id', 0);
442
443 6
		$success = $this->manager->update_ad($ad_id, array(
444 6
			'ad_enabled' => (int) $enable,
445 6
		));
446
447
		// If AJAX was used, show user a result message
448 6
		if ($this->request->is_ajax())
449 6
		{
450 2
			$json_response = new \phpbb\json_response;
451 2
			$json_response->send(array(
452 2
				'text'  => $this->language->lang($enable ? 'ENABLED' : 'DISABLED'),
453 2
				'title' => $this->language->lang('AD_ENABLE_TITLE', (int) $enable),
454 2
			));
455
		}
456
457
		// Otherwise, show traditional infobox
458
		if ($success)
459 4
		{
460 2
			$this->success($enable ? 'ACP_AD_ENABLE_SUCCESS' : 'ACP_AD_DISABLE_SUCCESS');
461
		}
462
		else
463
		{
464 2
			$this->error($enable ? 'ACP_AD_ENABLE_ERRORED' : 'ACP_AD_DISABLE_ERRORED');
465
		}
466
	}
467
468
	/**
469
	 * Prepare advertisement preview
470
	 *
471
	 * @param    string $code Ad code to preview
472
	 * @return    void
473
	 */
474
	protected function ad_preview($code)
475
	{
476
		$this->template->assign_var('PREVIEW', htmlspecialchars_decode($code));
477
	}
478
479
	/**
480
	 * Print success message.
481
	 *
482
	 * It takes arguments in the form of a language key, followed by language substitution values.
483
	 */
484 5
	protected function success()
485
	{
486 5
		trigger_error(call_user_func_array(array($this->language, 'lang'), func_get_args()) . adm_back_link($this->u_action));
487
	}
488
489
	/**
490
	 * Print error message.
491
	 *
492
	 * It takes arguments in the form of a language key, followed by language substitution values.
493
	 */
494 5
	protected function error()
495
	{
496 5
		trigger_error(call_user_func_array(array($this->language, 'lang'), func_get_args()) . adm_back_link($this->u_action), E_USER_WARNING);
497
	}
498
}
499