fix: unify pagination to resolve problems with inconsistent pagination (

#2720)

src/ocamlorg_frontend/components/pagination.eml

@@ -1,3 +1,10 @@
+type t = {
+  total_page_count : int;
+  page_number : int;
+  base_url : string;
+  queries : ( string * string ) list;
+}
+
 let rec construct_query_string = function
   | h :: t -> "&" ^ (fst h) ^ "=" ^ (snd h) ^ construct_query_string t
   | [] -> ""
@@ -26,7 +33,7 @@ let right_arrow ~base_url page ~queries =
     <%s! Icons.chevron_right "h-5 w-5" %>
   </a>
 
-let render ~total_page_count ~page_number ~base_url ~queries =
+let render { total_page_count; page_number; base_url; queries } =
   <div class="pt-16 flex items-center justify-center space-x-2.5">
   <% if  total_page_count > 1 then ( %>
     <%s! if page_number != 1 then (left_arrow ~base_url page_number ~queries) else "" %>

src/ocamlorg_frontend/components/tutorial_search.eml

@@ -1,8 +1,7 @@
 let render 
 ~search
 ~total
-~number_of_pages
-~page
+~(pagination_info : Pagination.t)
 (documents : Data.Tutorial.search_document list) =
 Learn_layout.single_column_layout
 ~title:"OCaml Tutorial Documents Search"
@@ -40,7 +39,7 @@ Learn_layout.single_column_layout
 
           <% if List.length documents > 0 then ( %>
             <div class="text-content dark:text-dark-content mb-2 lg:mb-4">
-              Showing <%i ((page - 1) * 50 + 1) %> - <%i (min (page * 50) total) %>
+              Showing <%i ((pagination_info.page_number - 1) * 50 + 1) %> - <%i (min (pagination_info.page_number * 50) total) %>
             </div>
           <ol class="flex flex-col">
             <% documents |> List.iter (fun (doc : Data.Tutorial.search_document) -> %>
@@ -62,7 +61,7 @@ Learn_layout.single_column_layout
             <% ); %>
           </ol>
           <% ); %>
-        <%s! Pagination.render ~total_page_count:number_of_pages ~page_number:page ~base_url:Url.tutorial_search ~queries:[("q", search)] %>
+        <%s! Pagination.render pagination_info %>
       </div>
     </div>
   </div>

src/ocamlorg_frontend/ocamlorg_frontend.ml

@@ -3,6 +3,7 @@ module Package_overview = Package_overview
 module Navmap = Navmap
 module Toc = Toc
 module Package = Package
+module Pagination = Pagination
 
 let about = About.render
 let academic_users = Academic_users.render

src/ocamlorg_frontend/pages/academic_institutions.eml

@@ -6,7 +6,7 @@ let render_tag tag show =
     in
     <span class="px-3 inline-block max-w-max rounded-3xl font-mono text-sm text-center text-white <%s _cls %> <%s if show then "" else "hidden" %>"><%s name %></span>
 
-let render ?(search = "") ?(continent = "All") ?(resource_type = "All") ~pages_number ~page (institutions : Data.Academic_institution.t list) =
+let render ?(search = "") ?(continent = "All") ?(resource_type = "All") ~pagination_info (institutions : Data.Academic_institution.t list) =
 Layout.render
 ~title:"Academic Institutions"
 ~description:"Many more academic institutions teach OCaml"
@@ -51,11 +51,11 @@ Layout.render
         </form>
         <% institutions |> List.iter (fun (item : Data.Academic_institution.t) -> let logo = match item.logo with | Some x -> Ocamlorg_static.Media.url x | None -> "" in %>
             <div class="flex flex-col md:flex-row items-start lg:items-center border-b border-separator_20 dark:border-dark-separator_30 py-4">
-                <div class="w-44 h-24 rounded-lg flex justify-center items-center border border-separator_20 dark:border-dark-separator_30 bg-white overflow-hidden p-4 mr-5 mb-4">
+                <a href="<%s item.url %>" class="w-44 h-24 rounded-lg flex justify-center items-center border border-separator_20 dark:border-dark-separator_30 bg-white overflow-hidden p-4 mr-5 mb-4">
                     <img class="max-h-16 m-auto" src="<%s logo %>" alt="<%s item.name %> logo">
-                </div>
+                </a>
                 <div class="flex flex-col flex-grow">
-                    <p class="font-bold text-lg text-title dark:text-dark-title mb-2"><%s item.name %></p>
+                    <a href="<%s item.url %>" class="font-bold text-lg text-title dark:text-dark-title mb-2 hover:underline"><%s item.name %></a>
                     <ul class="w-full flex flex-col">
                         <% item.courses 
                         |> List.filter (fun (course : Data.Academic_institution.course) -> match resource_type with
@@ -75,9 +75,11 @@ Layout.render
                                     <%s! Icons.link "h-5 mr-2 text-primary dark:text-dark-white group-hover:text-primary dark:group-hover:text-dark-primary w-4 h-5" %>
                                     <%s course.name %>
                                 </a>
-                                <%s! render_tag "lecture_notes" course.lecture_notes %>
-                                <%s! render_tag "exercises" course.exercises %>
-                                <%s! render_tag "video_recordings" course.video_recordings %>
+                                <div class="flex gap-2">
+                                    <%s! render_tag "lecture_notes" course.lecture_notes %>
+                                    <%s! render_tag "exercises" course.exercises %>
+                                    <%s! render_tag "video_recordings" course.video_recordings %>
+                                </div>
                             </li>
                         <% ); %>
                     </ul>
@@ -86,27 +88,8 @@ Layout.render
         <% ); %>
     </div>
     <div class="container-fluid">
-        <div class="pt-16 flex items-center justify-center space-x-2.5 w-full">
-            <a href="<%s Url.academic_institutions %>?p=<%i page - 1 %>"
-                class="w-10 h-10 lg:w-14 lg:h-14  border border-body-100 rounded-lg flex items-center justify-center text-content hover:bg-gray-50 hover:no-underline">
-                <%s! Icons.chevron_left "h-5 w-5" %>
-            </a>
-            <% (for i = 1 to (pages_number - 1) do
-                  if i = page then ( %>
-            <a href="<%s Url.academic_institutions %>?p=<%i i %>"
-                class="w-10 h-10 lg:w-14 lg:h-14  border-2 border-current rounded-lg flex items-center justify-center text-primary">
-                <%i i %>
-            </a>
-            <% ) else ( %>
-            <a href="<%s Url.academic_institutions %>?p=<%i i %>"
-                class="w-10 h-10 lg:w-14 lg:h-14  border border-body-100 rounded-lg flex items-center justify-center text-content hover:bg-gray-50 hover:no-underline">
-                <%i i %>
-            </a>
-            <% ) done); %>
-            <a href="<%s Url.academic_institutions %>?p=<%i page + 1 %>"
-                class="w-10 h-10 lg:w-14 lg:h-14  border border-body-100 rounded-lg flex items-center justify-center text-content hover:bg-gray-50 hover:no-underline">
-                <%s! Icons.chevron_right "h-5 w-5" %>
-            </a>
-          </div>
+        <div class="flex items-start">
+            <%s! Pagination.render pagination_info %> 
+        </div>
     </div>
 </div>

src/ocamlorg_frontend/pages/changelog.eml

@@ -1,7 +1,7 @@
-let render ~tags ?current_tag ~number_of_pages ~current_page (news : Data.Changelog.t list) =
+let render ~tags ?current_tag ~(pagination_info : Pagination.t) (news : Data.Changelog.t list) =
 News_layout.single_column_layout
 ~title:"OCaml Changelog"
-~canonical:(Url.changelog ^ if current_page = 1 then "" else "?p=" ^ string_of_int current_page)
+~canonical:(Url.changelog ^ if pagination_info.page_number = 1 then "" else "?p=" ^ string_of_int pagination_info.page_number)
 ~description:"Read the latest releases and updates from the OCaml ecosystem."
 ~current:Changelog @@
 <div class="intro-section-simple dark:dark-intro-section-simple py-12">
@@ -71,7 +71,7 @@ News_layout.single_column_layout
           </div>
         </article>
         <% ); %>
-        <%s! Pagination.render ~total_page_count:number_of_pages ~page_number:current_page ~base_url:Url.changelog ~queries:(match current_tag with Some current_tag -> [("t", current_tag)] | None -> [])%>
+        <%s! Pagination.render pagination_info %>
       </div>
     </div>
     <div class="prose dark:prose-invert max-w-none mt-8">

src/ocamlorg_frontend/pages/news.eml

@@ -1,8 +1,8 @@
-let render ~pages_number ~page (news : Data.News.t list) =
+let render ~(pagination_info : Pagination.t) (news : Data.News.t list) =
 News_layout.single_column_layout
 ~title:"News from the OCaml teams"
 ~description:"Read the latest news, releases, and updates from the OCaml compiler and platform developers."
-~canonical:(Url.news ^ if page = 1 then "" else "?p=" ^ string_of_int page)
+~canonical:(Url.news ^ if pagination_info.page_number = 1 then "" else "?p=" ^ string_of_int pagination_info.page_number)
 ~current:Newsletters @@
 <div class="intro-section-simple dark:dark-intro-section-simple py-12">
   <div class="container-fluid">
@@ -41,26 +41,7 @@ News_layout.single_column_layout
       <% ); %>
 
       <div class="pt-16 flex items-center justify-center space-x-2.5">
