Issues in department-summary.php - Fixed Issues - Inspection of "Objectified, cacheable Snapshots" - smtech/grading-analytics - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( 505b4c...62d9c0 )

by Seth

created 2017-06-07 18:00 UTC

department-summary.php (4 issues)

Labels

Severity

Minor 4

Upgrade to new PHP Analysis Engine

These results are based on our legacy PHP analysis, consider migrating to our new PHP analysis engine instead. Learn more

<?php


require_once 'common.inc.php';

use smtech\StMarksSmarty\StMarksSmarty;

/*
 * FIXME this should not be hard-coded -- roles should be configurable.
 */
$unrestrictedRoles = [
    'AccountAdmin',
    'Dean',
    'Department Chair'
];

$smarty->enable(StMarksSmarty::MODULE_SORTABLE);

$account_id = $toolProvider->user->getResourceLink()->settings['custom_canvas_account_id'];
$departments = $api->get("/accounts/$account_id");

/* verify user privileges */
$user_id = $toolProvider->user->getResourceLink()->settings['custom_canvas_user_id'];
$restricted = true;
$permissionsAccount = $departments;
do {
    $roles = $api->get(
        "/accounts/{$permissionsAccount['id']}/admins",
        [
            'user_id[]' => $user_id
        ]
    );
    foreach ($roles as $role) {
        if (in_array($role['role'], $unrestrictedRoles)) {
            $restricted = false;
            break;
        }
    }
    if ($restricted && !empty($permissionsAccount['parent_account_id'])) {
        $permissionsAccount = $api->get("/accounts/{$permissionsAccount['parent_account_id']}");
    } else {
        $permissionsAccount = false;
    }
} while ($restricted && $permissionsAccount !== false);

