FIX: proper handling of group memberships #3
Conversation
|
cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| end | ||
|
|
||
| def add_members | ||
| group = Group.find(params.require(:group_id).to_i) |
There was a problem hiding this comment.
Bug: Incorrect ID Parameter Halts Group Management.
The add_members method expects params[:group_id] but the route is defined as a member route under resources :groups, which provides the group ID as params[:id] instead. This causes the endpoint to fail when called from the frontend.
|
|
||
| return can_not_modify_automatic if group.automatic | ||
|
|
||
| group.users.delete(user_id) |
There was a problem hiding this comment.
Bug: ActiveRecord Delete: Object Parameter Mismatch
The delete method on ActiveRecord associations expects an object or array of objects, not an ID. Passing user_id directly will not remove the user from the group. This should use group.users.delete(User.find(user_id)) or a similar approach that passes the actual user object.
| totalPages: function() { | ||
| if (this.get("user_count") == 0) { return 0; } | ||
| return Math.floor(this.get("user_count") / this.get("limit")) + 1; | ||
| }.property("limit", "user_count"), |
There was a problem hiding this comment.
Bug: Pagination Error: Extra Empty Page Appears
The totalPages calculation uses Math.floor(user_count / limit) + 1, which incorrectly returns 3 pages when user_count is exactly 100 and limit is 50 (should be 2 pages). This causes an empty third page to appear. The formula should use Math.ceil(user_count / limit) instead.
Test 8
Replicated from ai-code-review-evaluation/discourse-cursor#8
Note
Implements paginated group members with admin UI to add/remove users and corresponding backend endpoints, replacing username-passing on save/create.
admin-groupcontroller/model (limit/offset/user_count,currentPage/totalPages, next/previous actions).admin/templates/group.hbswith list, pagination controls, and add/remove actions; newgroup-memberview/template.Groupmodel:findMembers()consumes/members.jsonwithmeta; newaddMembers(usernames)andremoveMember(member);create/saveno longer take usernames.admin_group_route,group-membersroute). Templates adjusted to usemembers.GroupsController#membersnow returns JSON withmembersandmeta { total, limit, offset }and supports pagination.Admin::GroupsController: newcreate/updateparam shape, ILIKE search; newadd_membersandremove_memberactions; forbid modifying automatic groups; simplifieddestroy.PUT /admin/groups/members(add_members) andDELETE /admin/groups/members(remove_member).admin_base.scss.selector_placeholder, add/remove confirmations/labels).Written by Cursor Bugbot for commit 060cda7. Configure here.