-        <a href="<%s Url.news %>?p=<%i page - 1 %>"
-            class="w-10 h-10 lg:w-14 lg:h-14  border border-body-100 rounded-lg flex items-center justify-center text-content hover:bg-gray-50 hover:no-underline">
-            <%s! Icons.chevron_left "h-5 w-5" %>
-        </a>
-        <% (for i = 1 to (pages_number - 1) do
-              if i = page then ( %>
-        <a href="<%s Url.news %>?p=<%i i %>"
-            class="w-10 h-10 lg:w-14 lg:h-14  border-2 border-current rounded-lg flex items-center justify-center text-primary">
-            <%i i %>
-        </a>
-        <% ) else ( %>
-        <a href="<%s Url.news %>?p=<%i i %>"
-            class="w-10 h-10 lg:w-14 lg:h-14  border border-body-100 rounded-lg flex items-center justify-center text-content hover:bg-gray-50 hover:no-underline">
-            <%i i %>
-        </a>
-        <% ) done); %>
-        <a href="<%s Url.news %>?p=<%i page + 1 %>"
-            class="w-10 h-10 lg:w-14 lg:h-14  border border-body-100 rounded-lg flex items-center justify-center text-content hover:bg-gray-50 hover:no-underline">
-            <%s! Icons.chevron_right "h-5 w-5" %>
-        </a>
+        <%s! Pagination.render pagination_info %>
       </div>
     </div>
   </div>

