Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Paginate the Package Search Results #1588

Closed
sabine opened this issue Oct 6, 2023 · 16 comments · Fixed by #1657
Closed

Paginate the Package Search Results #1588

sabine opened this issue Oct 6, 2023 · 16 comments · Fixed by #1657
Assignees
Labels
good first issue Good for newcomers outreachy Outreachy contributions and blog posts

Comments

@sabine
Copy link
Collaborator

sabine commented Oct 6, 2023

Note: this has been selected as an Outreachy issue. To be assigned, you must have completed the instructions in #1553.

Please only request to be assigned this issue if you are applying for the GUI project

Task

Currently, all the entries of the package search results page are displayed on a single HTML page. E.g. at https://ocaml.org/packages/search?q=irmin.

Since there are a lot of entries, it makes sense to add pagination:
Screenshot 2023-10-03 at 15-13-43 OCaml Blog

As an example of how to do pagination (and for possible code sharing), look at how it is done for the Blog:

  1. In handler.ml, the list of entries is paginated according to a query parameter "p" from the URL:
    let paginate ~req ~n items =
    let items_per_page = n in
    let page =
    Dream.query req "p" |> Option.map int_of_string |> Option.value ~default:1
    in
    let number_of_pages =
    int_of_float
    (Float.ceil
    (float_of_int (List.length items) /. float_of_int items_per_page))
    in
    let current_items =
    let skip = items_per_page * (page - 1) in
    items |> List.drop skip |> List.take items_per_page
    in
    (page, number_of_pages, current_items)
    let blog req =
    let page, number_of_pages, current_items =
    paginate ~req ~n:10
    (List.filter
    (fun (x : Data.Planet.Post.t) -> not x.featured)
    Data.Planet.Post.all)
    in
    let featured = Data.Planet.featured_posts |> List.take 3 in
    let news = Data.News.all |> List.take 20 in
    Dream.html
    (Ocamlorg_frontend.blog ~featured ~planet:current_items ~planet_page:page
    ~planet_pages_number:number_of_pages ~news)
  2. In the template, the pagination component is rendered:
    <%s! Pagination.render ~total_page_count:planet_pages_number ~page_number:planet_page ~base_url:Url.blog %>

--

What you need to do:

  1. modify the function packages_search in handler.ml to paginate the list of changelog entries into pages that have at most 50 entries
  2. modify the template packages_search.eml to render the pagination component
@sabine sabine added good first issue Good for newcomers outreachy Outreachy contributions and blog posts labels Oct 6, 2023
@godplayer56
Copy link

Hey! @sabine I'd love to work on this issue! 😄
Can you assign me this issue?

@sabine
Copy link
Collaborator Author

sabine commented Oct 7, 2023

@godplayer56 have you completed the setup and are you applying to the GUI project? For the setup, could you please post a screenshot?

@Girish-Jangam
Copy link
Contributor

Hey i would like to work on this, could you please assign this to me?

@sabine
Copy link
Collaborator Author

sabine commented Oct 7, 2023

@Girish-Jangam have you completed the setup and are you applying to the GUI project? For the setup, could you please post a screenshot?

@Girish-Jangam
Copy link
Contributor

I just want to make my first contribution, regardless of any project. I thinking this issue is very interesting and challenging & it's good for me,

Talking about set-up, Yes I'm stack? It'll take some time to up and running

@Girish-Jangam
Copy link
Contributor

image

Here's the screenshot, please assign this issue to me! I'll work hard on this

@sabine
Copy link
Collaborator Author

sabine commented Oct 8, 2023

You are now assigned to this issue @Girish-Jangam. Have fun! 🙂

@Girish-Jangam
Copy link
Contributor

Thanks! I do my best

@godplayer56
Copy link

Hey @sabine I was working on this issue but due to some trouble at home I wasn't able to share my progress 😞

I tried using paginate function like this -

