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

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.

diff --git a/app/assets/javascripts/admin/controllers/admin-users-list-show.js.es6 b/app/assets/javascripts/admin/controllers/admin-users-list-show.js.es6
index 16b148f..f80222c 100644
--- a/app/assets/javascripts/admin/controllers/admin-users-list-show.js.es6
+++ b/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();
     }
   }
 });
diff --git a/app/assets/javascripts/admin/routes/admin-users-list-show.js.es6 b/app/assets/javascripts/admin/routes/admin-users-list-show.js.es6
index 88fb204..e893139 100644
--- a/app/assets/javascripts/admin/routes/admin-users-list-show.js.es6
+++ b/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();
+      }
+    }
   }
 });
diff --git a/test/javascripts/acceptance/admin-users-list-test.js.es6 b/test/javascripts/acceptance/admin-users-list-test.js.es6
index 0bbfb49..0a2b5c4 100644
--- a/test/javascripts/acceptance/admin-users-list-test.js.es6
+++ b/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)
+  );
+});
diff --git a/test/javascripts/helpers/create-pretender.js.es6 b/test/javascripts/helpers/create-pretender.js.es6
index 9bf0cf8..5cda6c8 100644
--- a/test/javascripts/helpers/create-pretender.js.es6
+++ b/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] });

GitHub sha: 71360436