src/ocamlorg_frontend/pages/ocaml_planet.eml

@@ -49,11 +49,12 @@ let render_blog_post (post : Data.Planet.entry) =
         <% ); %>
       </div>
   </div>
-let render ?(category = "All") ~planet_pages_number ~planet_page (planet : Data.Planet.entry list) =
+
+let render ?(category = "All") ~(pagination_info : Pagination.t) (planet : Data.Planet.entry list) =
 News_layout.single_column_layout
 ~title:"The OCaml Planet"
 ~description:"The RSS aggregator for the OCaml community."
-~canonical:(Url.ocaml_planet ^ if planet_page = 1 then "" else "?p=" ^ string_of_int planet_page)
+~canonical:(Url.ocaml_planet ^ if pagination_info.page_number = 1 then "" else "?p=" ^ string_of_int pagination_info.page_number)
 ~alternate:{ href = "/planet.xml"; title = "The OCaml Planet"; type_ = "application/atom+xml" }
 ~current:OCamlPlanet @@
 <div class="bg-white dark:bg-dark-background py-12">
@@ -76,7 +77,7 @@ News_layout.single_column_layout
               |> String.concat "\n" ; %>
           </div>
           <div class="flex items-start">
-            <%s! Pagination.render ~total_page_count:planet_pages_number ~page_number:planet_page ~base_url:Url.ocaml_planet ~queries:[("category", category)]%> 
+            <%s! Pagination.render pagination_info %> 
           </div>
 
         </div> 