let packages_search t req =
  match Dream.query req "q" with
  | Some search ->
      let packages =
        Ocamlorg_package.search ~is_author_match ~sort_by_popularity:true t
          search
      in
      let page, current_items, number_of_pages = 
        paginate ~req ~n:10 packages
      in
      let total = List.length packages in
      let results = List.map (Package_helper.frontend_package t) packages in
      let search = Dream.from_percent_encoded search in
      Dream.html (Ocamlorg_frontend.packages_search ~total ~search ~page:page ~current_items:current_items ~number_of_pages:number_of_pages results)
  | None -> Dream.redirect req Url.packages

but vs code is showing error that Ocamlorg_frontend.packages_search takes too many arguments

I also edited the packages_search.eml file such as to include these parameters-

let render ~total ~search ~page ~current_items ~number_of_pages (packages : Package.package list) 

and also where should I add the pagination component in the eml file?

@Girish-Jangam
Copy link
Contributor

Hey, i'm working on this, I'll commit asap!

@godplayer56
Copy link

Sure @Girish-Jangam , I just wanted to share my progress with @sabine 👍

@sabine should I work on some other issue? I have already completed the setup as mentioned for outreachy applicants(I can share the progress with you if necessary).

I am thinking of working on an outreachy medium issue since all the beginner issues have been assigned 😞

@Girish-Jangam
Copy link
Contributor

Girish-Jangam commented Oct 15, 2023

let packages_search t req =
match Dream.query req "q" with
| Some search ->
let packages =
Ocamlorg_package.search ~is_author_match ~sort_by_popularity:true t
search
in

  let total = List.length packages in
  
  let page, number_of_pages, current_items =
    paginate ~req ~n:50 packages
  in
  
  Dream.log "page value = %d" page;
  Dream.log "number_of_pages value = %d" number_of_pages;
  Dream.log "current_items value = %d" (List.length current_items);

  let results = List.map (Package_helper.frontend_package t) current_items in

  let base_url = Url.packages_search ^ "?q=" ^ search 

  in
  let search = Dream.from_percent_encoded search in

  Dream.log "base_url = %s" base_url;
  Dream.log "search value = %s" search;

  Dream.html (Ocamlorg_frontend.packages_search ~total ~search ~page ~number_of_pages ~base_url results)

| None -> Dream.redirect req Url.packages

It uses paginate function

let paginate ~req ~n items =
let items_per_page = n in
let page =
Dream.query req "p" |> Option.map int_of_string |> Option.value ~default:1
in
let number_of_pages =
int_of_float
(Float.ceil
(float_of_int (List.length items) /. float_of_int items_per_page))
in
let current_items =
let skip = items_per_page * (page - 1) in
items |> List.drop skip |> List.take items_per_page
in
(page, number_of_pages, current_items)

<%s! Pagination.render ~total_page_count:number_of_pages ~page_number:page ~base_url:base_url %>

Error is shown in video,
Please guide me @sabine, @SaySayo

20231015114057.mp4

@SaySayo
Copy link
Contributor

SaySayo commented Oct 15, 2023

This bug looks like it might be coming from the way you define base_url. You're missing the &p= part in the URL, which is necessary for specifying the page number when you click on pagination links. It should look more like let base_url = Url.packages_search ^ "?q=" ^ search ^ "&p="

@Girish-Jangam
Copy link
Contributor

Girish-Jangam commented Oct 15, 2023

@SaySayo, Hello!

It's giving me following URL, I had tried this once

http://localhost:8080/packages/search?q=base&p=**?p=2**
http://localhost:8080/packages/search?q=base&p=**?p=3**
http://localhost:8080/packages/search?q=base&p=**?p=4**

Dream.query from paginate function appends ?p=number every time when i navigate through pages done on pagination.

20231015153150.mp4

@sabine
Copy link
Collaborator Author

sabine commented Oct 16, 2023

@Girish-Jangam it will be easier to comment on the current state (and for us to try and run it) if you open a Draft PR

@godplayer56 maybe you can solve #1640 and then be assigned a medium issue?

@godplayer56
Copy link

Sure @sabine I'll work on it👍

Girish-Jangam pushed a commit to Girish-Jangam/ocaml.org that referenced this issue Oct 16, 2023
Girish-Jangam pushed a commit to Girish-Jangam/ocaml.org that referenced this issue Oct 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers outreachy Outreachy contributions and blog posts
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants