Skip to content
This repository has been archived by the owner on Sep 24, 2020. It is now read-only.

Commit

Permalink
Fix coding style violations
Browse files Browse the repository at this point in the history
  • Loading branch information
@mudrd8mz
mudrd8mz committed Mar 19, 2018
1 parent 3fc6b14 commit 6ed1de2
Show file tree
Hide file tree
Showing 25 changed files with 293 additions and 84 deletions.
17 changes: 7 additions & 10 deletions amd/src/acceptances_filter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,13 @@ define(['jquery', 'core/form-autocomplete', 'core/str', 'core/notification'],
* @private
*/
var init = function() {
var stringkeys = [
{
key: 'filterplaceholder',
component: 'tool_policy'
},
{
key: 'nofiltersapplied',
component: 'tool_policy'
}
];
var stringkeys = [{
key: 'filterplaceholder',
component: 'tool_policy'
}, {
key: 'nofiltersapplied',
component: 'tool_policy'
}];

M.util.js_pending('acceptances_filter_datasource');
Str.get_strings(stringkeys).done(function(langstrings) {
Expand Down
102 changes: 90 additions & 12 deletions classes/acceptances_table.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,18 @@ class acceptances_table extends \table_sql {
*/
protected $countries;

/**
* Constructor.
*
* @param string $uniqueid Table identifier.
* @param acceptances_filter $acceptancesfilter
* @param renderer $output
*/
public function __construct($uniqueid, acceptances_filter $acceptancesfilter, renderer $output) {
global $CFG;
parent::__construct($uniqueid);
$this->acceptancesfilter = $acceptancesfilter;
$this->is_downloading(optional_param('download', 0, PARAM_ALPHA), 'user_acceptances'); // TODO add policy name and/or timestamp to the filename?
$this->is_downloading(optional_param('download', 0, PARAM_ALPHA), 'user_acceptances');
$this->baseurl = $acceptancesfilter->get_url();
$this->output = $output;

Expand All @@ -74,7 +81,8 @@ public function __construct($uniqueid, acceptances_filter $acceptancesfilter, re
$version = reset($versions);
$this->versionids[$version->id] = format_string($version->name);
if ($version->status != policy_version::STATUS_ACTIVE) {
$this->versionids[$version->id] .= '<br>' . format_string($version->revision); // TODO think about it
// TODO think about this.
$this->versionids[$version->id] .= '<br>' . format_string($version->revision);
}
}

Expand Down Expand Up @@ -105,6 +113,7 @@ public function __construct($uniqueid, acceptances_filter $acceptancesfilter, re

/**
* Remove randomness from the list by always sorting by user id in the end
*
* @return array
*/
public function get_sort_columns() {
Expand All @@ -119,6 +128,7 @@ public function get_sort_columns() {
* @param string $key
* @param string $label
* @param bool $sortable
* @param string $columnclass
*/
protected function add_column_header($key, $label, $sortable = true, $columnclass = '') {
if (empty($this->columns)) {
Expand All @@ -139,10 +149,14 @@ protected function add_column_header($key, $label, $sortable = true, $columnclas
}
}

/**
* Helper configuration method.
*/
protected function configure_for_single_version() {
$userfieldsmod = get_all_user_name_fields(true, 'm', null, 'mod');
$v = key($this->versionids);
$this->sql->fields .= ", $userfieldsmod, a{$v}.status AS status{$v}, a{$v}.note, a{$v}.timemodified, a{$v}.usermodified AS usermodified{$v}";
$this->sql->fields .= ", $userfieldsmod, a{$v}.status AS status{$v}, a{$v}.note, ".
"a{$v}.timemodified, a{$v}.usermodified AS usermodified{$v}";

$join = "JOIN {tool_policy_acceptances} a{$v} ON a{$v}.userid = u.id AND a{$v}.policyversionid=:versionid{$v}";
$filterstatus = $this->acceptancesfilter->get_status_filter();
Expand All @@ -166,6 +180,9 @@ protected function configure_for_single_version() {
$this->add_column_header('note', get_string('acceptancenote', 'tool_policy'), false);
}

/**
* Helper configuration method.
*/
protected function configure_for_multiple_versions() {
$this->add_column_header('statusall', get_string('acceptancestatusoverall', 'tool_policy'));
$filterstatus = $this->acceptancesfilter->get_status_filter();
Expand Down Expand Up @@ -203,7 +220,9 @@ public function download() {
}

/**
* @return string sql to add to where statement.
* Get sql to add to where statement.
*
* @return string
*/
public function get_sql_where() {
list($where, $params) = parent::get_sql_where();
Expand All @@ -212,6 +231,11 @@ public function get_sql_where() {
return [$where, $params];
}

/**
* Helper SQL query builder.
*
* @param array $userfields
*/
protected function build_sql_for_search_string($userfields) {
global $DB, $USER;

Expand Down Expand Up @@ -313,26 +337,26 @@ protected function build_sql_for_capability_filter() {

if (empty($forbiddenroles)) {
// There are no roles that prohibit to accept agreement on one own's behalf.
$this->sql->where .= ' AND ' . $this->sql_has_role($neededroles, $hascapability);
$this->sql->where .= ' AND ' . $this->sql_has_role($neededroles, $hascapability);
return;
}

$defaultuserroleid = isset($CFG->defaultuserroleid) ? $CFG->defaultuserroleid : 0;
if (!empty($neededroles[$defaultuserroleid])) {
// Default role allows to accept agreement. Make sure user has/does not have one of the roles prohibiting it.
$this->sql->where .= ' AND ' . $this->sql_has_role($forbiddenroles, !$hascapability);
$this->sql->where .= ' AND ' . $this->sql_has_role($forbiddenroles, !$hascapability);
return;
}

if ($hascapability) {
// User has at least one role allowing to accept and no roles prohibiting.
$this->sql->where .= ' AND ' . $this->sql_has_role($neededroles);
$this->sql->where .= ' AND ' . $this->sql_has_role($forbiddenroles, false);
$this->sql->where .= ' AND ' . $this->sql_has_role($neededroles);
$this->sql->where .= ' AND ' . $this->sql_has_role($forbiddenroles, false);
} else {
// Option 1: User has one of the roles prohibiting to accept.
$this->sql->where .= ' AND (' . $this->sql_has_role($forbiddenroles);
$this->sql->where .= ' AND (' . $this->sql_has_role($forbiddenroles);
// Option 2: User has none of the roles allowing to accept.
$this->sql->where .= ' OR ' . $this->sql_has_role($neededroles, false) . ")";
$this->sql->where .= ' OR ' . $this->sql_has_role($neededroles, false) . ")";
}
}

Expand Down Expand Up @@ -369,18 +393,38 @@ protected function build_sql_for_roles_filter() {
}
}

/**
* Render the table.
*/
public function display() {
$this->out(100, true);
}

/**
* Get the column fullname value.
*
* @param stdClass $row
* @return string
*/
public function col_fullname($row) {
global $OUTPUT;

$user = \user_picture::unalias($row, [], $this->useridfield);
$userpic = $this->is_downloading() ? '' : $OUTPUT->user_picture($user);

return $userpic . $this->username($user);
}

/**
* Helper content rendering method.
*
* @param stdClass $row
* @param string $fieldsprefix
* @param string $useridfield
* @return string
*/
protected function username($row, $fieldsprefix = '', $useridfield = 'id') {

if (!empty($row->$useridfield)) {
$user = (object)['id' => $row->$useridfield];
username_load_fields_from_object($user, $row, $fieldsprefix);
Expand All @@ -389,11 +433,16 @@ protected function username($row, $fieldsprefix = '', $useridfield = 'id') {
return $name;
}
$profileurl = new \moodle_url('/user/profile.php', array('id' => $user->id));
return \html_writer::link($profileurl, $name); // TODO cap view full names, cap to see profile
// TODO cap view full names, cap to see profile.
return \html_writer::link($profileurl, $name);
}

return null;
}

/**
* Helper.
*/
protected function get_return_url() {
$pageurl = $this->baseurl;
if ($this->currpage) {
Expand All @@ -402,6 +451,13 @@ protected function get_return_url() {
return $pageurl;
}

/**
* Helper.
*
* @param int $versionid
* @param stdClass $row
* @return string
*/
protected function status($versionid, $row) {
$status = $row->{'status' . $versionid};
if ($this->is_downloading()) {
Expand All @@ -411,6 +467,12 @@ protected function status($versionid, $row) {
return $this->output->render(new user_agreement($row->id, $status, $this->get_return_url(), $versionid, $onbehalf));
}

/**
* Get the column timemodified value.
*
* @param stdClass $row
* @return string
*/
public function col_timemodified($row) {
if ($row->timemodified) {
if ($this->is_downloading()) {
Expand All @@ -425,6 +487,12 @@ public function col_timemodified($row) {
}
}

/**
* Get the column note value.
*
* @param stdClass $row
* @return string
*/
public function col_note($row) {
if ($this->is_downloading()) {
return $row->note;
Expand All @@ -433,6 +501,12 @@ public function col_note($row) {
}
}

/**
* Get the column statusall value.
*
* @param stdClass $row
* @return string
*/
public function col_statusall($row) {
$totalcnt = count($this->versionids);
$cnt = 0;
Expand Down Expand Up @@ -478,8 +552,12 @@ public function col_country($data) {
/**
* You can override this method in a child class. See the description of
* build_table which calls this method.
*
* @param string $column
* @param stdClass $row
* @return string
*/
function other_cols($column, $row) {
public function other_cols($column, $row) {
if (preg_match('/^status([\d]+)$/', $column, $matches)) {
$versionid = $matches[1];
return $this->status($versionid, $row);
Expand Down
34 changes: 22 additions & 12 deletions classes/api.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -516,13 +516,14 @@ public static function inactivate($policyid) {

/**
* Delete the given version (if it is a draft). Also delete policy if this is the only version.
* @param $versionid
*
* @param int $versionid
*/
public static function delete($versionid) {
global $DB;

$version = api::get_policy_version($versionid);
$policy = api::list_policies([$version->policyid])[0];
$version = static::get_policy_version($versionid);
$policy = static::list_policies([$version->policyid])[0];
if ($version->archived || $policy->currentversionid == $version->id) {
// Can not delete archived or current version.
// TODO we could delete guest only versions potentially or versions without acceptances.
Expand Down Expand Up @@ -700,7 +701,8 @@ public static function is_user_version_accepted($userid, $versionid, $acceptance
}

/**
* Get the list of policies and versions that current user is able to see and the respective acceptance records for the selected user.
* Get the list of policies and versions that current user is able to see and the respective acceptance records for
* the selected user.
*
* @param int $userid
* @return array array with the same structure that list_policies() returns with additional attribute acceptance for versions
Expand All @@ -714,12 +716,16 @@ public static function get_policies_with_acceptances($userid) {
foreach ($policies as $i => $policy) {
$versions = [];
if ($policy->currentversion && $policy->currentversion->audience != policy_version::AUDIENCE_GUESTS) {
$policy->currentversion->acceptance = isset($acceptances[$policy->currentversion->id]) ?
$acceptances[$policy->currentversion->id] : 0;
if (isset($acceptances[$policy->currentversion->id])) {
$policy->currentversion->acceptance = $acceptances[$policy->currentversion->id];
} else {
$policy->currentversion->acceptance = 0;
}
$versions[] = $policy->currentversion;
}
foreach ($policy->archivedversions as $j => $version) {
if ($version->audience != policy_version::AUDIENCE_GUESTS && self::can_user_view_policy_version($version, $userid)) {
if ($version->audience != policy_version::AUDIENCE_GUESTS
&& static::can_user_view_policy_version($version, $userid)) {
$version->acceptance = isset($acceptances[$version->id]) ? $acceptances[$version->id] : 0;
$versions[] = $version;
}
Expand Down Expand Up @@ -784,7 +790,7 @@ public static function accept_policies($policyversionid, $userid = null, $note =
}
}

self::update_policyagreed($userid);
static::update_policyagreed($userid);
}

/**
Expand All @@ -807,7 +813,11 @@ public static function update_policyagreed($user = null) {
INNER JOIN {tool_policy_versions} v ON v.policyid = d.id AND v.id = d.currentversionid
LEFT JOIN {tool_policy_acceptances} a ON a.userid = :userid AND a.policyversionid = v.id
WHERE (v.audience = :audience OR v.audience = :audienceall)";
$params = ['audience' => policy_version::AUDIENCE_LOGGEDIN, 'audienceall' => policy_version::AUDIENCE_ALL, 'userid' => $user->id];
$params = [
'audience' => policy_version::AUDIENCE_LOGGEDIN,
'audienceall' => policy_version::AUDIENCE_ALL,
'userid' => $user->id
];
$policies = $DB->get_records_sql_menu($sql, $params);
$acceptedpolicies = array_filter($policies);
$policyagreed = (count($policies) == count($acceptedpolicies)) ? 1 : 0;
Expand Down Expand Up @@ -848,7 +858,7 @@ public static function revoke_acceptance($policyversionid, $userid, $note = null
acceptance_updated::create_from_record((object)($updatedata + (array)$currentacceptance))->trigger();
}

self::update_policyagreed($userid);
static::update_policyagreed($userid);
}

/**
Expand All @@ -872,11 +882,11 @@ public static function create_acceptances_user_created(\core\event\user_created
return;
}
// Get all active policies.
$currentpolicyversions = self::list_current_versions(policy_version::AUDIENCE_LOGGEDIN);
$currentpolicyversions = static::list_current_versions(policy_version::AUDIENCE_LOGGEDIN);
// Save active policies as accepted by the user.
if (!empty($currentpolicyversions)) {
$acceptances = array();
foreach($currentpolicyversions as $policy) {
foreach ($currentpolicyversions as $policy) {
$acceptances[] = array(
'policyversionid' => $policy->id,
'userid' => $userid,
Expand Down
Loading

0 comments on commit 6ed1de2

Please sign in to comment.