Skip to content

Commit

Permalink
Comply with Moodle SQL code style
Browse files Browse the repository at this point in the history 
Also slightly simplify SQL statement construction/concatenation
  • Loading branch information
@daniil-berg
daniil-berg committed Mar 5, 2024
1 parent 15874ca commit 0388940
Showing 1 changed file with 24 additions and 26 deletions.
50 changes: 24 additions & 26 deletions classes/course_files.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,14 +155,14 @@ public function get_file_list($offset, $limit) {
$usernameselect = get_all_user_name_fields(true, 'u');
}

$sql = 'FROM {files} f
LEFT JOIN {context} c ON (c.id = f.contextid)
LEFT JOIN {user} u ON (u.id = f.userid)
WHERE f.filename NOT LIKE \'.\'
AND (c.path LIKE :path OR c.id = :cid) ' . $sqlwhere;
$sql = "FROM {files} f
LEFT JOIN {context} c ON (c.id = f.contextid)
LEFT JOIN {user} u ON (u.id = f.userid)
WHERE f.filename NOT LIKE '.'
AND (c.path LIKE :path OR c.id = :cid) $sqlwhere";

$sqlselectfiles = 'SELECT f.*, c.contextlevel, c.instanceid, ' . $usernameselect .
' ' . $sql . ' ORDER BY f.component, f.filename';
$sqlselectfiles = "SELECT f.*, c.contextlevel, c.instanceid, $usernameselect $sql
ORDER BY f.component, f.filename";

$params = [
'path' => $this->context->path . '/%',
Expand All @@ -176,7 +176,7 @@ public function get_file_list($offset, $limit) {
if (count($this->filelist) < $limit) {
$this->filescount = count($this->filelist) + $offset;
} else {
$sqlcount = 'SELECT COUNT(*) ' . $sql;
$sqlcount = "SELECT COUNT(*) $sql";
$this->filescount = $DB->count_records_sql($sqlcount, $params);
}

Expand Down Expand Up @@ -230,12 +230,12 @@ public function get_components() {
return $this->components;
}

$sql = 'SELECT f.component
FROM {files} f
LEFT JOIN {context} c ON (c.id = f.contextid)
WHERE f.filename NOT LIKE \'.\'
AND (c.path LIKE :path OR c.id = :cid)
GROUP BY f.component';
$sql = "SELECT f.component
FROM {files} f
LEFT JOIN {context} c ON (c.id = f.contextid)
WHERE f.filename NOT LIKE '.'
AND (c.path LIKE :path OR c.id = :cid)
GROUP BY f.component";

This comment has been minimized.

Sign in to view

Copy link
@MartinGauk

MartinGauk Mar 6, 2024

Member

Ich fand die Einrückungen vorher aber besser. Das ist ja alles durcheinander.

This comment has been minimized.

Sign in to view

Copy link
@daniil-berg

daniil-berg Mar 6, 2024 •
edited
Loading

Author Contributor

Nee, das hat schon System. Die Idee ist, dass so Schlüsselworte wie SELECT, FROM, JOIN etc. untereinander rechtsbündig angeordnet sind. Siehe Moodle Style Guide. Ist vielleicht auf den ersten Blick etwas komisch, aber man gewöhnt sich recht schnell daran. Ich finde es so auch mittlerweile auch ganz gut lesbar, wenn man mental immer diese rote vertikale Linie vor Augen hat.
Dass nicht-top-level Schlüsselworte wie AND, NOT oder OR dabei rechts von besagter Linie bleiben sollen, hat auch irgendwie Sinn, auch wenn es wie gesagt auf den ersten Blick seltsam wirkt.

$params = ['path' => $this->context->path . '/%', 'cid' => $this->context->id];
$ret = $DB->get_fieldset_sql($sql, $params);
Expand Down Expand Up @@ -280,10 +280,10 @@ public function set_files_license($fileids, $license) {

// Check if the given files really belong to the context.
list($sqlin, $paramfids) = $DB->get_in_or_equal(array_keys($fileids), SQL_PARAMS_QM);
$sql = 'SELECT f.id, f.contextid, c.path
FROM {files} f
JOIN {context} c ON (c.id = f.contextid)
WHERE f.id ' . $sqlin;
$sql = "SELECT f.id, f.contextid, c.path
FROM {files} f
JOIN {context} c ON (c.id = f.contextid)
WHERE f.id $sqlin";
$res = $DB->get_records_sql($sql, $paramfids);

$checkedfileids = array_column($this->check_files_context($res), 'id');
Expand All @@ -293,9 +293,7 @@ public function set_files_license($fileids, $license) {

list($sqlin, $paramfids) = $DB->get_in_or_equal($checkedfileids, SQL_PARAMS_QM);
$transaction = $DB->start_delegated_transaction();
$sql = 'UPDATE {files}
SET license = ?
WHERE id ' . $sqlin;
$sql = "UPDATE {files} SET license = ? WHERE id $sqlin";
$DB->execute($sql, array_merge([$license], $paramfids));

foreach ($checkedfileids as $fid) {
Expand Down Expand Up @@ -351,11 +349,11 @@ public function download_files(&$fileids) {
}

list($sqlin, $paramfids) = $DB->get_in_or_equal(array_keys($fileids), SQL_PARAMS_QM);
$sql = 'SELECT f.*, c.path, r.repositoryid, r.reference, r.lastsync AS referencelastsync
FROM {files} f
LEFT JOIN {context} c ON (c.id = f.contextid)
LEFT JOIN {files_reference} r ON (f.referencefileid = r.id)
WHERE f.id ' . $sqlin;
$sql = "SELECT f.*, c.path, r.repositoryid, r.reference, r.lastsync AS referencelastsync
FROM {files} f
LEFT JOIN {context} c ON (c.id = f.contextid)
LEFT JOIN {files_reference} r ON (f.referencefileid = r.id)
WHERE f.id $sqlin";
$res = $DB->get_records_sql($sql, $paramfids);

$checkedfiles = $this->check_files_context($res);
Expand Down

0 comments on commit 0388940

Please sign in to comment.