From 22a6015bd5f35b1ef17166ac863e193dcaa2f676 Mon Sep 17 00:00:00 2001
From: Dan Marsden <dan@danmarsden.com>
Date: Wed, 30 Sep 2020 14:52:59 +1300
Subject: [PATCH] Tidy up PR from Nick

coding guideline stuff/renderers/using own editing button
---
 classes/event/session_report_updated.php |  3 +-
 lang/en/attendance.php                   |  1 +
 locallib.php                             |  6 +-
 renderables.php                          | 15 ++--
 renderer.php                             | 88 ++++++++++--------------
 styles.css                               |  3 +-
 view.php                                 | 38 ++++++++--
 7 files changed, 83 insertions(+), 71 deletions(-)

diff --git a/classes/event/session_report_updated.php b/classes/event/session_report_updated.php
index 408a369..1c649ec 100644
--- a/classes/event/session_report_updated.php
+++ b/classes/event/session_report_updated.php
@@ -35,7 +35,6 @@ defined('MOODLE_INTERNAL') || die();
  *      string mode Mode of the report updated.
  * }
  * @package    mod_attendance
- * @since      Moodle 2.7
  * @copyright  2013 onwards Dan Marsden
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
@@ -47,7 +46,7 @@ class session_report_updated extends \mod_attendance\event\session_report_viewed
     protected function init() {
         $this->data['crud'] = 'u';
         $this->data['edulevel'] = self::LEVEL_TEACHING;
-        // objecttable and objectid can't be meaningfully specified
+        // Objecttable and objectid can't be meaningfully specified.
     }
 
     /**
diff --git a/lang/en/attendance.php b/lang/en/attendance.php
index 018fb28..7a5b5ef 100644
--- a/lang/en/attendance.php
+++ b/lang/en/attendance.php
@@ -335,6 +335,7 @@ $string['noabsentstatusset'] = 'The status set in use does not have a status to
 $string['noattendanceusers'] = 'It is not possible to export any data as there are no students enrolled in the course.';
 $string['noattforuser'] = 'No attendance records exist for the user';
 $string['noautomark'] = 'Disabled';
+$string['nocapabilitytotakethisattendance'] = 'You tried to change the attendance of a session with the cmid: {$a} that you do not have permission to modify.';
 $string['nodescription'] = 'Regular class session';
 $string['noeventstoreset'] = 'There are no calendar events that require an update.';
 $string['nogroups'] = 'You can\'t add group sessions. No groups exists in course.';
diff --git a/locallib.php b/locallib.php
index ff8bc1e..7764bef 100644
--- a/locallib.php
+++ b/locallib.php
@@ -145,16 +145,14 @@ function attendance_get_user_sessions_log_full($userid, $pageparams) {
 
     if ($pageparams->groupby === 'date') {
         $ordersql = "ats.sessdate ASC, c.fullname ASC, att.name ASC, att.id ASC";
-    }
-    else {
+    } else {
         $ordersql = "c.fullname ASC, att.name ASC, att.id ASC, ats.sessdate ASC";
     }
 
     // WHERE clause is important:
     // gm.userid not null => get unmarked attendances for user's current groups
     // ats.groupid 0 => get all sessions that are for all students enrolled in course
-    // al.id not null => get all marked sessions whether or not user currently still in group
-    //
+    // al.id not null => get all marked sessions whether or not user currently still in group.
     $sql = "SELECT ats.id, ats.groupid, ats.sessdate, ats.duration, ats.description, ats.statusset,
                    al.statusid, al.remarks, ats.studentscanmark, ats.autoassignstatus,
                    ats.preventsharedip, ats.preventsharediptime,
diff --git a/renderables.php b/renderables.php
index 250d685..6246025 100644
--- a/renderables.php
+++ b/renderables.php
@@ -574,14 +574,12 @@ class attendance_user_data implements renderable {
         $now = time();
         $sesslog = array();
         $formdata = (array)$formdata;
-        $dbstudlogs = array();
         $updatedsessions = array();
         $sessionatt = array();
 
         foreach ($formdata as $key => $value) {
             // Look at Remarks field because the user options may not be passed if empty.
             if (substr($key, 0, 7) == 'remarks') {
-                $formlog = array();
                 $parts = explode('sess', substr($key, 7));
                 $stid = $parts[0];
                 if (!(is_numeric($stid))) { // Sanity check on $stid.
@@ -596,13 +594,15 @@ class attendance_user_data implements renderable {
                 $context = context_module::instance($dbsession->cmid);
                 if (!has_capability('mod/attendance:takeattendances', $context)) {
                     // How do we tell user about this?
-                    error_log("User without capability tried to take attendance for context, cmid:", $dbsession->cmid);
+                    \core\notification::warning(get_string("nocapabilitytotakethisattendance", "attendance", $dbsession->cmid));
                     continue;
                 }
 
                 $formkey = 'user'.$stid.'sess'.$sessid;
                 $attid = $dbsession->attendanceid;
-                $statusset = array_filter($this->statuses[$attid], function($x) use($dbsession) { return $x->setnumber === $dbsession->statusset; });
+                $statusset = array_filter($this->statuses[$attid], function($x) use($dbsession) {
+                                                                       return $x->setnumber === $dbsession->statusset;
+                                                                   });
                 $sessionatt[$sessid] = $attid;
                 $formlog = new stdClass();
                 if (array_key_exists($formkey, $formdata) && is_numeric($formdata[$formkey])) {
@@ -624,8 +624,9 @@ class attendance_user_data implements renderable {
 
         $updateatts = array();
         foreach ($sesslog as $stid => $userlog) {
-            $dbstudlog = $DB->get_records('attendance_log', array('studentid' => $stid), '', 'sessionid,statusid,remarks,id,statusset');
-            foreach($userlog as $log) {
+            $dbstudlog = $DB->get_records('attendance_log', array('studentid' => $stid), '',
+                'sessionid,statusid,remarks,id,statusset');
+            foreach ($userlog as $log) {
                 if (array_key_exists($log->sessionid, $dbstudlog)) {
                     $attid = $sessionatt[$log->sessionid];
                     // Check if anything important has changed before updating record.
@@ -663,7 +664,7 @@ class attendance_user_data implements renderable {
 
         if (!empty($updateatts)) {
             $attendancegrade = $DB->get_records_list('attendance', 'id', array_keys($updateatts), '', 'id, grade');
-            foreach($updateatts as $attid => $updateusers) {
+            foreach ($updateatts as $attid => $updateusers) {
                 if ($attendancegrade[$attid] != 0) {
                     attendance_update_users_grades_by_id($attid, $grade, $updateusers);
                 }
diff --git a/renderer.php b/renderer.php
index 9bfde85..759c091 100644
--- a/renderer.php
+++ b/renderer.php
@@ -1009,6 +1009,7 @@ class mod_attendance_renderer extends plugin_renderer_base {
      */
     private function construct_take_session_controls(attendance_take_data $takedata, $user) {
         $celldata = array();
+        $celldata['remarks'] = '';
         if ($user->enrolmentend and $user->enrolmentend < $takedata->sessioninfo->sessdate) {
             $celldata['text'] = get_string('enrolmentend', 'attendance', userdate($user->enrolmentend, '%d.%m.%Y'));
             $celldata['colspan'] = count($takedata->statuses) + 1;
@@ -1053,7 +1054,7 @@ class mod_attendance_renderer extends plugin_renderer_base {
             if ($takedata->pageparams->viewmode == mod_attendance_take_page_params::SORTED_GRID) {
                 $input = html_writer::empty_tag('br').$input;
             }
-            $celldata['text'][] = $input;
+            $celldata['remarks'] = $input;
 
             if ($user->enrolmentstart > $takedata->sessioninfo->sessdate + $takedata->sessioninfo->duration) {
                 $celldata['warning'] = get_string('enrolmentstart', 'attendance',
@@ -1392,7 +1393,7 @@ class mod_attendance_renderer extends plugin_renderer_base {
      * @return string
      */
     private function construct_user_allsessions_log(attendance_user_data $userdata) {
-        global $OUTPUT, $USER;
+        global $USER;
 
         $allsessions = new stdClass();
 
@@ -1402,9 +1403,6 @@ class mod_attendance_renderer extends plugin_renderer_base {
             $shortform = true;
         }
 
-        $context = context_module::instance($userdata->filtercontrols->cm->id);
-
-        // 'course', 'activity', 'date'
         $groupby = $userdata->pageparams->groupby;
 
         $table = new html_table();
@@ -1418,7 +1416,7 @@ class mod_attendance_renderer extends plugin_renderer_base {
 
         // If grouping by date, we need some form of date up front.
         // Only need course column if we are not using course to group
-        // (currently date is only option which does not use course)
+        // (currently date is only option which does not use course).
         if ($groupby === 'date') {
             $table->head[] = '';
             $table->align[] = 'left';
@@ -1448,7 +1446,7 @@ class mod_attendance_renderer extends plugin_renderer_base {
             }
         }
 
-        // Need activity column unless we are using activity to group
+        // Need activity column unless we are using activity to group.
         if ($groupby !== 'activity') {
             $table->head[] = get_string('pluginname', 'mod_attendance');
             $table->align[] = 'left';
@@ -1457,7 +1455,7 @@ class mod_attendance_renderer extends plugin_renderer_base {
             $colcount++;
         }
 
-        // If grouping by date, it belongs up front rather than here
+        // If grouping by date, it belongs up front rather than here.
         if ($groupby !== 'date') {
             $table->head[] = get_string('date');
             $table->align[] = 'left';
@@ -1466,7 +1464,7 @@ class mod_attendance_renderer extends plugin_renderer_base {
             $colcount++;
         }
 
-        // use "session" instead of "description"
+        // Use "session" instead of "description".
         $table->head[] = get_string('session', 'attendance');
         $table->align[] = 'left';
         $table->colclasses[] = 'desccol';
@@ -1476,12 +1474,12 @@ class mod_attendance_renderer extends plugin_renderer_base {
         if (!$shortform) {
             $table->head[] = get_string('sessiontypeshort', 'attendance');
             $table->align[] = '';
-            $table->size[] = '1px';
+            $table->size[] = '*';
             $table->colclasses[] = '';
             $colcount++;
         }
 
-        if (!empty($USER->editing)) {
+        if (!empty($USER->attendanceediting)) {
             $table->head[] = get_string('status', 'attendance');
             $table->align[] = 'center';
             $table->colclasses[] = 'statuscol';
@@ -1561,8 +1559,8 @@ class mod_attendance_renderer extends plugin_renderer_base {
                         );
                     }
                     $statussetmaxpoints = $statusmaxpoints[$sess->attendanceid];
-                    // ensure all possible acronyms for current sess's statusset are available as
-                    // keys in status array for period
+                    // Ensure all possible acronyms for current sess's statusset are available as
+                    // keys in status array for period.
                     //
                     // A bit yucky because we can't tell whether we've seen statusset before, and
                     // we usually will have, so much wasted spinning.
@@ -1576,7 +1574,7 @@ class mod_attendance_renderer extends plugin_renderer_base {
                             }
                         }
                     }
-                    // array_key_exists check is for hidden statuses
+                    // The array_key_exists check is for hidden statuses.
                     if (isset($sess->statusid) && array_key_exists($sess->statusid, $userdata->statuses[$sess->attendanceid])) {
                         $status = $userdata->statuses[$sess->attendanceid][$sess->statusid];
                         $stats['date'][$weekformat]['statuses'][$status->acronym]['count']++;
@@ -1588,9 +1586,8 @@ class mod_attendance_renderer extends plugin_renderer_base {
                     }
                     $stats['date'][$weekformat]['maxpoints'] += $statussetmaxpoints[$sess->statusset];
                     $stats['overall']['maxpoints'] += $statussetmaxpoints[$sess->statusset];
-                }
-                else {
-                    // By course and perhaps activity
+                } else {
+                    // By course and perhaps activity.
                     if (
                         ($sess->courseid != $lastgroup[0]) ||
                         ($groupby === 'activity' && $sess->cmid != $lastgroup[1])
@@ -1613,7 +1610,7 @@ class mod_attendance_renderer extends plugin_renderer_base {
                         );
                     }
                     $statussetmaxpoints = $statusmaxpoints[$sess->attendanceid];
-                    // ensure all possible acronyms for current sess's statusset are available as
+                    // Ensure all possible acronyms for current sess's statusset are available as
                     // keys in status array for course
                     //
                     // A bit yucky because we can't tell whether we've seen statusset before, and
@@ -1628,7 +1625,7 @@ class mod_attendance_renderer extends plugin_renderer_base {
                             }
                         }
                     }
-                    // array_key_exists check is for hidden statuses
+                    // The array_key_exists check is for hidden statuses.
                     if (isset($sess->statusid) && array_key_exists($sess->statusid, $userdata->statuses[$sess->attendanceid])) {
                         $status = $userdata->statuses[$sess->attendanceid][$sess->statusid];
                         $stats['course'][$sess->courseid]['statuses'][$status->acronym]['count']++;
@@ -1652,7 +1649,7 @@ class mod_attendance_renderer extends plugin_renderer_base {
                         );
                     }
                     $statussetmaxpoints = $statusmaxpoints[$sess->attendanceid];
-                    // ensure all possible acronyms for current sess's statusset are available as
+                    // Ensure all possible acronyms for current sess's statusset are available as
                     // keys in status array for period
                     //
                     // A bit yucky because we can't tell whether we've seen statusset before, and
@@ -1667,7 +1664,7 @@ class mod_attendance_renderer extends plugin_renderer_base {
                             }
                         }
                     }
-                    // array_key_exists check is for hidden statuses
+                    // The array_key_exists check is for hidden statuses.
                     if (isset($sess->statusid) && array_key_exists($sess->statusid, $userdata->statuses[$sess->attendanceid])) {
                         $status = $userdata->statuses[$sess->attendanceid][$sess->statusid];
                         $stats['activity'][$sess->cmid]['statuses'][$status->acronym]['count']++;
@@ -1687,7 +1684,6 @@ class mod_attendance_renderer extends plugin_renderer_base {
 
         $points = $stats['overall']['points'];
         $maxpoints = $stats['overall']['maxpointstodate'];
-        $summary = '';
         $summarytable = new html_table();
         $summarytable->attributes['class'] = 'generaltable table-bordered table-condensed';
         $row = new html_table_row();
@@ -1702,8 +1698,7 @@ class mod_attendance_renderer extends plugin_renderer_base {
             $row->cells[] = $status['count'];
             $summarytable->data[] = $row;
         }
-        /* $row = new html_table_row(); */
-        /* $summarytable->data[] = $row; */
+
         $row = new html_table_row();
         if ($maxpoints !== 0) {
             $pctodate = format_float( $points * 100 / $maxpoints);
@@ -1735,7 +1730,7 @@ class mod_attendance_renderer extends plugin_renderer_base {
                 $row->cells[] = $cell;
                 $week = date("W", $sess->sessdate);
                 $year = date("Y", $sess->sessdate);
-                // ISO week starts on day 1, Monday
+                // ISO week starts on day 1, Monday.
                 $weekstart = date_timestamp_get(date_isodate_set(date_create(), $year, $week, 1));
                 $dmywformat = get_string('strftimedmyw', 'attendance');
                 $cell = new html_table_cell(get_string('weekcommencing', 'attendance') . ": " . userdate($weekstart, $dmywformat));
@@ -1769,15 +1764,14 @@ class mod_attendance_renderer extends plugin_renderer_base {
                 $row->cells[] = $cell;
                 $table->data[] = $row;
                 $lastgroup[0] = date("YW", $weekstart);
-            }
-            else {
+            } else {
                 if ($groupby === 'course' || $sess->courseid !== $lastgroup[0]) {
                     $row = new html_table_row();
                     $row->attributes['class'] = 'grouper';
                     $cell = new html_table_cell();
                     $cell->rowspan = count($group) + 2;
                     if ($groupby === 'activity') {
-                        $headcell = $cell; // keep ref to be able to adjust rowspan later
+                        $headcell = $cell; // Keep ref to be able to adjust rowspan later.
                         $cell->rowspan += 2;
                         $row->cells[] = $cell;
                         $cell = new html_table_cell();
@@ -1866,13 +1860,9 @@ class mod_attendance_renderer extends plugin_renderer_base {
             foreach ($group as $sess) {
                 $row = new html_table_row();
 
-                // partialdate? / course? / activity? / date? / session / type / status / points / remarks / action
-                //
-
                 // If grouping by date, we need some form of date up front.
                 // Only need course column if we are not using course to group
-                // (currently date is only option which does not use course)
-                //
+                // (currently date is only option which does not use course).
                 if ($groupby === 'date') {
                     // What part of date do we want if grouped by it already?
                     $row->cells[] = userdate($sess->sessdate, get_string('strftimedmw', 'attendance')) .
@@ -1882,8 +1872,7 @@ class mod_attendance_renderer extends plugin_renderer_base {
                     $row->cells[] = html_writer::link($courseurl, $sess->cname);
                 }
 
-                // Need activity column unless we are using activity to group
-                //
+                // Need activity column unless we are using activity to group.
                 if ($groupby !== 'activity') {
                     $attendanceurl = new moodle_url('/mod/attendance/view.php', array('id' => $sess->cmid,
                                                                                       'studentid' => $userdata->user->id,
@@ -1891,8 +1880,7 @@ class mod_attendance_renderer extends plugin_renderer_base {
                     $row->cells[] = html_writer::link($attendanceurl, $sess->attname);
                 }
 
-                // If grouping by date, it belongs up front rather than here
-                //
+                // If grouping by date, it belongs up front rather than here.
                 if ($groupby !== 'date') {
                     $row->cells[] = userdate($sess->sessdate, get_string('strftimedmyw', 'attendance')) .
                         " ". $this->construct_time($sess->sessdate, $sess->duration);
@@ -1918,13 +1906,10 @@ class mod_attendance_renderer extends plugin_renderer_base {
                     $row->cells[] = html_writer::tag('nobr', $sessiontypeshort);
                 }
 
-                if (!empty($USER->editing)) {
+                if (!empty($USER->attendanceediting)) {
                     $context = context_module::instance($sess->cmid);
                     if (has_capability('mod/attendance:takeattendances', $context)) {
-                        // TODO: add ability to edit attendance here
-                        $celltext = '';
-
-                        // takedata needs:
+                        // Takedata needs:
                         // sessioninfo->sessdate
                         // sessioninfo->duration
                         // statuses
@@ -1938,7 +1923,7 @@ class mod_attendance_renderer extends plugin_renderer_base {
                         // enrolmentstart
                         // enrolmentend
                         // enrolmentstatus
-                        // id
+                        // id.
 
                         $nastyhack = new ReflectionClass('attendance_take_data');
                         $takedata = $nastyhack->newInstanceWithoutConstructor();
@@ -1963,10 +1948,13 @@ class mod_attendance_renderer extends plugin_renderer_base {
                         }
 
                         $cell = new html_table_cell($celltext);
-                        $cell->colspan = 2;
                         $row->cells[] = $cell;
-                    }
-                    else {
+
+                        $celltext = empty($ucdata['remarks']) ? '' : $ucdata['remarks'];
+                        $cell = new html_table_cell($celltext);
+                        $row->cells[] = $cell;
+
+                    } else {
                         if (!empty($sess->statusid)) {
                             $status = $userdata->statuses[$sess->attendanceid][$sess->statusid];
                             $row->cells[] = $status->description;
@@ -2014,26 +2002,26 @@ class mod_attendance_renderer extends plugin_renderer_base {
             }
         }
 
-        if (!empty($USER->editing)) {
+        if (!empty($USER->attendanceediting)) {
             $row = new html_table_row();
             $params = array(
                 'type'  => 'submit',
                 'class' => 'btn btn-primary',
                 'value' => get_string('save', 'attendance'));
             $cell = new html_table_cell(html_writer::tag('center', html_writer::empty_tag('input', $params)));
-            $cell->colspan = $colcount + (($groupby == 'activity')? 2 : 1);
+            $cell->colspan = $colcount + (($groupby == 'activity') ? 2 : 1);
             $row->cells[] = $cell;
             $table->data[] = $row;
         }
 
         $logtext = html_writer::table($table);
 
-        if (!empty($USER->editing)) {
+        if (!empty($USER->attendanceediting)) {
             $formtext = html_writer::start_div('no-overflow');
             $formtext .= $logtext;
             $formtext .= html_writer::input_hidden_params($userdata->url(array('sesskey' => sesskey())));
             $formtext .= html_writer::end_div();
-            // could use userdata->urlpath if not private or userdata->url_path() if existed, but '' turns
+            // Could use userdata->urlpath if not private or userdata->url_path() if existed, but '' turns
             // out to DTRT.
             $logtext = html_writer::tag('form', $formtext, array('method' => 'post', 'action' => '',
                                                                  'id' => 'attendancetakeform'));
diff --git a/styles.css b/styles.css
index 29446a6..e635046 100644
--- a/styles.css
+++ b/styles.css
@@ -17,7 +17,6 @@
     margin-bottom: 10px;
     margin-left: auto;
     margin-right: auto;
-/*    width: 90%; */
 }
 
 .path-mod-attendance .attfiltercontrols #currentdate {
@@ -117,7 +116,7 @@
     text-align: right;
 }
 .path-mod-attendance table.allsessions tr.grouper td {
-    background-color: #eeeeee;
+    background-color: #eee;
 }
 .path-mod-attendance table.allsessions td.groupheading {
     font-weight: bold;
diff --git a/view.php b/view.php
index efed400..dea964d 100644
--- a/view.php
+++ b/view.php
@@ -29,6 +29,7 @@ require_once(dirname(__FILE__).'/locallib.php');
 $pageparams = new mod_attendance_view_page_params();
 
 $id                     = required_param('id', PARAM_INT);
+$edit                   = optional_param('edit', -1, PARAM_BOOL);
 $pageparams->studentid  = optional_param('studentid', null, PARAM_INT);
 $pageparams->mode       = optional_param('mode', mod_attendance_view_page_params::MODE_THIS_COURSE, PARAM_INT);
 $pageparams->view       = optional_param('view', null, PARAM_INT);
@@ -74,6 +75,33 @@ if (isset($pageparams->studentid) && $USER->id != $pageparams->studentid) {
 $url = $att->url_view($pageparams->get_significant_params());
 $PAGE->set_url($url);
 
+$buttons = '';
+$capabilities = array('mod/attendance:takeattendances', 'mod/attendance:changeattendances');
+if (has_any_capability($capabilities, $context) &&
+    $pageparams->mode == mod_attendance_view_page_params::MODE_ALL_SESSIONS) {
+
+    if (!isset($USER->attendanceediting)) {
+        $USER->attendanceediting = false;
+    }
+
+    if (($edit == 1) and confirm_sesskey()) {
+        $USER->attendanceediting = true;
+    } else if ($edit == 0 and confirm_sesskey()) {
+        $USER->attendanceediting = false;
+    }
+
+    if ($USER->attendanceediting) {
+        $options['edit'] = 0;
+        $string = get_string('turneditingoff');
+    } else {
+        $options['edit'] = 1;
+        $string = get_string('turneditingon');
+    }
+    $options['sesskey'] = sesskey();
+    $button = new single_button(new moodle_url($PAGE->url, $options), $string, 'post');
+    $PAGE->set_button($OUTPUT->render($button));
+}
+
 $userdata = new attendance_user_data($att, $userid);
 
 // Create url for link in log screen.
@@ -93,22 +121,20 @@ if (empty($userdata->pageparams->studentid)) {
     $relateduserid = $userdata->pageparams->studentid;
 }
 
-if (($formdata = data_submitted()) && confirm_sesskey()) {
+if (($formdata = data_submitted()) && confirm_sesskey() && $edit == -1) {
     $userdata->take_sessions_from_form_data($formdata);
 
-    // Trigger updated event
+    // Trigger updated event.
     $event = \mod_attendance\event\session_report_updated::create(array(
         'relateduserid' => $relateduserid,
         'context' => $context,
         'other' => $params));
     $event->add_record_snapshot('course_modules', $cm);
-    //$event->add_record_snapshot('user', $);
     $event->trigger();
 
     redirect($url, get_string('attendancesuccess', 'attendance'));
-}
-else {
-    // Trigger viewed event
+} else {
+    // Trigger viewed event.
     $event = \mod_attendance\event\session_report_viewed::create(array(
         'relateduserid' => $relateduserid,
         'context' => $context,