FIX: users list show was loading multiple times with different params (#7058)

A first load was happening in route, which was setting properties on controller. These properties were observed on the controller and were triggering a reload of the AdminUser model.

Not only was it doing loading two times it was also sometimes resulting on the controller model refresh end to happen after route has been changed, resulting in a wrong model.

app/assets/javascripts/admin/controllers/admin-users-list-show.js.es6

@@ -1,12 +1,11 @@
 import debounce from "discourse/lib/debounce";
 import { i18n } from "discourse/lib/computed";
 import AdminUser from "admin/models/admin-user";
-import { observes } from "ember-addons/ember-computed-decorators";
 import CanCheckEmails from "discourse/mixins/can-check-emails";
 
 export default Ember.Controller.extend(CanCheckEmails, {
+  model: null,
   query: null,
-  queryParams: ["order", "ascending"],
   order: null,
   ascending: null,
   showEmails: false,
@@ -47,8 +46,7 @@ export default Ember.Controller.extend(CanCheckEmails, {
     this._refreshUsers();
   }, 250).observes("listFilter"),
 
-  @observes("order", "ascending")
-  _refreshUsers: function() {
+  _refreshUsers() {
     this.set("refreshing", true);
 
     AdminUser.findAll(this.get("query"), {
@@ -57,12 +55,8 @@ export default Ember.Controller.extend(CanCheckEmails, {
       order: this.get("order"),
       ascending: this.get("ascending")
     })
-      .then(result => {
-        this.set("model", result);
-      })
-      .finally(() => {
-        this.set("refreshing", false);
-      });
+      .then(result => this.set("model", result))
+      .finally(() => this.set("refreshing", false));
   },
 
   actions: {
@@ -95,7 +89,7 @@ export default Ember.Controller.extend(CanCheckEmails, {
 
     showEmails: function() {
       this.set("showEmails", true);
-      this._refreshUsers(true);
+      this._refreshUsers();
     }
   }
 });

app/assets/javascripts/admin/routes/admin-users-list-show.js.es6

@@ -1,17 +1,28 @@
-import AdminUser from "admin/models/admin-user";
-
 export default Discourse.Route.extend({
-  model: function(params) {
-    this.userFilter = params.filter;
-    return AdminUser.findAll(params.filter);
+  queryParams: {
+    order: { refreshModel: true },
+    ascending: { refreshModel: true }
   },
 
-  setupController: function(controller, model) {
-    controller.setProperties({
-      model: model,
-      query: this.userFilter,
-      showEmails: false,
-      refreshing: false
-    });
+  // TODO: this has been introduced to fix a bug in admin-users-list-show
+  // loading AdminUser model multiple times without refactoring the controller
+  beforeModel(transition) {
+    const routeName = "adminUsersList.show";
+
+    if (transition.targetName === routeName) {
+      const params = transition.params[routeName];
+      const controller = this.controllerFor(routeName);
+      if (controller) {
+        controller.setProperties({
+          order: transition.queryParams.order,
+          ascending: transition.queryParams.ascending,
+          query: params.filter,
+          showEmails: false,
+          refreshing: false
+        });
+
+        controller._refreshUsers();
+      }
+    }
   }
 });

test/javascripts/acceptance/admin-users-list-test.js.es6

@@ -8,3 +8,46 @@ QUnit.test("lists users", async assert => {
   assert.ok(exists(".users-list .user"));
   assert.ok(!exists(".user:eq(0) .email small"), "escapes email");
 });
+
+QUnit.test("switching tabs", async assert => {
+  const activeUser = "eviltrout@example.com";
+  const suspectUser = "sam@example.com";
+  const activeTitle = I18n.t("admin.users.titles.active");
+  const suspectTitle = I18n.t("admin.users.titles.suspect");
+
+  await visit("/admin/users/list/active");
+
+  assert.equal(find(".admin-title h2").text(), activeTitle);
+  assert.ok(
+    find(".users-list .user:nth-child(1) .email")
+      .text()
+      .includes(activeUser)
+  );
+
+  await click('a[href="/admin/users/list/suspect"]');
+
+  assert.equal(find(".admin-title h2").text(), suspectTitle);
+  assert.ok(
+    find(".users-list .user:nth-child(1) .email")
+      .text()
+      .includes(suspectUser)
+  );
+
+  await click(".users-list .sortable:nth-child(4)");
+
+  assert.equal(find(".admin-title h2").text(), suspectTitle);
+  assert.ok(
+    find(".users-list .user:nth-child(1) .email")
+      .text()
+      .includes(suspectUser)
+  );
+
+  await click('a[href="/admin/users/list/active"]');
+
+  assert.equal(find(".admin-title h2").text(), activeTitle);
+  assert.ok(
+    find(".users-list .user:nth-child(1) .email")
+      .text()
+      .includes(activeUser)
+  );
+});

test/javascripts/helpers/create-pretender.js.es6

@@ -458,6 +458,16 @@ export default function() {
       ]);
     });
 
+    this.get("/admin/users/list/suspect.json", () => {
+      return response(200, [
+        {
+          id: 2,
+          username: "sam",
+          email: "<small>sam@example.com</small>"
+        }
+      ]);
+    });
+
     this.get("/admin/customize/site_texts", request => {
       if (request.queryParams.overridden) {
         return response(200, { site_texts: [overridden] });