Skip to content

Commit 882a5b9

Browse files
bagofarmsbagofarms
authored
Merge pull request #370 from ucfopen/dev/2-3-3
Merging 2.3.3 into Master
2 parents 25cc9e0 + 58ea8fb commit 882a5b9

File tree

5 files changed

+74
-33
lines changed

5 files changed

+74
-33
lines changed

‎config/settings.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
define('ENV_TEST', 'test');
33
define('ENV_PROD', 'prod');
44
define('ENV_DEV', 'dev');
5-
define('UDOIT_VERSION', '2.3.2');
5+
define('UDOIT_VERSION', '2.3.3');
66

77
// SET UP AUTOLOADER (uses autoload rules from composer)
88
require_once(__DIR__.'/../vendor/autoload.php');

‎lib/Udoit.php‎

Lines changed: 46 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,12 @@ class Udoit
2424
{
2525
/**
2626
* Retrieves an entire group of content by type and scans it
27-
* @param string api_key - API Key of the user were acting as
28-
* @param string content_type - The group of content types we'll retrieve EX: 'pages' or 'assignments'
27+
* @param string $api_key API Key of the user were acting as
28+
* @param string $canvas_api_url The base URL of the Canvas API
29+
* @param string $course_id The Canvas course id
30+
* @param string $content_type The group of content types we'll retrieve EX: 'pages' or 'assignments'
2931
*
30-
* @return array - Results of the scan
32+
* @return array Results of the scan
3133
*/
3234
public static function retrieveAndScan($api_key, $canvas_api_url, $course_id, $content_type)
3335
{
@@ -107,9 +109,9 @@ public static function retrieveAndScan($api_key, $canvas_api_url, $course_id, $c
107109

108110
/**
109111
* Calls the Quail library to generate a UDOIT report
110-
* @param array $scanned_content - The items from whatever type of Canvas content was scanned
112+
* @param array $content_items The items from whatever type of Canvas content was scanned
111113
*
112-
* @return array - The report results
114+
* @return array The report results
113115
*/
114116
public static function scanContent(array $content_items)
115117
{
@@ -172,11 +174,12 @@ public static function scanContent(array $content_items)
172174

173175
/**
174176
* Communicates with the Canvas API to retrieve course content
175-
* @param string $api_key - Api key for the user we're acting as
176-
* @param string $canvas_api_url - Base uri for your canvas api
177-
* @param string $type - The type of course content to be scanned
177+
* @param string $api_key Api key for the user we're acting as
178+
* @param string $canvas_api_url Base uri for your canvas api
179+
* @param string $course_id The canvas course id
180+
* @param string $type The type of course content to be scanned
178181
*
179-
* @return array - The report results
182+
* @return array The report results
180183
*/
181184
public static function getCourseContent($api_key, $canvas_api_url, $course_id, $type)
182185
{
@@ -226,6 +229,14 @@ public static function getCourseContent($api_key, $canvas_api_url, $course_id, $
226229
break;
227230

228231
case 'files':
232+
// Gather all modules out here so we can attach them to files later
233+
$all_modules = static::apiGet("{$api_url}modules", $api_key)->send()->body;
234+
if (is_array($all_modules) || is_object($all_modules)) {
235+
foreach ($all_modules as $m) {
236+
$m->items = static::apiGet($m->items_url, $api_key)->send()->body;
237+
}
238+
}
239+
229240
$contents = static::apiGetAllLinks($api_key, "{$api_url}files?");
230241
foreach ($contents as $c) {
231242
if (substr($c->display_name, 0, 2) === '._') {
@@ -250,14 +261,10 @@ public static function getCourseContent($api_key, $canvas_api_url, $course_id, $
250261
// saves modules item is in for unscannable section
251262
unset($modules);
252263
$modules = [];
253-
$all_modules = static::apiGet("{$api_url}modules", $api_key)->send()->body;
254-
if (is_array($all_modules) || is_object($all_modules)) {
255-
foreach ($all_modules as $m) {
256-
$items = static::apiGet($m->items_url, $api_key)->send()->body;
257-
foreach ($items as $i) {
258-
if ($i->title == $c->display_name) {
259-
$modules[] = $m->name;
260-
}
264+
foreach ($all_modules as $m) {
265+
foreach ($m->items as $i) {
266+
if ($i->title == $c->display_name) {
267+
$modules[] = $m->name;
261268
}
262269
}
263270
}
@@ -357,7 +364,13 @@ public static function getCourseContent($api_key, $canvas_api_url, $course_id, $
357364
return $content_result;
358365
}
359366

360-
// abstraction of Request::get that adds the api key to the header
367+
/**
368+
* An abstraction of Request::get that adds the api key to the header
369+
* @param string $url The Canvas API URL
370+
* @param string $key The Canvas API key
371+
*
372+
* @return The results of the request
373+
**/
361374
protected static function apiGet($url, $key = null)
362375
{
363376
global $logger;
@@ -371,7 +384,13 @@ protected static function apiGet($url, $key = null)
371384
return $req;
372385
}
373386

374-
// get every page from the Canvas API till there's none left
387+
/**
388+
* Gets every page from the Canvas API till there's none left
389+
* @param string $api_key The Canvas API Key
390+
* @param string $url The Canvas API URL
391+
*
392+
* @return The results of the request
393+
**/
375394
protected static function apiGetAllLinks($api_key, $url)
376395
{
377396
global $logger;
@@ -387,6 +406,12 @@ protected static function apiGetAllLinks($api_key, $url)
387406
}
388407

389408
$links = static::apiParseLinks($response->headers->toArray()['link']);
409+
410+
if (empty($response->body)) {
411+
$logger->addError("Canvas API returned empty body for {$filtered_url}");
412+
break;
413+
}
414+
390415
foreach ($response->body as $thing) {
391416
$results[] = $thing;
392417
}
@@ -403,9 +428,9 @@ protected static function apiGetAllLinks($api_key, $url)
403428

404429
/**
405430
* Parses pagination links returned from Canvas
406-
* @param string $links - The pagination links
431+
* @param string $links The pagination links
407432
*
408-
* @return array - An array of the separated links
433+
* @return array An array of the separated links
409434
*/
410435
protected static function apiParseLinks($links)
411436
{

‎lib/UdoitJob.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ protected static function combineJobResults($job_group)
202202
if (!isset($error_summary[$item])) {
203203
$error_summary[$item] = $data;
204204
} else {
205-
$error_summary[$item][count] += $data[count];
205+
$error_summary[$item]['count'] += $data['count'];
206206
}
207207
}
208208
}
@@ -213,7 +213,7 @@ protected static function combineJobResults($job_group)
213213
if (!isset($suggestion_summary[$item])) {
214214
$suggestion_summary[$item] = $data;
215215
} else {
216-
$suggestion_summary[$item][count] += $data[count];
216+
$suggestion_summary[$item]['count'] += $data['count'];
217217
}
218218
}
219219
}

‎tests/BaseTest.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,10 @@ protected static function assertPDOStatementBindValueCalled($stmt, $key, $value)
7676
* both arguments incriment the return value on each call
7777
* First call returns $header_to_array_returns[0] and body_returns[0]
7878
* Yea, this is pretty cryptic and nested, mocking chained function calls php is gnarly
79+
*
80+
* @param array $header_to_array_returns Headers that should be returned with the request(s)
81+
* @param array $body_returns Bodies that should be returned with the requests(s)
82+
*
7983
* @return Object Mock Get Object created by Request::get()
8084
*/
8185
protected static function mockGetRequestResult(array $header_to_array_returns, array $body_returns)

‎tests/UdoitTest.php‎

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -257,11 +257,22 @@ public function testGetCoursContentGetsDiscussions()
257257
public function testGetCoursContentGetsUnscannableFiles()
258258
{
259259
$header_to_array_returns = [
260-
['link' => ''], // just one page
260+
['link' => ''],
261261
];
262262

263263
$body_returns = [
264-
[
264+
[ // Modules list
265+
(object) [
266+
'name' => "Test Module",
267+
'items_url' => "something/that/doesnt/matter",
268+
],
269+
],
270+
[ // Only a single item in the module
271+
(object) [
272+
'title' => "display_name_value.pdf",
273+
],
274+
],
275+
[ // Only a single file
265276
(object) [
266277
'display_name' => "display_name_value.pdf",
267278
'filename' => "filename.pdf",
@@ -274,6 +285,7 @@ public function testGetCoursContentGetsUnscannableFiles()
274285
];
275286

276287
$mock_get_result = self::mockGetRequestResult($header_to_array_returns, $body_returns);
288+
277289

278290
// overload Httpful\Request::get() to return $mock_get
279291
Mockery::mock('overload:Httpful\Request')
@@ -300,7 +312,8 @@ public function testGetCoursContentGetsFiles()
300312
];
301313

302314
$body_returns = [
303-
[
315+
[], // We don't care about modules, no modules means no items
316+
[ // An HTML file since we aren't testing unscannable files
304317
(object) [
305318
'display_name' => "display_name_value.html",
306319
'filename' => "filename.html",
@@ -309,7 +322,7 @@ public function testGetCoursContentGetsFiles()
309322
'html_url' => 'url_value',
310323
],
311324
],
312-
'<p>some html</p>',
325+
'<p>some html</p>', // Getting the html contents of the file
313326
[],
314327
];
315328

@@ -711,8 +724,9 @@ public function testRetrieveAndScanReturnsFiles()
711724
]';
712725

713726
$body_returns = [
714-
json_decode($api_body),
715-
'<img src="http://url.com/image.jpg"/>',
727+
[], // We don't care about Modules here
728+
json_decode($api_body), // one HTML file and one unscannable file
729+
'<img src="http://url.com/image.jpg"/>', // Contents of the HTML file
716730
[],
717731
];
718732

@@ -729,11 +743,10 @@ public function testRetrieveAndScanReturnsFiles()
729743

730744
self::assertEquals(0, $result['total_results']['errors']);
731745
self::assertEquals(0, $result['total_results']['warnings']);
732-
self::assertEquals(1, $result['total_results']['suggestions']);
746+
self::assertEquals(1, $result['total_results']['suggestions']); // This should be its own test
733747

734748
$res = $result['scan_results'];
735749

736-
self::assertCount(1, $res['unscannable']);
737750
self::assertArrayHasKey('unscannable', $res);
738751
self::assertCount(1, $res['unscannable']);
739752

@@ -756,7 +769,6 @@ public function testRetrieveAndScanReturnsFiles()
756769
self::assertArrayHasKey('suggestion', $res['files']['items'][0]);
757770

758771
self::assertCount(1, $res['files']['items'][0]['suggestion']);
759-
self::assertCount(1, $res['files']['items'][0]['suggestion']);
760772
}
761773

762774
public function testRetrieveAndScanReturnsModules()

0 commit comments

Comments
 (0)