UX: sort admins and moderators by last seen date on /about page UX: only show active admins and moderators on /about page

UX: sort admins and moderators by last seen date on /about page UX: only show active admins and moderators on /about page

diff --git a/app/assets/javascripts/discourse/routes/about.js.es6 b/app/assets/javascripts/discourse/routes/about.js.es6
index 0027e79..019a233 100644
--- a/app/assets/javascripts/discourse/routes/about.js.es6
+++ b/app/assets/javascripts/discourse/routes/about.js.es6
@@ -1,7 +1,23 @@
 import { ajax } from "discourse/lib/ajax";
 export default Discourse.Route.extend({
   model() {
-    return ajax("/about.json").then(result => result.about);
+    return ajax("/about.json").then(result => {
+      let activeAdmins = [];
+      let activeModerators = [];
+      const yearAgo = moment()
+        .locale("en")
+        .utc()
+        .subtract(1, "year");
+      result.about.admins.forEach(r => {
+        if (moment(r.last_seen_at) > yearAgo) activeAdmins.push(r);
+      });
+      result.about.moderators.forEach(r => {
+        if (moment(r.last_seen_at) > yearAgo) activeModerators.push(r);
+      });
+      result.about.admins = activeAdmins;
+      result.about.moderators = activeModerators;
+      return result.about;
+    });
   },
 
   titleToken() {
diff --git a/app/models/about.rb b/app/models/about.rb
index 8392df3..126eead 100644
--- a/app/models/about.rb
+++ b/app/models/about.rb
@@ -36,11 +36,13 @@ class About
   def moderators
     @moderators ||= User.where(moderator: true, admin: false)
       .human_users
-      .order(:username_lower)
+      .order("last_seen_at DESC")
   end
 
   def admins
-    @admins ||= User.where(admin: true).human_users.order(:username_lower)
+    @admins ||= User.where(admin: true)
+      .human_users
+      .order("last_seen_at DESC")
   end
 
   def stats
diff --git a/app/serializers/about_serializer.rb b/app/serializers/about_serializer.rb
index 7111c6a..ebcac25 100644
--- a/app/serializers/about_serializer.rb
+++ b/app/serializers/about_serializer.rb
@@ -1,6 +1,11 @@
 class AboutSerializer < ApplicationSerializer
-  has_many :moderators, serializer: UserNameSerializer, embed: :objects
-  has_many :admins, serializer: UserNameSerializer, embed: :objects
+
+  class UserAboutSerializer < BasicUserSerializer
+    attributes :title, :last_seen_at
+  end
+
+  has_many :moderators, serializer: UserAboutSerializer, embed: :objects
+  has_many :admins, serializer: UserAboutSerializer, embed: :objects
 
   attributes :stats,
              :description,
diff --git a/test/javascripts/fixtures/about.js.es6 b/test/javascripts/fixtures/about.js.es6
index 08f89c2..c7b9d0d 100644
--- a/test/javascripts/fixtures/about.js.es6
+++ b/test/javascripts/fixtures/about.js.es6
@@ -21,37 +21,43 @@ export default {
           id: 3,
           username: "supermathie",
           uploaded_avatar_id: 5247,
-          avatar_template: "/images/avatar.png"
+          avatar_template: "/images/avatar.png",
+          last_seen_at: "2019-01-15T06:17:45.799Z"
         },
         {
           id: 32,
           username: "codinghorror",
           uploaded_avatar_id: 5297,
-          avatar_template: "/images/avatar.png"
+          avatar_template: "/images/avatar.png",
+          last_seen_at: "2019-01-15T06:17:45.799Z"
         },
         {
           id: 19,
           username: "eviltrout",
           uploaded_avatar_id: 5275,
           avatar_template: "/images/avatar.png"
+          last_seen_at: "2019-01-15T06:17:45.799Z"
         },
         {
           id: 2,
           username: "neil",
           uploaded_avatar_id: 5245,
           avatar_template: "/images/avatar.png"
+          last_seen_at: "2019-01-15T06:17:45.799Z"
         },
         {
           id: 1,
           username: "sam",
           uploaded_avatar_id: 5243,
           avatar_template: "/images/avatar.png"
+          last_seen_at: "2019-01-15T06:17:45.799Z"
         },
         {
           id: 1995,
           username: "zogstrip",
           uploaded_avatar_id: 8630,
           avatar_template: "/images/avatar.png"
+          last_seen_at: "2019-01-15T06:17:45.799Z"
         }
       ],
       admins: [
@@ -60,48 +66,56 @@ export default {
           username: "supermathie",
           uploaded_avatar_id: 5247,
           avatar_template: "/images/avatar.png"
+          last_seen_at: "2019-01-15T06:17:45.799Z"
         },
         {
           id: 32,
           username: "codinghorror",
           uploaded_avatar_id: 5297,
           avatar_template: "/images/avatar.png"
+          last_seen_at: "2019-01-15T06:17:45.799Z"
         },
         {
           id: 19,
           username: "eviltrout",
           uploaded_avatar_id: 5275,
           avatar_template: "/images/avatar.png"
+          last_seen_at: "2019-01-15T06:17:45.799Z"
         },
         {
           id: 38,
           username: "frandallfarmer",
           uploaded_avatar_id: 5307,
           avatar_template: "/images/avatar.png"
+          last_seen_at: "2019-01-15T06:17:45.799Z"
         },
         {
           id: 6626,
           username: "riking",
           uploaded_avatar_id: 9779,
           avatar_template: "/images/avatar.png"
+          last_seen_at: "2019-01-15T06:17:45.799Z"
         },
         {
           id: 2,
           username: "neil",
           uploaded_avatar_id: 5245,
           avatar_template: "/images/avatar.png"
+          last_seen_at: "2019-01-15T06:17:45.799Z"
         },
         {
           id: 1,
           username: "sam",
           uploaded_avatar_id: 5243,
           avatar_template: "/images/avatar.png"
+          last_seen_at: "2019-01-15T06:17:45.799Z"
         },
         {
           id: 1995,
           username: "zogstrip",
           uploaded_avatar_id: 8630,
           avatar_template: "/images/avatar.png"
+          last_seen_at: "2019-01-15T06:17:45.799Z"
         }
       ]
     }

GitHub sha: c4422fdc

Should this be tested?

Should we test the logic here too?

this seems like a super trivial thing to add the engineering effort of a test for?

this seems like a super trivial thing to add the engineering effort of a test for?

If it is a feature, my view is that it needs to be tested. We wouldn’t know right now when someone breaks this soring in the future. On top of that, it is much hard to work on this particular piece of the code in the future because there wouldn’t be test cases in place to document the expected behavior.

I guess it can make sense as de-facto documentation, but I don’t feel every atom of the universe needs to be tested… only things that functionally matter should be tested, not every possible operation