Quoting Lukas Fleischer (2018-08-05 18:31:46)
+ [$pagination_templs, $per_page, $offset] = calculate_pagination($total_comment_count);
Interesting. I don't think we are using this syntax anywhere so far and it might be a good idea to use list() instead, even if it is just for consistency. Thoughts?
While I prefer the short syntax since it's both less noisy and more consistent with the short array creation syntax, it doesn't matter much, so I'll change it.
+ + $username = $row["Username"]; + $uid = $row["ID"]; + $comments = account_comments($row["ID"], $per_page, $offset);
We can replace the first parameter with $uid here.
Fixed.
diff --git a/web/html/css/aurweb.css b/web/html/css/aurweb.css index f5e1037..593c9ae 100644 --- a/web/html/css/aurweb.css +++ b/web/html/css/aurweb.css @@ -148,3 +148,45 @@ label.confirmation, color: red; font-weight: bold; } + +.package-comments { + margin-top: 1.5em; +} + +.comments-header { + display: flex; + justify-content: space-between; + align-items: flex-start; +} + +/* arrowed headings */ +.comments-header h3 span.text { + display: block; + background: #1794D1; + font-size: 15px; + padding: 2px 10px; + color: white; +} + +.comments-header .comments-header-nav { + align-self: flex-end; +} + +.comment-header { + clear: both; + font-size: 1em; + margin-top: 1.5em; + border-bottom: 1px dotted #bbb; +} + +.comments div { + margin-bottom: 1em; +} + +.comments div p { + margin-bottom: 0.5em; +} + +.comments .more { + font-weight: normal; +}
What is the rationale for adding those new styles? Can't we just display comments the way they are currently displayed under the package details?
I had to add more divs to be able to style the pagination links in a reasonable manner, which means that the archweb.css styles no longer apply.
diff --git a/web/lib/acctfuncs.inc.php b/web/lib/acctfuncs.inc.php index df57375..192d879 100644 --- a/web/lib/acctfuncs.inc.php +++ b/web/lib/acctfuncs.inc.php @@ -1403,3 +1403,45 @@ function accept_terms($uid, $termrev) { $dbh->exec($q); } } + +function account_comments($uid, $limit, $offset=0) { + $dbh = DB::connect(); + $q = "SELECT PackageComments.ID, Comments, UsersID, "; + $q.= "PackageBaseId, CommentTS, DelTS, EditedTS, B.UserName AS EditUserName, "; + $q.= "PinnedTS, "; + $q.= "C.UserName as DelUserName, RenderedComment, "; + $q.= "PB.ID as PackageBaseID, PB.Name as PackageBaseName "; + $q.= "FROM PackageComments "; + $q.= "LEFT JOIN PackageBases PB ON PackageComments.PackageBaseID = PB.ID "; + $q.= "LEFT JOIN Users A ON PackageComments.UsersID = A.ID "; + $q.= "LEFT JOIN Users B ON PackageComments.EditedUsersID = B.ID "; + $q.= "LEFT JOIN Users C ON PackageComments.DelUsersID = C.ID "; + $q.= "WHERE A.ID = " . $dbh->quote($uid) . " "; + $q.= "ORDER BY CommentTS DESC"; + + if ($limit > 0) { + $q.=" LIMIT " . $limit;
Please add intval() around $limit here. Even if we know that the current callers always pass integer values, it does not hurt to enforce the conversion here to prevent from potential security issues in the future.
+ } + + if ($offset > 0) { + $q.=" OFFSET " . $offset;
Same here. Add intval($offset).
Both fixed.
+ } + + $result = $dbh->query($q); + if (!$result) { + return null; + } + + return $result->fetchAll(); +} + +function account_comments_count($uid) { + $dbh = DB::connect(); + $q = "SELECT COUNT(*) "; + $q.= "FROM PackageComments "; + $q.= "LEFT JOIN Users A ON PackageComments.UsersID = A.ID "; + $q.= "WHERE A.ID = " . $dbh->quote($uid); + + $result = $dbh->query($q); + return $result->fetch(PDO::FETCH_NUM)[0];
Is this equivalent to "return $result->fetchColumn();"?
Yeah, fixed.
+} diff --git a/web/lib/aur.inc.php b/web/lib/aur.inc.php index feb4006..89da81a 100644 --- a/web/lib/aur.inc.php +++ b/web/lib/aur.inc.php @@ -705,3 +705,56 @@ function aur_location() { } return $location; } + +/** + * Calculate pagination templates + * + * @return array The array of pagination templates, per page, and offset values + */ +function calculate_pagination($total_comment_count) { [...]
Nice! I wonder whether we can use this helper function elsewhere, such as for paginating package search results?
Yep, that's the plan.
diff --git a/web/lib/pkgbasefuncs.inc.php b/web/lib/pkgbasefuncs.inc.php index 72c33b6..953a581 100644 --- a/web/lib/pkgbasefuncs.inc.php +++ b/web/lib/pkgbasefuncs.inc.php [...] @@ -71,6 +71,9 @@ function pkgbase_comments($base_id, $limit, $include_deleted, $only_pinned=false if ($limit > 0) { $q.=" LIMIT " . $limit; } + if ($offset > 0) { + $q.=" OFFSET " . $offset;
Same as above, please add intval().
Fixed.
Everything else looks good so far. And there's FS#59512 [1] (which you are probably already aware of) to be addressed in a re-roll. Thanks a lot put working on this!
Yeah, I hadn't even thought about letting regular users list their own comments, but there's no real reason not to, so fixed. -- Sincerely, Johannes Löthberg PGP Key ID: 0x50FB9B273A9D0BB5 PGP Key FP: 5134 EF9E AF65 F95B 6BB1 608E 50FB 9B27 3A9D 0BB5 https://theos.kyriasis.com/~kyrias/