Skip to content

Add option to restrict a user to a group instead of banning them - #26

Merged
bonelifer merged 2 commits into
masterfrom
restrict-instead-of-ban
Sep 3, 2026
Merged

bonelifer merged 2 commits into
masterfrom
restrict-instead-of-ban

Conversation

@bonelifer

Copy link
Copy Markdown
Contributor

Answers CDB topic 240381, which asked whether Ban Hammer could move a user into a heavily-limited group for a set time instead of banning them outright.

What this adds:

  • A new "Restrict this user" option on the member profile page, next to the existing Ban Hammer options, shown only when a restrict-group is configured in the ACP and the viewer has m_ban.
  • A moderator picks a duration (or permanent), confirms, and the user is moved into the configured group as their new default group — they keep the ability to log in, with whatever permissions that group has.
  • A new cron task (restriction_expiry) checks periodically for expired restrictions and automatically restores the user's original group.
  • A new ACP setting, "Restrict to group," mirroring the existing "Move to group" field. Leaving it unset (the default) keeps this feature off entirely — nothing changes for boards that don't configure it.

New table: banhammer_restrict, tracking user_id, original_group_id, and restrict_until so the cron task knows what to restore and when.

cc @kaileymsnay

@bonelifer
bonelifer force-pushed the restrict-instead-of-ban branch from 5c01df4 to b5819cd Compare September 3, 2026 23:23
@bonelifer

Copy link
Copy Markdown
Contributor Author

(Claude, replying on William's behalf)

This branch needed a rebase onto current master (it was 13 commits behind) to resolve merge conflicts in config/services.yml and event/banhammer_listener.php, both caused by #25 (merged) adding new content in the same spots this PR does. Resolved by keeping both sides' additions - the ban_domain and restriction_expiry service blocks together, and core.mcp_queue_approve_details_template added alongside this PR's core.memberlist_view_profile change.

While validating the rebased content, found and fixed three real bugs, in a follow-up commit:

  • Duration math: restrict_until was computed as time() + (restrict_time * 86400), treating restrict_time as days. It's actually minutes (ban_length_options()'s keys, same unit user_ban() uses elsewhere in this file) - every duration was off by a factor of 1440x, and the "1 year" option risked overflowing the column entirely. Fixed the multiplier to 60.
  • Panel-toggle JS bug: the new "Restrict this user" trigger reuses the .bh-click class, but the existing click handler hardcoded #bh-options as the panel to toggle - clicking it opened/closed the unrelated Ban Hammer panel instead of its own. Changed it to toggle the clicked trigger's own next .inner sibling instead.
  • Stale file headers on the two new files (cron/task/restriction_expiry.php, migrations/restrict_group.php) - updated to match the header standardized across the rest of the repo.

Also added a check do_restrict_stuff() was missing: it didn't skip already-banned users the way do_ban_hammer_stuff() does, so it was offering to "restrict" someone who can't log in at all regardless of group.

PR is clean/mergeable against current master now.

bonelifer and others added 2 commits September 3, 2026 18:34
Moderators with m_ban can now move a user into a configured, severely
limited group for a set time (or permanently) from their profile page,
as an alternative to a full ban. A cron task restores the user's
original group automatically once the restriction expires.

Answers CDB topic 240381.
- event/banhammer_listener.php: restrict_until was computed as
  time() + (restrict_time * 86400), treating restrict_time as days.
  It's actually minutes (ban_length_options()'s keys, same unit
  user_ban() consumes elsewhere in this file) - every duration was
  off by a factor of 1440x, and the largest option (524160 minutes,
  "1 year") would compute a timestamp far enough out to risk
  overflowing the restrict_until column under phpBB's own enforced
  strict SQL mode. Fixed the multiplier to 60.
- Same file: do_restrict_stuff() didn't check whether the target is
  already banned before offering to restrict them, unlike the sibling
  do_ban_hammer_stuff(), which does. A banned user can't log in
  regardless of group, so the option was just confusing there.
- styles/prosilver/theme/banhammer.js: the restrict panel reuses the
  .bh-click trigger class, but the click handler hardcoded #bh-options
  as the panel to toggle - clicking the new "Restrict this user"
  trigger opened/closed the unrelated Ban Hammer panel instead of its
  own. Now toggles the clicked trigger's own next .inner sibling.
- cron/task/restriction_expiry.php, migrations/restrict_group.php:
  both still had the pre-standardization file header (old @Package
  tag, phpbbmodders.net). Updated to match the rest of the repo.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@bonelifer
bonelifer force-pushed the restrict-instead-of-ban branch from 24dacce to 9a98f8e Compare September 3, 2026 23:34
@bonelifer

Copy link
Copy Markdown
Contributor Author

(Claude, replying on William's behalf)

One more rebase — merging #33 just now reintroduced a conflict in controller/admin_controller.php (both PRs add a new field to the same $this->template->assign_vars() array). Resolved by keeping both: RESTRICT_GROUP alongside the already-merged SFS_ALLOW_HTTP/SFS_API_KEY/SFS_CURL. No functional change beyond that.

Clean/mergeable against current master again.

@bonelifer
bonelifer merged commit 8ffacf5 into master Sep 3, 2026
1 check passed
@bonelifer
bonelifer deleted the restrict-instead-of-ban branch September 3, 2026 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant