Skip to content

Commit ffedc77

Browse files
sei-npachecosei-npacheco
authored
Warnings UI fixes (#13)
* fixing 2 warnings * change from invite to invitation * fixing strpos warning * saving progress for task bug * changing mod form ui for manage tasks page
1 parent bf2c14a commit ffedc77

File tree

11 files changed

+191
-133
lines changed

11 files changed

+191
-133
lines changed

‎classes/crucible.php

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -488,45 +488,47 @@ public function init_attempt() {
488488
*/
489489
public function filter_scenario_tasks($tasks, $isvisible = false, $isexecutable = false) {
490490
global $DB;
491-
491+
492492
if (is_null($tasks)) {
493493
return [];
494494
}
495-
495+
496496
$filtered = [];
497-
497+
498498
foreach ($tasks as $task) {
499499
$include = true;
500-
501-
// Filter by visibility from DB.
502-
if ($isvisible) {
503-
$sql = 'SELECT visible FROM {crucible_tasks} WHERE ' .
504-
$DB->sql_compare_text('name') . ' = ' . $DB->sql_compare_text(':name');
505-
506-
$rec = $DB->get_record_sql($sql, ['name' => $task->name]);
507-
508-
if (!$rec || empty($rec->visible)) {
509-
$include = false;
510-
}
500+
501+
// Look up task visibility, gradable, and points by name.
502+
$sql = 'SELECT visible, gradable, points FROM {crucible_tasks} WHERE ' .
503+
$DB->sql_compare_text('name') . ' = ' . $DB->sql_compare_text(':name');
504+
$rec = $DB->get_record_sql($sql, ['name' => $task->name]);
505+
506+
// Filter by visibility flag if required.
507+
if ($isvisible && (!$rec || empty($rec->visible))) {
508+
$include = false;
511509
}
512-
510+
513511
// Filter by executable flag from task object.
514512
if ($isexecutable && empty($task->userExecutable)) {
515513
$include = false;
516514
}
517-
515+
518516
// Include task if it passes all checks.
519517
if ($include) {
520-
if (!empty($task->userExecutable)) {
521-
$task->points = $task->totalScore ?? 0;
518+
// Set points if task is gradable in DB.
519+
if ($rec && !empty($rec->gradable)) {
520+
$task->points = $rec->points ?? 1; // default to 1 if missing
521+
} else {
522+
$task->points = 0; // not gradable
522523
}
524+
523525
$filtered[] = $task;
524526
}
525527
}
526-
528+
527529
usort($filtered, "tasksort");
528530
return $filtered;
529-
}
531+
}
530532

531533
/**
532534
* Retrieves all open attempts that do not belong to the current user.

‎classes/crucible_attempt.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,6 @@ public function save() {
183183
$newid = $DB->insert_record('crucible_attempts', $this->attempt);
184184
$this->attempt->id = $newid;
185185
} catch (\Exception $e) {
186-
var_dump($e);
187186
return false; // Return false on failure.
188187
}
189188
}

‎classes/crucible_tasks_form.php

Lines changed: 71 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -65,69 +65,85 @@ public function definition() {
6565
$mform->setType("id", PARAM_INT);
6666

6767
foreach ($this->_customdata['tasks'] as $task) {
68+
$taskprefix = "task_$index";
6869

69-
$group = [];
70+
$mform->addElement('header', "{$taskprefix}_header", $task->name);
7071

71-
$mform->addElement('header', 'task', $task->name);
72-
$group[] = $mform->createElement('hidden', 'name', $task->name);
73-
74-
$mform->addElement('html', '<div>Description<pre>' . $task->description . '</pre></div>');
75-
$group[] = $mform->createElement('hidden', 'description', $task->description);
76-
77-
$mform->addElement('html', '<div>Task ID<pre>' . $task->id . '</pre></div>');
78-
$group[] = $mform->createElement('hidden', 'dispatchtaskid', $task->id);
79-
80-
$mform->addElement('html', '<div>Scenario Template ID<pre>' . $task->scenarioTemplateId. '</pre></div>');
81-
$group[] = $mform->createElement('hidden', 'scenariotemplateid', $task->scenarioTemplateId);
82-
83-
$mform->addElement('html', '<div>VM Mask<pre>' . $task->vmMask. '</pre></div>');
84-
$input = isset($task->inputString) ? $task->inputString : '';
85-
$mform->addElement('html', '<div>Input String<pre>' . $input . '</pre></div>');
86-
$mform->addElement('html', '<div>Expected Output<pre>' . $task->expectedOutput . '</pre></div>');
87-
88-
$group[] = $mform->createElement('checkbox', 'visible', get_string('visible', 'crucible'));
89-
// $mform->addHelpButton('visible', 'visiblehelp', 'crucible');
90-
91-
$group[] = $mform->createElement('checkbox', 'gradable', get_string('gradable', 'crucible'));
92-
// $mform->addHelpButton('gradble', 'gradablehelp', 'crucible');
93-
94-
$group[] = $mform->createElement('checkbox', 'multiple', get_string('multiple', 'crucible'));
95-
// $mform->addHelpButton('mutiple', 'multiplehelp', 'crucible');
96-
97-
$group[] = $mform->createElement('text', 'points', get_string('points', 'crucible'), ['size' => '5']);
98-
// $mform->addHelpButton('points', 'pointshelp', 'crucible');
99-
100-
$group[] = $mform->createElement('html', get_string('points', 'crucible'));
101-
102-
$mform->addGroup($group, "options-$index", 'Options', [' '], true);
103-
104-
$mform->setType("options-" . $index . "[name]", PARAM_RAW);
105-
$mform->setType("options-" . $index . "[description]", PARAM_RAW);
106-
$mform->setType("options-" . $index . "[dispatchtaskid]", PARAM_ALPHANUMEXT);
107-
$mform->setType("options-" . $index . "[scenariotemplateid]", PARAM_ALPHANUMEXT);
108-
$mform->setType("options-" . $index . "[points]", PARAM_INT);
109-
110-
$rec = $DB->get_record_sql('SELECT * from {crucible_tasks} WHERE '
111-
. $DB->sql_compare_text('dispatchtaskid') . ' = '
112-
. $DB->sql_compare_text(':dispatchtaskid'), ['dispatchtaskid' => $task->id]);
113-
114-
if ($rec === false) {
115-
$mform->setDefault("options-" . $index . "[visible]", 1);
116-
$mform->setDefault("options-" . $index . "[gradable]", 1);
117-
$mform->setDefault("options-" . $index . "[multiple]", 1);
118-
$mform->setDefault("options-" . $index . "[points]", 1);
72+
// HTML display of description.
73+
if (!empty($task->description)) {
74+
$mform->addElement('html', '<div><strong>Description</strong><pre>' . s($task->description) . '</pre></div>');
75+
}
76+
77+
if (!empty($task->id)) {
78+
$mform->addElement('html', '<div><strong>Task ID</strong><pre>' . s($task->id) . '</pre></div>');
79+
}
80+
81+
if (!empty($task->scenarioTemplateId)) {
82+
$mform->addElement('html', '<div><strong>Scenario Template ID</strong><pre>' . s($task->scenarioTemplateId) . '</pre></div>');
83+
}
84+
85+
if (!empty($task->vmMask)) {
86+
$mform->addElement('html', '<div><strong>VM Mask</strong><pre>' . s($task->vmMask) . '</pre></div>');
87+
}
88+
89+
if (!empty($task->inputString)) {
90+
$mform->addElement('html', '<div><strong>Input String</strong><pre>' . s($task->inputString) . '</pre></div>');
91+
}
92+
93+
if (!empty($task->expectedOutput)) {
94+
$mform->addElement('html', '<div><strong>Expected Output</strong><pre>' . s($task->expectedOutput) . '</pre></div>');
95+
}
96+
97+
// Hidden fields to track values.
98+
$mform->addElement('hidden', "{$taskprefix}_name", $task->name);
99+
$mform->setType("{$taskprefix}_name", PARAM_RAW);
100+
101+
$mform->addElement('hidden', "{$taskprefix}_description", $task->description);
102+
$mform->setType("{$taskprefix}_description", PARAM_RAW);
103+
104+
$mform->addElement('hidden', "{$taskprefix}_dispatchtaskid", $task->id);
105+
$mform->setType("{$taskprefix}_dispatchtaskid", PARAM_ALPHANUMEXT);
106+
107+
$mform->addElement('hidden', "{$taskprefix}_scenariotemplateid", $task->scenarioTemplateId);
108+
$mform->setType("{$taskprefix}_scenariotemplateid", PARAM_ALPHANUMEXT);
109+
110+
// Input fields per task.
111+
$mform->addElement('checkbox', "{$taskprefix}_visible", get_string('visible', 'crucible'));
112+
$mform->addElement('checkbox', "{$taskprefix}_gradable", get_string('gradable', 'crucible'));
113+
$mform->addElement('checkbox', "{$taskprefix}_multiple", get_string('multiple', 'crucible'));
114+
$mform->addElement('html', '<div><strong>' . get_string('points', 'crucible') . '</strong></div>');
115+
$mform->addElement('text', "{$taskprefix}_points", '', ['size' => '5']);
116+
$mform->setType("{$taskprefix}_points", PARAM_INT);
117+
$mform->disabledIf("{$taskprefix}_points", "{$taskprefix}_gradable");
118+
119+
// Help Buttons
120+
$mform->addHelpButton("{$taskprefix}_visible", 'visible', 'crucible');
121+
$mform->addHelpButton("{$taskprefix}_gradable", 'gradable', 'crucible');
122+
$mform->addHelpButton("{$taskprefix}_multiple", 'multiple', 'crucible');
123+
$mform->addHelpButton("{$taskprefix}_points", 'points', 'crucible');
124+
125+
// Set defaults based on DB record.
126+
$rec = $DB->get_record_sql(
127+
'SELECT * FROM {crucible_tasks} WHERE ' . $DB->sql_compare_text('dispatchtaskid') . ' = ' . $DB->sql_compare_text(':dispatchtaskid'),
128+
['dispatchtaskid' => $task->id]
129+
);
130+
131+
if ($rec) {
132+
$mform->setDefault("{$taskprefix}_visible", $rec->visible);
133+
$mform->setDefault("{$taskprefix}_gradable", $rec->gradable);
134+
$mform->setDefault("{$taskprefix}_multiple", $rec->multiple);
135+
$mform->setDefault("{$taskprefix}_points", $rec->points);
119136
} else {
120-
$mform->setDefault("options-" . $index . "[visible]", $rec->visible);
121-
$mform->setDefault("options-" . $index . "[gradable]", $rec->gradable);
122-
$mform->setDefault("options-" . $index . "[multiple]", $rec->multiple);
123-
$mform->setDefault("options-" . $index . "[points]", $rec->points);
137+
$mform->setDefault("{$taskprefix}_visible", 1);
138+
$mform->setDefault("{$taskprefix}_gradable", 1);
139+
$mform->setDefault("{$taskprefix}_multiple", 1);
140+
$mform->setDefault("{$taskprefix}_points", 1);
124141
}
125142

126-
$mform->disabledIf("options-" . $index . "[points]", "options-" .$index . "[gradable]");
127-
128143
$index++;
129144
}
130145
$this->add_action_buttons();
146+
131147
}
132148

133149
/**

‎classes/utils/grade.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ public function process_attempt($attempt) {
156156

157157
// Update grades to gradebookapi.
158158
foreach ($userids as $userid) {
159-
$updated = crucible_update_grades($this->crucible->crucible, $userid, false, $grades[$userid]);
159+
$updated = crucible_update_grades($this->crucible->crucible, $userid, false);
160160

161161
if ($updated === GRADE_UPDATE_FAILED) {
162162
$transaction->rollback(new \Exception('Unable to save grades to gradebook'));

‎lang/en/crucible.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,14 @@
149149
$string['gradable'] = 'Gradable';
150150
$string['multiple'] = 'Multiple VMs';
151151
$string['points'] = 'Points';
152+
$string['visible'] = 'Visible';
153+
$string['visible_help'] = 'If checked, this task will be visible to students.';
154+
$string['gradable'] = 'Gradable';
155+
$string['gradable_help'] = "If checked, this task will contribute to the user's score.";
156+
$string['multiple'] = 'Multiple VMs';
157+
$string['multiple_help'] = 'If checked, this task can be executed on multiple virtual machines whose names match a specified string or mask.';
158+
$string['points'] = 'Points';
159+
$string['points_help'] = 'Set the number of points this task is worth if gradable is enabled.';
152160

153161
// Roles
154162
$string['crucible:manage'] = 'Manage Crucible activities';

‎lib.php

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -342,14 +342,41 @@ function crucible_update_grades($crucible, $userid = 0, $nullifnone = true) {
342342
require_once($CFG->libdir . '/gradelib.php');
343343

344344
$grades = [];
345-
foreach ($userid as $user) {
346-
$rawgrade = crucible_get_user_grades($crucible, $userid);
347-
$grade = new stdClass();
348-
$grade->userid = $userid;
349-
$grade->rawgrade = $rawgrade;
350-
$grades[$user] = $grade;
345+
346+
// Case 1: All users.
347+
if ($userid === 0) {
348+
$grades = crucible_get_user_grades($crucible, 0);
349+
}
350+
351+
// Case 2: Array of user IDs.
352+
else if (is_array($userid)) {
353+
foreach ($userid as $uid) {
354+
$usergrades = crucible_get_user_grades($crucible, $uid);
355+
if (!empty($usergrades[$uid])) {
356+
$grades[$uid] = $usergrades[$uid];
357+
} else if ($nullifnone) {
358+
$grade = new stdClass();
359+
$grade->userid = $uid;
360+
$grade->rawgrade = null;
361+
$grades[$uid] = $grade;
362+
}
363+
}
351364
}
352-
return crucible_grade_item_update($crucible, $grades);
365+
366+
// Case 3: Single user ID.
367+
else {
368+
$usergrades = crucible_get_user_grades($crucible, $userid);
369+
if (!empty($usergrades[$userid])) {
370+
$grades[$userid] = $usergrades[$userid];
371+
} else if ($nullifnone) {
372+
$grade = new stdClass();
373+
$grade->userid = $userid;
374+
$grade->rawgrade = null;
375+
$grades[$userid] = $grade;
376+
}
377+
}
378+
379+
return crucible_grade_item_update($crucible, $grades);
353380
}
354381

355382
/**

‎renderer.php

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,6 @@ public function display_tasks($tasks) {
343343
];
344344

345345
foreach ($tasks as $task) {
346-
// var_dump($task);
347346
$rowdata = [];
348347
// $rowdata[] = $task->id;
349348
$rowdata[] = $task->name;
@@ -352,7 +351,11 @@ public function display_tasks($tasks) {
352351
} else {
353352
$rowdata[] = "No description available";
354353
}
355-
$rowdata[] = $task->score;
354+
if (empty($task->points) && isset($task->score)) {
355+
$rowdata[] = $task->score;
356+
} else {
357+
$rowdata[] = $task->points;
358+
}
356359
$data->tabledata[] = $rowdata;
357360
}
358361

@@ -385,7 +388,6 @@ public function display_tasks_form($tasks) {
385388
];
386389

387390
foreach ($tasks as $task) {
388-
// var_dump($task);
389391
$rowdata = [];
390392
$rowdata[] = $task->id;
391393
$rowdata[] = $task->name;
@@ -473,7 +475,7 @@ public function display_results($tasks, $review = false) {
473475
if (isset($task->totalScoreEarned)) {
474476
$rowdata->score = $task->totalScoreEarned;
475477
}
476-
$rowdata->points = $task->totalScore;
478+
$rowdata->points = $task->points;
477479
$data->tabledata[] = $rowdata;
478480
}
479481

@@ -535,7 +537,11 @@ public function display_results_detail($a, $tasks) {
535537
if (isset($task->score)) {
536538
$rowdata->score = $task->score;
537539
}
538-
$rowdata->points = $task->points;
540+
if (empty($task->points) && isset($task->score)) {
541+
$rowdata->points = $task->score;
542+
} else {
543+
$rowdata->points = $task->points;
544+
}
539545
$data->tabledata[] = $rowdata;
540546
}
541547

0 commit comments

Comments
 (0)