FIX: proper handling of group memberships #3
Conversation
| controller.set('model', model); | ||
| controller.set('members', this.get('_members')); | ||
| controller.set("model", model); | ||
| model.findMembers(); |
There was a problem hiding this comment.
[Best Practice] Inconsistent Data Loading Strategy * Problem: Data loading has shifted from the route's afterModel hook to the controller's setupController, which can cause race conditions or UI inconsistencies. * Fix: Adhere to Ember's conventional pattern by resolving all necessary data in the route's model or afterModel hooks.
| @module Discourse | ||
| **/ | ||
| Discourse.Group = Discourse.Model.extend({ | ||
| limit: 50, |
There was a problem hiding this comment.
[Refactor] UI State in Domain Model and Controller * Problem: The Group model and AdminGroup controller are managing UI-specific pagination state, coupling them to the presentation layer. * Fix: Extract pagination state and logic into a dedicated presentation-layer component or service to improve model purity and reusability.
|
|
||
| return can_not_modify_automatic if group.automatic | ||
|
|
||
| usernames.split(",").each do |username| |
There was a problem hiding this comment.
[Performance] Inefficient Batch Operations * Problem: The add_members action iterates through usernames, performing individual database lookups and additions, leading to excessive queries and performance issues. * Fix: Refactor to fetch all users in a single batched query (e.g., User.where(username: usernames.split(',',))) and add them to the group more efficiently (e.g., group.users << users_to_add).
Test 8