/* find the most recent day's timestamp */
$response = $sql->query("
    SELECT * FROM `course_statistics`
        WHERE `course[account_id]` = '$account_id'" .
        ($restricted ? "AND `teacher[id]s` regexp 'a:[0-9]+:\{(i:[0-9]+;i:[0-9]+;)*i:[0-9]+;i:$user_id;'" : '') . "
        ORDER BY
            `timestamp` DESC
        LIMIT 1
");
$row = $response->fetch_assoc();
preg_match('/(\d{4,4}-\d{2,2}-\d{2,2}).*/', $row['timestamp'], $match);

$response = $sql->query("
	SELECT * FROM `course_statistics`
		WHERE
			`course[account_id]` = '$account_id' AND `timestamp` like '{$match[1]}%'" .
            ($restricted ? "AND `teacher[id]s` regexp 'a:[0-9]+:\{(i:[0-9]+;i:[0-9]+;)*i:[0-9]+;i:$user_id;'" : '') . "
		ORDER BY
			`timestamp` DESC,
			`course[name]` ASC
");

class Level {
namespace YourVendor;

class YourClass { }
	public static $GREATER_THAN = 0;
	public static $GREATER_THAN_OR_EQUAL = 1;
	public static $LESS_THAN_OR_EQUAL = 2;
	public static $LESS_THAN = 3;

	public $level;
	public $value;
	public $comparison;

	public function __construct($level, $value, $comparison) {
		$this->level = $level;
		$this->value = $value;
		$this->comparison = $comparison;
	}
}

$levels = array(
	'average_grading_turn_around' => array(
		'warning' => array(
			new Level(3, 14, Level::$GREATER_THAN),
			new Level(2, 7, Level::$GREATER_THAN)
		),
		'highlight' => array(
			new Level (3, 3, Level::$LESS_THAN),
			new Level (2, 7, Level::$LESS_THAN)
		)
	),
	'average_assignment_lead_time' => array(
		'warning' => array(
			new Level(3, 1, Level::$LESS_THAN),
			new Level(2, 2, Level::$LESS_THAN)
		),
		'highlight' => array(
			new Level(3, 10, Level::$GREATER_THAN),
			new Level(2, 7, Level::$GREATER_THAN)
		)
	),
	'average_submissions_graded' => array(
		'warning' => array(
			new Level(3, 0.5, Level::$LESS_THAN),
			new Level(2, 0.75, Level::$LESS_THAN)
		),
		'highlight' => array(
			new Level(3, 1.0, Level::$GREATER_THAN_OR_EQUAL),
			new Level(2, 0.9, Level::$GREATER_THAN)
		)
	),
	'dateless_assignment_count' => array(
		'warning' => array(
			new Level(3, 20, Level::$GREATER_THAN),
			new Level(2, 10, Level::$GREATER_THAN)
		),
		'highlight' => array(
			new Level(3, 1, Level::$LESS_THAN),
			new Level(2, 5, Level::$LESS_THAN)
		)
	),
	'gradeable_assignment_count' => array(
		'warning' => array(
			new Level(3, 0, Level::$LESS_THAN_OR_EQUAL)
		)
	),
	'graded_assignment_count' => array(
		'warning' => array(
			new Level(3, 0, Level::$LESS_THAN_OR_EQUAL)
		)
	),
	'created_after_due_count' => array(
		'warning' => array(
			new Level(3, 10, Level::$GREATER_THAN),
			new Level(2, 5, Level::$GREATER_THAN)
		),
		'highlight' => array(
			new Level(3, 1, Level::$LESS_THAN),
			new Level(2, 5, Level::$LESS_THAN)
		)
	),
	'zero_point_assignment_count' => array(
		'warning' => array(
			new Level(3, 10, Level::$GREATER_THAN_OR_EQUAL),
			new Level(2, 0, Level::$GREATER_THAN)
		)
	)
);

function getLevel($key, $value) {
	global $levels;
function myFunction($a, $b) {
    // Do something
}
	foreach ($levels[$key] as $mode => $modeLevels)
	{
		foreach ($modeLevels as $level) {
			$match = false;
			switch ($level->comparison) {
				case Level::$GREATER_THAN:
					$match = $value > $level->value;
					break;
				case Level::$GREATER_THAN_OR_EQUAL:
					$match = $value >= $level->value;
					break;
				case Level::$LESS_THAN:
					$match = $value < $level->value;
					break;
				case Level::$LESS_THAN_OR_EQUAL:
					$match = $value <= $level->value;
					break;
			}
			if ($match) {
				return " class=\"$mode level-{$level->level}\"";
			}
		}
	}
	return "";
}

$statistics = array();
$firstCourseId = false;
while (($statistic = $response->fetch_assoc()) && ($firstCourseId != $statistic['course[id]'])) {
	if (!$firstCourseId) {
		$firstCourseId = $statistic['course[id]'];
	}
	// FIXME this shouldn't be hard coded!
	$statistic['analytics_page'] = "{$_SESSION['canvasInstanceUrl']}/courses/{$statistic['course[id]']}/external_tools/1174";
	$statistics[] = $statistic;
}

$smarty->addStylesheet("{$metadata['APP_URL']}/css/department-summary.css");
if (isset($smarty->register_function)) {
	$smarty->register_function('getLevel', 'getLevel');
}
$smarty->assign('statistics', $statistics);
$smarty->assign('departments', $departments);
$smarty->display('department-summary.tpl');

?>



Push — master ( 505b4c...62d9c0 )

department-summary.php (4 issues)

Labels

Severity

Introduced By

Upgrade to new PHP Analysis Engine

1. Pass all data via parameters

2. Create a class that maintains your state

1			<?php
			0 ignored issues – show Coding Style Compatibility introduced 2017-03-27 12:19 UTC by Report Bug Copy Issue Report Show Similar Issues like this For compatibility and reusability of your code, PSR1 recommends that a file should introduce either new symbols (like classes, functions, etc.) or have side-effects (like outputting something, or including other files), but not both at the same time. The first symbol is defined on line 67 and the first side effect is on line 3. The PSR-1: Basic Coding Standard recommends that a file should either introduce new symbols, that is classes, functions, constants or similar, or have side effects. Side effects are anything that executes logic, like for example printing output, changing ini settings or writing to a file. The idea behind this recommendation is that merely auto-loading a class should not change the state of an application. It also promotes a cleaner style of programming and makes your code less prone to errors, because the logic is not spread out all over the place. To learn more about the PSR-1, please see the PHP-FIG site on the PSR-1. Loading history...
2
3			require_once 'common.inc.php';
4
5			use smtech\StMarksSmarty\StMarksSmarty;
6
7			/*
8			* FIXME this should not be hard-coded -- roles should be configurable.
9			*/
10			$unrestrictedRoles = [
11			'AccountAdmin',
12			'Dean',
13			'Department Chair'
14			];
15
16			$smarty->enable(StMarksSmarty::MODULE_SORTABLE);
17
18			$account_id = $toolProvider->user->getResourceLink()->settings['custom_canvas_account_id'];
19			$departments = $api->get("/accounts/$account_id");
20
21			/* verify user privileges */
22			$user_id = $toolProvider->user->getResourceLink()->settings['custom_canvas_user_id'];
23			$restricted = true;
24			$permissionsAccount = $departments;
25			do {
26			$roles = $api->get(
27			"/accounts/{$permissionsAccount['id']}/admins",
28			[
29			'user_id[]' => $user_id
30			]
31			);
32			foreach ($roles as $role) {
33			if (in_array($role['role'], $unrestrictedRoles)) {
34			$restricted = false;
35			break;
36			}
37			}
38			if ($restricted && !empty($permissionsAccount['parent_account_id'])) {
39			$permissionsAccount = $api->get("/accounts/{$permissionsAccount['parent_account_id']}");
40			} else {
41			$permissionsAccount = false;
42			}
43			} while ($restricted && $permissionsAccount !== false);
44
45			/* find the most recent day's timestamp */
46			$response = $sql->query("
47			SELECT * FROM `course_statistics`
48			WHERE `course[account_id]` = '$account_id'" .
49			($restricted ? "AND `teacher[id]s` regexp 'a:[0-9]+:\{(i:[0-9]+;i:[0-9]+;)*i:[0-9]+;i:$user_id;'" : '') . "
50			ORDER BY
51			`timestamp` DESC
52			LIMIT 1
53			");
54			$row = $response->fetch_assoc();
55			preg_match('/(\d{4,4}-\d{2,2}-\d{2,2}).*/', $row['timestamp'], $match);
56
57			$response = $sql->query("
58			SELECT * FROM `course_statistics`
59			WHERE
60			`course[account_id]` = '$account_id' AND `timestamp` like '{$match[1]}%'" .
61			($restricted ? "AND `teacher[id]s` regexp 'a:[0-9]+:\{(i:[0-9]+;i:[0-9]+;)*i:[0-9]+;i:$user_id;'" : '') . "
62			ORDER BY
63			`timestamp` DESC,
64			`course[name]` ASC
65			");
66
67			class Level {
			0 ignored issues – show Coding Style Compatibility introduced 2017-03-27 12:19 UTC by Report Bug Copy Issue Report Show Similar Issues like this PSR1 recommends that each class must be in a namespace of at least one level to avoid collisions. You can fix this by adding a namespace to your class: namespace YourVendor; class YourClass { } When choosing a vendor namespace, try to pick something that is not too generic to avoid conflicts with other libraries. Loading history...
68			public static $GREATER_THAN = 0;
69			public static $GREATER_THAN_OR_EQUAL = 1;
70			public static $LESS_THAN_OR_EQUAL = 2;
71			public static $LESS_THAN = 3;
72
73			public $level;
74			public $value;
75			public $comparison;
76
77			public function __construct($level, $value, $comparison) {
78			$this->level = $level;
79			$this->value = $value;
80			$this->comparison = $comparison;
81			}
82			}
83
84			$levels = array(
85			'average_grading_turn_around' => array(
86			'warning' => array(
87			new Level(3, 14, Level::$GREATER_THAN),
88			new Level(2, 7, Level::$GREATER_THAN)
89			),
90			'highlight' => array(
91			new Level (3, 3, Level::$LESS_THAN),
92			new Level (2, 7, Level::$LESS_THAN)
93			)
94			),
95			'average_assignment_lead_time' => array(
96			'warning' => array(
97			new Level(3, 1, Level::$LESS_THAN),
98			new Level(2, 2, Level::$LESS_THAN)
99			),
100			'highlight' => array(
101			new Level(3, 10, Level::$GREATER_THAN),
102			new Level(2, 7, Level::$GREATER_THAN)
103			)
104			),
105			'average_submissions_graded' => array(
106			'warning' => array(
107			new Level(3, 0.5, Level::$LESS_THAN),
108			new Level(2, 0.75, Level::$LESS_THAN)
109			),
110			'highlight' => array(
111			new Level(3, 1.0, Level::$GREATER_THAN_OR_EQUAL),
112			new Level(2, 0.9, Level::$GREATER_THAN)
113			)
114			),
115			'dateless_assignment_count' => array(
116			'warning' => array(
117			new Level(3, 20, Level::$GREATER_THAN),
118			new Level(2, 10, Level::$GREATER_THAN)
119			),
120			'highlight' => array(
121			new Level(3, 1, Level::$LESS_THAN),
122			new Level(2, 5, Level::$LESS_THAN)
123			)
124			),
125			'gradeable_assignment_count' => array(
126			'warning' => array(
127			new Level(3, 0, Level::$LESS_THAN_OR_EQUAL)
128			)
129			),
130			'graded_assignment_count' => array(
131			'warning' => array(
132			new Level(3, 0, Level::$LESS_THAN_OR_EQUAL)
133			)
134			),
135			'created_after_due_count' => array(
136			'warning' => array(
137			new Level(3, 10, Level::$GREATER_THAN),
138			new Level(2, 5, Level::$GREATER_THAN)
139			),
140			'highlight' => array(
141			new Level(3, 1, Level::$LESS_THAN),
142			new Level(2, 5, Level::$LESS_THAN)
143			)
144			),
145			'zero_point_assignment_count' => array(
146			'warning' => array(
147			new Level(3, 10, Level::$GREATER_THAN_OR_EQUAL),
148			new Level(2, 0, Level::$GREATER_THAN)
149			)
150			)
151			);
152
153			function getLevel($key, $value) {
154			global $levels;
			0 ignored issues – show Compatibility Best Practice introduced 2017-03-27 12:19 UTC by Report Bug Copy Issue Report Show Similar Issues like this Use of `global` functionality is not recommended; it makes your code harder to test, and less reusable. Instead of relying on `global` state, we recommend one of these alternatives: 1. Pass all data via parameters function myFunction($a, $b) { // Do something } 2. Create a class that maintains your state class MyClass { private $a; private $b; public function __construct($a, $b) { $this->a = $a; $this->b = $b; } public function myFunction() { // Do something } } Loading history...
155			foreach ($levels[$key] as $mode => $modeLevels)
156			{
157			foreach ($modeLevels as $level) {
158			$match = false;
159			switch ($level->comparison) {
160			case Level::$GREATER_THAN:
161			$match = $value > $level->value;
162			break;
163			case Level::$GREATER_THAN_OR_EQUAL:
164			$match = $value >= $level->value;
165			break;
166			case Level::$LESS_THAN:
167			$match = $value < $level->value;
168			break;
169			case Level::$LESS_THAN_OR_EQUAL:
170			$match = $value <= $level->value;
171			break;
172			}
173			if ($match) {
174			return " class=\"$mode level-{$level->level}\"";
175			}
176			}
177			}
178			return "";
179			}
180
181			$statistics = array();
182			$firstCourseId = false;
183			while (($statistic = $response->fetch_assoc()) && ($firstCourseId != $statistic['course[id]'])) {
184			if (!$firstCourseId) {
185			$firstCourseId = $statistic['course[id]'];
186			}
187			// FIXME this shouldn't be hard coded!
188			$statistic['analytics_page'] = "{$_SESSION['canvasInstanceUrl']}/courses/{$statistic['course[id]']}/external_tools/1174";
189			$statistics[] = $statistic;
190			}
191
192			$smarty->addStylesheet("{$metadata['APP_URL']}/css/department-summary.css");
193			if (isset($smarty->register_function)) {
194			$smarty->register_function('getLevel', 'getLevel');
195			}
196			$smarty->assign('statistics', $statistics);
197			$smarty->assign('departments', $departments);
198			$smarty->display('department-summary.tpl');
199
200			?>
			0 ignored issues – show Best Practice introduced 2017-03-27 12:19 UTC by Report Bug Copy Issue Report Show Similar Issues like this It is not recommended to use PHP's closing tag `?>` in files other than templates. Using a closing tag in PHP files that only contain PHP code is not recommended as you might accidentally add whitespace after the closing tag which would then be output by PHP. This can cause severe problems, for example headers cannot be sent anymore. A simple precaution is to leave off the closing tag as it is not required, and it also has no negative effects whatsoever. Loading history...
201

smtech / grading-analytics

Push — master ( 505b4c...62d9c0 )

department-summary.php (4 issues)

Labels

Severity

Introduced By

Upgrade to new PHP Analysis Engine

1. Pass all data via parameters

2. Create a class that maintains your state

Duplication Side-by-Side

Filter issues like