src/ocamlorg_frontend/pages/packages_search.eml

@@ -1,4 +1,4 @@
-let render ~total ~search ~page ~number_of_pages (packages : Package.package list) = Layout.render ~title:"OCaml Packages · Search Result"
+let render ~total ~search ~(pagination_info : Pagination.t) (packages : Package.package list) = Layout.render ~title:"OCaml Packages · Search Result"
 ~description:"Find the package you need to build your application in the thousands of available OCaml packages."
 ~canonical:(Url.packages_search ^ "?q=" ^ search)
 ~active_top_nav_item:Header.Packages @@
@@ -43,7 +43,7 @@ let render ~total ~search ~page ~number_of_pages (packages : Package.package lis
 
         <% if List.length packages > 0 then ( %>
           <div class="text-title dark:text-dark-content mb-4">
-            Showing <%i ((page - 1) * 50 + 1) %> - <%i (min (page * 50) total) %>
+            Showing <%i ((pagination_info.page_number - 1) * 50 + 1) %> - <%i (min (pagination_info.page_number * 50) total) %>
           </div>
         <ol class="flex flex-col">
           <% packages |> List.iter (fun (package : Package.package) -> %>
@@ -122,7 +122,7 @@ let render ~total ~search ~page ~number_of_pages (packages : Package.package lis
           <% ); %>
         </ol>
         <% ); %>
-        <%s! Pagination.render ~total_page_count:number_of_pages ~page_number:page ~base_url:Url.packages_search ~queries:[("q", search)] %>
+        <%s! Pagination.render pagination_info %>
       </div>
     </div>
   </div>

src/ocamlorg_web/lib/handler.ml

@@ -250,16 +250,30 @@ let paginate ~req ~n items =
   in
   (page, number_of_pages, current_items)
 
+let query_param ~name value =
+  match value with None -> [] | Some v -> [ (name, v) ]
+
 let learn_documents_search req =
-  let q = Dream.query req "q" |> Option.value ~default:"" in
-  let search_results = Data.Tutorial.search_documents q in
-  let page, number_of_pages, current_items =
-    paginate ~req ~n:50 search_results
+  let q = Dream.query req "q" in
+  let search_results =
+    Data.Tutorial.search_documents (q |> Option.value ~default:"")
   in
   let total = List.length search_results in
+  let page_number, total_page_count, current_items =
+    paginate ~req ~n:50 search_results
+  in
+  let pagination_info =
+    Ocamlorg_frontend.Pagination.
+      {
+        total_page_count;
+        page_number;
+        base_url = Url.tutorial_search;
+        queries = query_param ~name:"q" q;
+      }
+  in
   Dream.html
-    (Ocamlorg_frontend.tutorial_search current_items ~total ~page
-       ~number_of_pages ~search:q)
+    (Ocamlorg_frontend.tutorial_search current_items ~total ~pagination_info
+       ~search:(q |> Option.value ~default:""))
 
 let changelog req =
   let current_tag = Dream.query req "t" in
@@ -277,10 +291,23 @@ let changelog req =
             List.exists (( = ) tag) change.tags)
           Data.Changelog.all
   in
-  let page, number_of_pages, current_changes = paginate ~req ~n:30 changes in
+
+  let page_number, total_page_count, current_changes =
+    paginate ~req ~n:50 changes
+  in
+  let pagination_info =
+    Ocamlorg_frontend.Pagination.
+      {
+        total_page_count;
+        page_number;
+        base_url = Url.changelog;
+        queries = query_param ~name:"t" current_tag;
+      }
+  in
+
   Dream.html
-    (Ocamlorg_frontend.changelog ?current_tag ~tags ~number_of_pages
-       ~current_page:page current_changes)
+    (Ocamlorg_frontend.changelog ?current_tag ~tags ~pagination_info
+       current_changes)
 
 let changelog_entry req =
   let slug = Dream.param req "id" in
@@ -411,13 +438,24 @@ let academic_institutions req =
       (fun institution -> matches_criteria institution continent resource_type)
       users
   in
-  let page, number_of_pages, institutions =
+  let page_number, total_page_count, institutions =
     paginate ~req ~n:10 filtered_institutions
   in
-
+  let pagination_info =
+    Ocamlorg_frontend.Pagination.
+      {
+        total_page_count;
+        page_number;
+        base_url = Url.academic_institutions;
+        queries =
+          query_param ~name:"q" query
+          @ query_param ~name:"resource_type" resource_type
+          @ query_param ~name:"continent" continent;
+      }
+  in
   Dream.html
     (Ocamlorg_frontend.academic_institutions ?search:query ?continent
-       ?resource_type ~page ~pages_number:number_of_pages institutions)
+       ?resource_type ~pagination_info institutions)
 
 let about _req = Dream.html (Ocamlorg_frontend.about ())
 
