From b203e316acc5fa3112c79af923226aa31a738db6 Mon Sep 17 00:00:00 2001
From: Bianca Nenciu <nbianca@users.noreply.github.com>
Date: Tue, 9 Nov 2021 12:18:23 +0200
Subject: [PATCH] FEATURE: Add pagination to API keys page (#14777)

---
 .../addon/controllers/admin-api-keys-index.js |  39 +++++-
 .../admin/addon/templates/api-keys-index.hbs  | 126 +++++++++---------
 app/controllers/admin/api_controller.rb       |  20 +--
 spec/requests/admin/api_controller_spec.rb    |  19 ++-
 4 files changed, 126 insertions(+), 78 deletions(-)

diff --git a/app/assets/javascripts/admin/addon/controllers/admin-api-keys-index.js b/app/assets/javascripts/admin/addon/controllers/admin-api-keys-index.js
index 5b4ee4ee0a3..7b98fdba1d1 100644
--- a/app/assets/javascripts/admin/addon/controllers/admin-api-keys-index.js
+++ b/app/assets/javascripts/admin/addon/controllers/admin-api-keys-index.js
@@ -1,14 +1,39 @@
 import Controller from "@ember/controller";
+import { action } from "@ember/object";
 import { popupAjaxError } from "discourse/lib/ajax-error";
 
 export default Controller.extend({
-  actions: {
-    revokeKey(key) {
-      key.revoke().catch(popupAjaxError);
-    },
+  loading: false,
 
-    undoRevokeKey(key) {
-      key.undoRevoke().catch(popupAjaxError);
-    },
+  @action
+  revokeKey(key) {
+    key.revoke().catch(popupAjaxError);
+  },
+
+  @action
+  undoRevokeKey(key) {
+    key.undoRevoke().catch(popupAjaxError);
+  },
+
+  @action
+  loadMore() {
+    if (this.loading || this.model.loaded) {
+      return;
+    }
+
+    const limit = 50;
+
+    this.set("loading", true);
+    this.store
+      .findAll("api-key", { offset: this.model.length, limit })
+      .then((keys) => {
+        this.model.addObjects(keys);
+        if (keys.length < limit) {
+          this.model.set("loaded", true);
+        }
+      })
+      .finally(() => {
+        this.set("loading", false);
+      });
   },
 });
diff --git a/app/assets/javascripts/admin/addon/templates/api-keys-index.hbs b/app/assets/javascripts/admin/addon/templates/api-keys-index.hbs
index ac5dbf7c6ba..f5608434adb 100644
--- a/app/assets/javascripts/admin/addon/templates/api-keys-index.hbs
+++ b/app/assets/javascripts/admin/addon/templates/api-keys-index.hbs
@@ -5,67 +5,71 @@
   label="admin.api.new_key"}}
 
 {{#if model}}
-  <table class="api-keys grid">
-    <thead>
-      <th>{{i18n "admin.api.key"}}</th>
-      <th>{{i18n "admin.api.description"}}</th>
-      <th>{{i18n "admin.api.user"}}</th>
-      <th>{{i18n "admin.api.created"}}</th>
-      <th>{{i18n "admin.api.last_used"}}</th>
-      <th>&nbsp;</th>
-    </thead>
-    <tbody>
-      {{#each model as |k|}}
-        <tr class={{if k.revoked_at "revoked"}}>
-          <td class="key">
-            {{#if k.revoked_at}}{{d-icon "times-circle"}}{{/if}}
-            {{k.truncatedKey}}
-          </td>
-          <td class="key-description">
-            {{k.shortDescription}}
-          </td>
-          <td class="key-user">
-            <div class="label">{{i18n "admin.api.user"}}</div>
-            {{#if k.user}}
-              {{#link-to "adminUser" k.user}}
-                {{avatar k.user imageSize="small"}}
-              {{/link-to}}
-            {{else}}
-              {{i18n "admin.api.all_users"}}
-            {{/if}}
-          </td>
-          <td class="key-created">
-            <div class="label">{{i18n "admin.api.created"}}</div>
-            {{format-date k.created_at}}
-          </td>
-          <td class="key-last-used">
-            <div class="label">{{i18n "admin.api.last_used"}}</div>
-            {{#if k.last_used_at}}
-              {{format-date k.last_used_at}}
-            {{else}}
-              {{i18n "admin.api.never_used"}}
-            {{/if}}
-          </td>
-          <td class="key-controls">
-            {{d-button action=(route-action "show" k) icon="far-eye" title="admin.api.show_details"}}
-            {{#if k.revoked_at}}
-              {{d-button
-                action=(action "undoRevokeKey")
-                actionParam=k icon="undo"
-                title="admin.api.undo_revoke"}}
-            {{else}}
-              {{d-button
-                class="btn-danger"
-                action=(action "revokeKey")
-                actionParam=k
-                icon="times"
-                title="admin.api.revoke"}}
-            {{/if}}
-          </td>
-        </tr>
-      {{/each}}
-    </tbody>
-  </table>
+  {{#load-more selector=".api-keys tr" action=(action "loadMore")}}
+    <table class="api-keys grid">
+      <thead>
+        <th>{{i18n "admin.api.key"}}</th>
+        <th>{{i18n "admin.api.description"}}</th>
+        <th>{{i18n "admin.api.user"}}</th>
+        <th>{{i18n "admin.api.created"}}</th>
+        <th>{{i18n "admin.api.last_used"}}</th>
+        <th>&nbsp;</th>
+      </thead>
+      <tbody>
+        {{#each model as |k|}}
+          <tr class={{if k.revoked_at "revoked"}}>
+            <td class="key">
+              {{#if k.revoked_at}}{{d-icon "times-circle"}}{{/if}}
+              {{k.truncatedKey}}
+            </td>
+            <td class="key-description">
+              {{k.shortDescription}}
+            </td>
+            <td class="key-user">
+              <div class="label">{{i18n "admin.api.user"}}</div>
+              {{#if k.user}}
+                {{#link-to "adminUser" k.user}}
+                  {{avatar k.user imageSize="small"}}
+                {{/link-to}}
+              {{else}}
+                {{i18n "admin.api.all_users"}}
+              {{/if}}
+            </td>
+            <td class="key-created">
+              <div class="label">{{i18n "admin.api.created"}}</div>
+              {{format-date k.created_at}}
+            </td>
+            <td class="key-last-used">
+              <div class="label">{{i18n "admin.api.last_used"}}</div>
+              {{#if k.last_used_at}}
+                {{format-date k.last_used_at}}
+              {{else}}
+                {{i18n "admin.api.never_used"}}
+              {{/if}}
+            </td>
+            <td class="key-controls">
+              {{d-button action=(route-action "show" k) icon="far-eye" title="admin.api.show_details"}}
+              {{#if k.revoked_at}}
+                {{d-button
+                  action=(action "undoRevokeKey")
+                  actionParam=k icon="undo"
+                  title="admin.api.undo_revoke"}}
+              {{else}}
+                {{d-button
+                  class="btn-danger"
+                  action=(action "revokeKey")
+                  actionParam=k
+                  icon="times"
+                  title="admin.api.revoke"}}
+              {{/if}}
+            </td>
+          </tr>
+        {{/each}}
+      </tbody>
+    </table>
+  {{/load-more}}
+
+  {{conditional-loading-spinner condition=loading}}
 {{else}}
   <p>{{i18n "admin.api.none"}}</p>
 {{/if}}
diff --git a/app/controllers/admin/api_controller.rb b/app/controllers/admin/api_controller.rb
index e045131c563..627c4c40093 100644
--- a/app/controllers/admin/api_controller.rb
+++ b/app/controllers/admin/api_controller.rb
@@ -5,18 +5,22 @@ class Admin::ApiController < Admin::AdminController
   # If we used "api_key", then our user provider would try to use the value for authentication
 
   def index
+    offset = (params[:offset] || 0).to_i
+    limit = (params[:limit] || 50).to_i.clamp(1, 50)
+
     keys = ApiKey
       .where(hidden: false)
       .includes(:user, :api_key_scopes)
+      # Sort revoked keys by revoked_at and active keys by created_at
+      .order("revoked_at DESC NULLS FIRST, created_at DESC")
+      .offset(offset)
+      .limit(limit)
 
-    # Put active keys first
-    # Sort active keys by created_at, sort revoked keys by revoked_at
-    keys = keys.order(<<~SQL)
-      CASE WHEN revoked_at IS NULL THEN 0 ELSE 1 END,
-      COALESCE(revoked_at, created_at) DESC
-    SQL
-
-    render_serialized(keys.to_a, ApiKeySerializer, root: 'keys')
+    render_json_dump(
+      keys: serialize_data(keys, ApiKeySerializer),
+      offset: offset,
+      limit: limit
+    )
   end
 
   def show
diff --git a/spec/requests/admin/api_controller_spec.rb b/spec/requests/admin/api_controller_spec.rb
index c52b343a556..4e088a32649 100644
--- a/spec/requests/admin/api_controller_spec.rb
+++ b/spec/requests/admin/api_controller_spec.rb
@@ -12,6 +12,7 @@ describe Admin::ApiController do
 
   fab!(:key1, refind: false) { Fabricate(:api_key, description: "my key") }
   fab!(:key2, refind: false) { Fabricate(:api_key, user: admin) }
+  fab!(:key3, refind: false) { Fabricate(:api_key, user: admin) }
 
   context "as an admin" do
     before do
@@ -22,7 +23,21 @@ describe Admin::ApiController do
       it "succeeds" do
         get "/admin/api/keys.json"
         expect(response.status).to eq(200)
-        expect(response.parsed_body["keys"].length).to eq(2)
+        expect(response.parsed_body["keys"].length).to eq(3)
+      end
+
+      it "can paginate results" do
+        get "/admin/api/keys.json?offset=0&limit=2"
+        expect(response.status).to eq(200)
+        expect(response.parsed_body["keys"].map { |x| x["id"] }).to contain_exactly(key3.id, key2.id)
+
+        get "/admin/api/keys.json?offset=1&limit=2"
+        expect(response.status).to eq(200)
+        expect(response.parsed_body["keys"].map { |x| x["id"] }).to contain_exactly(key2.id, key1.id)
+
+        get "/admin/api/keys.json?offset=2&limit=2"
+        expect(response.status).to eq(200)
+        expect(response.parsed_body["keys"].map { |x| x["id"] }).to contain_exactly(key1.id)
       end
     end
 
@@ -246,7 +261,7 @@ describe Admin::ApiController do
       }
       expect(response.status).to eq(404)
 
-      expect(ApiKey.count).to eq(2)
+      expect(ApiKey.count).to eq(3)
     end
   end
 end