FIX: proper handling of group memberships#8
Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 060cda7772
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| totalPages: function() { | ||
| if (this.get("user_count") == 0) { return 0; } | ||
| return Math.floor(this.get("user_count") / this.get("limit")) + 1; |
There was a problem hiding this comment.
Compute total pages with ceil to avoid empty extra page
When user_count is an exact multiple of limit, Math.floor(user_count/limit) + 1 overstates the page count (e.g., 50 users with limit 50 yields 2 pages). That makes the UI allow a “next” action to an empty page and prevents showingLast from becoming true on the real last page. Use Math.ceil(user_count / limit) (or Math.floor((user_count - 1)/limit) + 1) to avoid the off‑by‑one when user_count % limit === 0.
Useful? React with 👍 / 👎.
| @@ -0,0 +1 @@ | |||
| {{avatar member imageSize="small"}} {{member.username}} {{#unless automatic}}<a class='remove' {{action "removeMember" member}}>{{fa-icon "times"}}</a>{{/unless}} | |||
There was a problem hiding this comment.
Bind group member template to actual item context
This itemView template references member and automatic, but the {{each ... itemView="group-member"}} helper supplies the member as the view’s content context, not as a member property. As a result, member is undefined, so avatars/usernames don’t render and the remove action is invoked with undefined (raising errors when it calls member.get(...)). Bind to the item context (e.g., {{avatar this}} / {{username}}) or add a member: Em.computed.alias("content") on the view and reference controller.automatic for the group flag.
Useful? React with 👍 / 👎.
Test 8
Replicated from ai-code-review-evaluation/discourse-cursor#8