@@ -542,19 +580,31 @@ let ocaml_planet req =
   let filtered_entries =
     Data.Planet.all |> List.filter (fun item -> matches_criteria item category)
   in
-  let page, number_of_pages, current_items =
+  let page_number, total_page_count, current_items =
     paginate ~req ~n:10 filtered_entries
   in
+  let pagination_info =
+    Ocamlorg_frontend.Pagination.
+      {
+        total_page_count;
+        page_number;
+        base_url = Url.ocaml_planet;
+        queries = query_param ~name:"category" category;
+      }
+  in
+
   Dream.html
-    (Ocamlorg_frontend.ocaml_planet ~planet_page:page
-       ~planet_pages_number:number_of_pages ?category current_items)
+    (Ocamlorg_frontend.ocaml_planet ~pagination_info ?category current_items)
 
 let news req =
-  let page, number_of_pages, current_items =
+  let page_number, total_page_count, current_items =
     paginate ~req ~n:10 Data.News.all
   in
-  Dream.html
-    (Ocamlorg_frontend.news ~page ~pages_number:number_of_pages current_items)
+  let pagination_info =
+    Ocamlorg_frontend.Pagination.
+      { total_page_count; page_number; base_url = Url.news; queries = [] }
+  in
+  Dream.html (Ocamlorg_frontend.news ~pagination_info current_items)
 
 let news_post req =
   let slug = Dream.param req "id" in
@@ -982,18 +1032,30 @@ let packages_search t req =
     | None -> Ocamlorg_package.all_latest t
   in
   let total = List.length packages in
-  let page, number_of_pages, current_items = paginate ~req ~n:50 packages in
+
   let search =
     Dream.from_percent_encoded
       (match Dream.query req "q" with Some search -> search | None -> "")
   in
 
+  let page_number, total_page_count, current_items =
+    paginate ~req ~n:50 packages
+  in
+  let pagination_info =
+    Ocamlorg_frontend.Pagination.
+      {
+        total_page_count;
+        page_number;
+        base_url = Url.packages_search;
+        queries = [ ("q", search) ];
+      }
+  in
+
   let open Lwt.Syntax in
   let* results = prepare_search_result_packages t current_items in
 
   Dream.html
-    (Ocamlorg_frontend.packages_search ~total ~search ~page ~number_of_pages
-       results)
+    (Ocamlorg_frontend.packages_search ~total ~search ~pagination_info results)
 
 let packages_autocomplete_fragment t req =
   match Dream.query req "q" with