FIX: proper handling of group memberships#3
Conversation
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| <a {{bind-attr class=":next showingLast:disabled"}} {{action "next"}}>{{fa-icon "fast-forward"}}</a> | ||
| </div> | ||
| <div class="ac-wrap clearfix"> | ||
| {{each member in members itemView="group-member"}} |
There was a problem hiding this comment.
Using the Handlebars each helper with itemView won’t instantiate your group-member view; this likely results in members not rendering (and the member variable in the item template not resolving). This will break the admin members list display.
🤖 Was this useful? React with 👍 or 👎
|
|
||
| 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.
totalPages uses Math.floor(...)+1, which overcounts when user_count is an exact multiple of limit, showing an extra page. This leads to an empty page at the end of pagination.
🤖 Was this useful? React with 👍 or 👎
| if (this.get("showingLast")) { return; } | ||
|
|
||
| var group = this.get("model"), | ||
| offset = Math.min(group.get("offset") + group.get("limit"), group.get("user_count")); |
There was a problem hiding this comment.
In next(), the computed offset can equal user_count, which yields an empty result set and advances to a blank page. This makes the last pagination step show no members.
🤖 Was this useful? React with 👍 or 👎
|
|
||
| render_serialized(members.to_a, GroupUserSerializer) | ||
| limit = (params[:limit] || 50).to_i | ||
| offset = params[:offset].to_i |
There was a problem hiding this comment.
offset = params[:offset].to_i is not sanitized; a negative value can produce an invalid SQL OFFSET and a 500 if a crafted request supplies a negative offset. This server-side edge case bypasses the client’s clamping.
🤖 Was this useful? React with 👍 or 👎
2 participants
No description provided.