Default pagination metabase#17200 (metabase#17210)

Change default behavior of pagination to be what i imagined it to be. Really should've taken Dan's question to heart and actually tested that bit...

Author: howonlee

frontend/test/metabase/scenarios/admin/people/people.cy.spec.js

@@ -203,13 +203,11 @@ describe("scenarios > admin > people", () => {
       assertTableRowsCount(TOTAL_USERS);
     });
 
-    it.skip("should display more than 50 groups (metabase#17200)", () => {
-      generateGroups(50);
+    it("should display more than 50 groups (metabase#17200)", () => {
+      generateGroups(51);
 
       cy.visit("/admin/people/groups");
-      // The assertion depends on the way we'll implement this.
-      // a) if we go with the pagination, the assertion will have to be updated
-      // b) if we remove the limit, the current assertion will work
+      cy.scrollTo("bottom");
       cy.findByText("readonly");
     });
 

src/metabase/api/collection.clj

@@ -410,7 +410,10 @@
                      :order-by sql-order}
         ;; We didn't implement collection pagination for snippets namespace for root/items
         ;; Rip out the limit for now and put it back in when we want it
-        limit-query (if (= (:collection-namespace options) "snippets")
+        limit-query (if (or
+                          (nil? offset-paging/*limit*)
+                          (nil? offset-paging/*offset*)
+                          (= (:collection-namespace options) "snippets"))
                       rows-query
                       (assoc rows-query
                              :limit  offset-paging/*limit*

src/metabase/server/middleware/offset_paging.clj

@@ -3,14 +3,11 @@
             [metabase.server.middleware.security :as mw.security ]
             [metabase.util.i18n :refer [tru]]))
 
+(def ^:dynamic *limit* "Limit for offset-limit paging." nil)
 (def ^:private default-limit 50)
 
-(def ^:dynamic *limit*
-  "Limit for offset-limit paging.
-  Default set by default-limit in offset paging middleware."
-  default-limit)
-
-(def ^:dynamic *offset* "Offset for offset-limit paging. Default is no offset." 0)
+(def ^:dynamic *offset* "Offset for offset-limit paging." nil)
+(def ^:private default-offset 0)
 
 (def ^:dynamic *paged?*
   "Bool for whether a request is paged or not.
@@ -24,7 +21,7 @@
   (let [limit  (or (some-> limit Integer/parseUnsignedInt)
                    default-limit)
         offset (or (some-> offset Integer/parseUnsignedInt)
-                   0)]
+                   default-offset)]
     {:limit limit, :offset offset}))
 
 (defn- with-paging-params [request {:keys [limit offset]}]
@@ -56,7 +53,7 @@
                                  {:message (tru "Error parsing paging parameters: {0}" (ex-message e))})}))
           (let [{:keys [limit offset]} paging-params
                 request                (with-paging-params request paging-params)]
-            (binding [*limit*         limit
-                      *offset*        offset
+            (binding [*limit*  limit
+                      *offset* offset
                       *paged?* true]
               (handler request respond raise))))))))

test/metabase/server/middleware/offset_paging_test.clj

@@ -24,8 +24,8 @@
   (testing "no paging params"
     (is (= {:status  200
             :headers {}
-            :body    {:limit  @#'mw.offset-paging/default-limit
-                      :offset 0
+            :body    {:limit  nil
+                      :offset nil
                       :paged? false
                       :params {}}}
            (handler (ring.mock/request :get "/")))))