Add batch "Dismiss all" and "Ban all" actions to join requests UI#239
Add batch "Dismiss all" and "Ban all" actions to join requests UI#239allen0099 wants to merge 3 commits intoTDesktop-x64:devfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds batch actions to the join requests UI, allowing admins to dismiss or ban all pending join requests at once. The implementation includes confirmation dialogs to prevent accidental bulk actions.
Key Changes:
- New "Dismiss all" and "Ban all" buttons in the join requests interface
- Confirmation dialogs for batch operations
- New localization strings for the batch action buttons and confirmations
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| edit_peer_requests_box.h | Declares new methods for batch actions widget creation and request processing |
| edit_peer_requests_box.cpp | Implements batch actions with confirmation dialogs and processes all pending requests |
| boxes.style | Adds styling for the "Ban all" button |
| lang.strings | Adds localization strings for batch action buttons and confirmation messages |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| object_ptr<Ui::RpWidget> RequestsBoxController::createBatchActionsWidget() { | ||
| auto result = object_ptr<Ui::RpWidget>((QWidget*)nullptr); |
There was a problem hiding this comment.
Use nullptr directly instead of C-style cast (QWidget*)nullptr. The object_ptr constructor should accept nullptr without requiring a cast.
| auto result = object_ptr<Ui::RpWidget>((QWidget*)nullptr); | |
| auto result = object_ptr<Ui::RpWidget>(nullptr); |
| std::vector<not_null<UserData*>> users; | ||
| const auto fullCount = delegate()->peerListFullRowsCount(); | ||
| for (auto i = 0; i < fullCount; ++i) { |
There was a problem hiding this comment.
The code duplicates the user collection logic between dismissAllRequests() and banAllRequests(). Consider extracting this into a private helper method that returns the vector of users.
| textTop: 6px; | ||
| } | ||
| requestsBanAllButton: RoundButton(defaultActiveButton) { | ||
| width: -28px; |
There was a problem hiding this comment.
Negative width value -28px is likely incorrect. This should be a positive value or use a different sizing approach.
|
there are bulk approve and dismiss method available, but not sure working or not, for a really large count of join requests |
* Initial plan * Add dismiss all and ban all buttons to join requests Co-authored-by: allen0099 <22383656+allen0099@users.noreply.github.com> * Fix widget creation to use RpWidget instead of FixedHeightWidget Co-authored-by: allen0099 <22383656+allen0099@users.noreply.github.com> * Fix include path for confirm_box.h to use correct UI path Co-authored-by: allen0099 <22383656+allen0099@users.noreply.github.com> * Show batch action buttons by default and make ban all button red Co-authored-by: allen0099 <22383656+allen0099@users.noreply.github.com> * Fix style compilation error by using red text color instead of background Co-authored-by: allen0099 <22383656+allen0099@users.noreply.github.com> * Remove text color for ban all button --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: allen0099 <22383656+allen0099@users.noreply.github.com> Co-authored-by: Allen <s96016641@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| textBg: banButtonBg; | ||
| textBgOver: banButtonBgOver; | ||
| width: -28px; | ||
| height: 30px; | ||
| textTop: 6px; | ||
| ripple: RippleAnimation(defaultRippleAnimation) { | ||
| color: banButtonBgRipple; | ||
| } |
There was a problem hiding this comment.
Inconsistent indentation: this line uses spaces while surrounding lines use tabs. Should use tabs to match the file's indentation style.
| textBg: banButtonBg; | |
| textBgOver: banButtonBgOver; | |
| width: -28px; | |
| height: 30px; | |
| textTop: 6px; | |
| ripple: RippleAnimation(defaultRippleAnimation) { | |
| color: banButtonBgRipple; | |
| } | |
| textBg: banButtonBg; | |
| textBgOver: banButtonBgOver; | |
| width: -28px; | |
| height: 30px; | |
| textTop: 6px; | |
| ripple: RippleAnimation(defaultRippleAnimation) { | |
| color: banButtonBgRipple; | |
| } |
| color: banButtonBgRipple; | ||
| } |
There was a problem hiding this comment.
Inconsistent indentation: this line uses spaces while surrounding lines use tabs. Should use tabs to match the file's indentation style.
| color: banButtonBgRipple; | |
| } | |
| color: banButtonBgRipple; | |
| } |
| // Collect all users | ||
| std::vector<not_null<UserData*>> users; | ||
| const auto fullCount = delegate()->peerListFullRowsCount(); | ||
| for (auto i = 0; i < fullCount; ++i) { | ||
| const auto row = delegate()->peerListRowAt(i); | ||
| if (const auto user = row->peer()->asUser()) { | ||
| users.push_back(user); | ||
| } | ||
| } | ||
|
|
||
| // Process all requests | ||
| for (const auto user : users) { | ||
| processRequest(user, false, false); | ||
| } |
There was a problem hiding this comment.
This user collection and processing logic is duplicated in both dismissAllRequests() and banAllRequests(). Consider extracting this into a helper method that takes a callback or boolean parameter to reduce code duplication.
Implement "Dismiss all" and "Ban all" actions for admin in join requests UI.
Hide buttons when search is applied.
Screenshots:
