Completed
Pull Request — master (#16)
by Jakub
15:59
created

admin_controller::action_delete()   B

Complexity

Conditions 5
Paths 5

Size

Total Lines 40
Code Lines 19

Duplication

Lines 0
Ratio 0 %

Code Coverage

Tests 24
CRAP Score 5.0113

Importance

Changes 2
Bugs 0 Features 0
Metric Value
dl 0
loc 40
ccs 24
cts 26
cp 0.9231
rs 8.439
c 2
b 0
f 0
cc 5
eloc 19
nc 5
nop 0
crap 5.0113
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 by
$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 by
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 by
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 . '&amp;action=' . ($ad_enabled ? 'disable' : 'enable') . '&amp;id=' . $row['ad_id'],
367 13
				'U_EDIT'				=> $this->u_action . '&amp;action=edit&amp;id=' . $row['ad_id'],
368 13
				'U_DELETE'				=> $this->u_action . '&amp;action=delete&amp;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 . '&amp;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 by
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