web: require credit to post outside Help categories#6962
Conversation
Configurable by NEED_CREDIT_TO_POST_EXCEPT_HELP in project.inc (default true) Also minor code cleanup Also change ops/delete_account.php to use wipe_account()
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="html/inc/forum.inc">
<violation number="1" location="html/inc/forum.inc:1330">
P1: The new help-category credit check can dereference null on team forums because it looks up a category using a team id.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| if (NEED_CREDIT_TO_POST_EXCEPT_HELP) { | ||
| if ($user->total_credit == 0) { | ||
| $category = BoincCategory::lookup_id($forum->category); | ||
| if (!$category->is_helpdesk) { |
There was a problem hiding this comment.
P1: The new help-category credit check can dereference null on team forums because it looks up a category using a team id.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At html/inc/forum.inc, line 1330:
<comment>The new help-category credit check can dereference null on team forums because it looks up a category using a team id.</comment>
<file context>
@@ -1306,34 +1314,49 @@ function check_post_access($user, $forum) {
+ if (NEED_CREDIT_TO_POST_EXCEPT_HELP) {
+ if ($user->total_credit == 0) {
+ $category = BoincCategory::lookup_id($forum->category);
+ if (!$category->is_helpdesk) {
+ error_page(
+ tra("To create a thread you must have computing credit.")
</file context>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a configurable rule requiring users to have credit to create threads outside Helpdesk categories, plus some forum posting UX/permission refactoring and an ops script update to use wipe_account() for deletions.
Changes:
- Introduce
NEED_CREDIT_TO_POST_EXCEPT_HELP(defaulttrue) and enforce “must have credit” outside Helpdesk categories. - Refactor “New thread” button logic (
user_can_create_thread→show_post_button) and move thread-create permission checks intocheck_post_access(). - Update ops user deletion to call
wipe_account().
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| html/user/forum_post.php | Removes inline create-thread gating in favor of centralized check_post_access(). |
| html/user/forum_forum.php | Updates “New thread” button rendering to use show_post_button(). |
| html/ops/delete_user.php | Switches deletion implementation to wipe_account(). |
| html/inc/forum.inc | Adds new config flag + new credit gating logic; refactors post button + access checks. |
| html/inc/delete_account.inc | Clarifies deletion-method docs and adds editorial note. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (NEED_CREDIT_TO_POST_EXCEPT_HELP) { | ||
| if ($user->total_credit == 0) { | ||
| $category = BoincCategory::lookup_id($forum->category); | ||
| if (!$category->is_helpdesk) { | ||
| error_page( | ||
| tra("To create a thread you must have computing credit.") | ||
| ); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
$forum->category is not always a BoincCategory ID (e.g., team forums use category as a team ID—see existing is_moderator() logic). In those cases BoincCategory::lookup_id($forum->category) will return the wrong record or null, and $category->is_helpdesk can break. Fix by applying the Helpdesk exception only for parent_type == 0 (public forums/categories), or otherwise branching per parent_type and only doing BoincCategory lookup when the ID is actually a category ID.
| function check_post_access($user, $forum) { | ||
| if (is_admin($user)) return; | ||
| if ($forum->is_dev_blog) { | ||
| error_page("Can't post to News"); |
There was a problem hiding this comment.
This introduces a non-localized error string in a user-facing path while nearby errors use tra(). Wrap the message in tra() for translation consistency, and consider aligning wording with the action being blocked (e.g., thread creation vs posting generally) since this function is used for access enforcement.
| error_page("Can't post to News"); | |
| error_page(tra("Can't post to News")); |
| $user = BoincUser::lookup_id((int) $argv[1]); | ||
| if (!$user) die("no such user\n"); | ||
| $retval = delete_account($user); | ||
| $retval = wipe_account($user); |
There was a problem hiding this comment.
PR description says it changes ops/delete_account.php to use wipe_account(), but the actual change here is in html/ops/delete_user.php. Please update the PR description to match the filename being modified (or adjust the code change if the intent was a different script).
| // delete_account(): call one of these (or project-supplied func) | ||
| // based on config setting | ||
| // | ||
| // NOTE: this is way too complex. Just use the wipe option. |
There was a problem hiding this comment.
The NOTE is editorial and not actionable documentation for maintainers/users of this API. Consider rephrasing to an objective recommendation (e.g., when/why wipe_account() is preferred) or removing it to keep the header comment focused on behavior and selection logic.
| // NOTE: this is way too complex. Just use the wipe option. | |
| // Projects that intend to remove the account and all related DB records | |
| // should prefer the wipe method. |
| if (!defined('MAXIMUM_EDIT_TIME')) { | ||
| define('MAXIMUM_EDIT_TIME', 3600); | ||
| // allow edits of forums posts up till one hour after posting. | ||
| // allow edits of forums posts up to one hour after posting. |
There was a problem hiding this comment.
Correct the phrase 'forums posts' to 'forum posts'.
| // allow edits of forums posts up to one hour after posting. | |
| // allow edits of forum posts up to one hour after posting. |
Configurable by NEED_CREDIT_TO_POST_EXCEPT_HELP in project.inc (default true)
Also minor code cleanup
Also change ops/delete_account.php to use wipe_account()
Summary by cubic
Require computing credit to post outside Help categories, controlled by
NEED_CREDIT_TO_POST_EXCEPT_HELP(default true). Also update the delete-user CLI to usewipe_account().New Features
Refactors
user_can_create_thread()withshow_post_button()and centralize rules incheck_post_access().NEED_CREDIT_TO_POST_EXCEPT_HELPinforum.inc(default true).html/ops/delete_user.phpto callwipe_account().delete_account.incand minor copy cleanups.Written for commit 673b3e7. Summary will update on new commits.