From eda039f3e5b0420cc7a6b346b041f6c6787997ac Mon Sep 17 00:00:00 2001 From: dusk Date: Wed, 20 Aug 2025 23:27:09 +0300 Subject: [PATCH] appview: improve pagination API Change-Id: llstyrzusrmvlkxnnyunzszpvnyoqvlp Signed-off-by: dusk --- appview/db/issues.go | 6 +- appview/db/notifications.go | 6 +- appview/issues/issues.go | 27 +++++-- appview/issues/router.go | 2 +- appview/middleware/middleware.go | 46 ++++++------ appview/notifications/notifications.go | 7 +- appview/pages/pages.go | 6 +- .../templates/goodfirstissues/index.html | 14 ++-- .../pages/templates/notifications/list.html | 13 ++-- .../pages/templates/repo/issues/issues.html | 12 +-- appview/pagination/page.go | 74 +++++++++++++++---- appview/repo/feed.go | 2 +- appview/state/gfi.go | 15 ++-- 13 files changed, 147 insertions(+), 83 deletions(-) diff --git a/appview/db/issues.go b/appview/db/issues.go index 028668b0..c13ff146 100644 --- a/appview/db/issues.go +++ b/appview/db/issues.go @@ -98,8 +98,8 @@ func GetIssuesPaginated(e Execer, page pagination.Page, filters ...filter) ([]mo whereClause = " where " + strings.Join(conditions, " and ") } - pLower := FilterGte("row_num", page.Offset+1) - pUpper := FilterLte("row_num", page.Offset+page.Limit) + pLower := FilterGte("row_num", page.Count*page.No+1) + pUpper := FilterLte("row_num", page.Count*(page.No+1)) args = append(args, pLower.Arg()...) args = append(args, pUpper.Arg()...) @@ -244,7 +244,7 @@ func GetIssuesPaginated(e Execer, page pagination.Page, filters ...filter) ([]mo } func GetIssues(e Execer, filters ...filter) ([]models.Issue, error) { - return GetIssuesPaginated(e, pagination.FirstPage(), filters...) + return GetIssuesPaginated(e, pagination.Page{No: 0, Count: 30}, filters...) } func GetIssue(e Execer, repoAt syntax.ATURI, issueId int) (*models.Issue, error) { diff --git a/appview/db/notifications.go b/appview/db/notifications.go index 6282ef56..0584e8f2 100644 --- a/appview/db/notifications.go +++ b/appview/db/notifications.go @@ -68,7 +68,7 @@ func GetNotificationsPaginated(e Execer, page pagination.Page, filters ...filter limit ? offset ? `, whereClause) - args = append(args, page.Limit, page.Offset) + args = append(args, page.Count, page.Count*page.No+1) rows, err := e.QueryContext(context.Background(), query, args...) if err != nil { @@ -142,7 +142,7 @@ func GetNotificationsWithEntities(e Execer, page pagination.Page, filters ...fil limit ? offset ? `, whereClause) - args = append(args, page.Limit, page.Offset) + args = append(args, page.Count, page.Count*page.No+1) rows, err := e.QueryContext(context.Background(), query, args...) if err != nil { @@ -247,7 +247,7 @@ func GetNotificationsWithEntities(e Execer, page pagination.Page, filters ...fil // GetNotifications retrieves notifications with filters func GetNotifications(e Execer, filters ...filter) ([]*models.Notification, error) { - return GetNotificationsPaginated(e, pagination.FirstPage(), filters...) + return GetNotificationsPaginated(e, pagination.Page{No: 0, Count: 30}, filters...) } func CountNotifications(e Execer, filters ...filter) (int64, error) { diff --git a/appview/issues/issues.go b/appview/issues/issues.go index d454d890..850443bb 100644 --- a/appview/issues/issues.go +++ b/appview/issues/issues.go @@ -769,12 +769,6 @@ func (rp *Issues) RepoIssues(w http.ResponseWriter, r *http.Request) { isOpen = true } - page, ok := r.Context().Value("page").(pagination.Page) - if !ok { - log.Println("failed to get page") - page = pagination.FirstPage() - } - user := rp.oauth.GetUser(r) f, err := rp.repoResolver.Resolve(r) if err != nil { @@ -782,13 +776,30 @@ func (rp *Issues) RepoIssues(w http.ResponseWriter, r *http.Request) { return } + issueCounts, err := db.GetIssueCount(rp.db, f.RepoAt()) + if err != nil { + log.Println("failed to get issue count", err) + rp.pages.Notice(w, "issues", "Failed to load issues. Try again later.") + return + } + page, ok := r.Context().Value("page").(pagination.Page) + if !ok { + log.Println("failed to get page") + page = pagination.Page{No: 0, Count: 10} + } + issueCount := issueCounts.Closed + if isOpen { + issueCount = issueCounts.Open + } + paginate := pagination.NewFromPage(page, issueCount) + openVal := 0 if isOpen { openVal = 1 } issues, err := db.GetIssuesPaginated( rp.db, - page, + paginate.CurrentPage(), db.FilterEq("repo_at", f.RepoAt()), db.FilterEq("open", openVal), ) @@ -820,7 +831,7 @@ func (rp *Issues) RepoIssues(w http.ResponseWriter, r *http.Request) { Issues: issues, LabelDefs: defs, FilteringByOpen: isOpen, - Page: page, + Pagination: paginate, }) } diff --git a/appview/issues/router.go b/appview/issues/router.go index dc34c67a..887b539e 100644 --- a/appview/issues/router.go +++ b/appview/issues/router.go @@ -11,7 +11,7 @@ func (i *Issues) Router(mw *middleware.Middleware) http.Handler { r := chi.NewRouter() r.Route("/", func(r chi.Router) { - r.With(middleware.Paginate).Get("/", i.RepoIssues) + r.With(middleware.Paginate(30)).Get("/", i.RepoIssues) r.Route("/{issue}", func(r chi.Router) { r.Use(mw.ResolveIssue) diff --git a/appview/middleware/middleware.go b/appview/middleware/middleware.go index 2d7366ab..074d4586 100644 --- a/appview/middleware/middleware.go +++ b/appview/middleware/middleware.go @@ -81,33 +81,35 @@ func AuthMiddleware(o *oauth.OAuth) middlewareFunc { } } -func Paginate(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - page := pagination.FirstPage() +func Paginate(paginationCount int) func(next http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + page := pagination.Page{No: 0, Count: paginationCount} - offsetVal := r.URL.Query().Get("offset") - if offsetVal != "" { - offset, err := strconv.Atoi(offsetVal) - if err != nil { - log.Println("invalid offset") - } else { - page.Offset = offset + pageNoVal := r.URL.Query().Get("page") + if pageNoVal != "" { + pageNo, err := strconv.Atoi(pageNoVal) + if err != nil { + log.Println("invalid page no") + } else if pageNo > 0 { + page.No = pageNo - 1 + } } - } - limitVal := r.URL.Query().Get("limit") - if limitVal != "" { - limit, err := strconv.Atoi(limitVal) - if err != nil { - log.Println("invalid limit") - } else { - page.Limit = limit + pageCountVal := r.URL.Query().Get("count") + if pageCountVal != "" { + pageCount, err := strconv.Atoi(pageCountVal) + if err != nil { + log.Println("invalid page count") + } else { + page.Count = pageCount + } } - } - ctx := context.WithValue(r.Context(), "page", page) - next.ServeHTTP(w, r.WithContext(ctx)) - }) + ctx := context.WithValue(r.Context(), "page", page) + next.ServeHTTP(w, r.WithContext(ctx)) + }) + } } func (mw Middleware) knotRoleMiddleware(group string) middlewareFunc { diff --git a/appview/notifications/notifications.go b/appview/notifications/notifications.go index 5cda8e39..e3cc5f76 100644 --- a/appview/notifications/notifications.go +++ b/appview/notifications/notifications.go @@ -34,7 +34,7 @@ func (n *Notifications) Router(mw *middleware.Middleware) http.Handler { r.Group(func(r chi.Router) { r.Use(middleware.AuthMiddleware(n.oauth)) - r.With(middleware.Paginate).Get("/", n.notificationsPage) + r.With(middleware.Paginate(30)).Get("/", n.notificationsPage) r.Post("/{id}/read", n.markRead) r.Post("/read-all", n.markAllRead) r.Delete("/{id}", n.deleteNotification) @@ -49,7 +49,7 @@ func (n *Notifications) notificationsPage(w http.ResponseWriter, r *http.Request page, ok := r.Context().Value("page").(pagination.Page) if !ok { log.Println("failed to get page") - page = pagination.FirstPage() + page = pagination.Page{No: 0, Count: 30} } total, err := db.CountNotifications( @@ -61,6 +61,7 @@ func (n *Notifications) notificationsPage(w http.ResponseWriter, r *http.Request n.pages.Error500(w) return } + paginate := pagination.NewFromPage(page, int(total)) notifications, err := db.GetNotificationsWithEntities( n.db, @@ -84,7 +85,7 @@ func (n *Notifications) notificationsPage(w http.ResponseWriter, r *http.Request LoggedInUser: user, Notifications: notifications, UnreadCount: unreadCount, - Page: page, + Pagination: paginate, Total: total, }) } diff --git a/appview/pages/pages.go b/appview/pages/pages.go index ad3b248b..3148f5ee 100644 --- a/appview/pages/pages.go +++ b/appview/pages/pages.go @@ -320,7 +320,7 @@ type GoodFirstIssuesParams struct { RepoGroups []*models.RepoGroup LabelDefs map[string]*models.LabelDefinition GfiLabel *models.LabelDefinition - Page pagination.Page + Pagination pagination.Pagination } func (p *Pages) GoodFirstIssues(w io.Writer, params GoodFirstIssuesParams) error { @@ -341,7 +341,7 @@ type NotificationsParams struct { LoggedInUser *oauth.User Notifications []*models.NotificationWithEntity UnreadCount int - Page pagination.Page + Pagination pagination.Pagination Total int64 } @@ -968,7 +968,7 @@ type RepoIssuesParams struct { Active string Issues []models.Issue LabelDefs map[string]*models.LabelDefinition - Page pagination.Page + Pagination pagination.Pagination FilteringByOpen bool } diff --git a/appview/pages/templates/goodfirstissues/index.html b/appview/pages/templates/goodfirstissues/index.html index b4fa8dd1..efc5fdde 100644 --- a/appview/pages/templates/goodfirstissues/index.html +++ b/appview/pages/templates/goodfirstissues/index.html @@ -130,15 +130,15 @@ {{ end }} - {{ if or (gt .Page.Offset 0) (eq (len .RepoGroups) .Page.Limit) }} + {{ if or .Pagination.HasPreviousPage .Pagination.HasNextPage }}
- {{ if gt .Page.Offset 0 }} - {{ $prev := .Page.Previous }} + {{ if .Pagination.HasPreviousPage }} + {{ $prev := .Pagination.PreviousPage }} {{ i "chevron-left" "w-4 h-4" }} previous @@ -147,12 +147,12 @@
{{ end }} - {{ if eq (len .RepoGroups) .Page.Limit }} - {{ $next := .Page.Next }} + {{ if .Pagination.HasNextPage }} + {{ $next := .Pagination.NextPage }} next {{ i "chevron-right" "w-4 h-4" }} diff --git a/appview/pages/templates/notifications/list.html b/appview/pages/templates/notifications/list.html index 4e0ffd73..c9434fab 100644 --- a/appview/pages/templates/notifications/list.html +++ b/appview/pages/templates/notifications/list.html @@ -35,12 +35,12 @@ {{ define "pagination" }}
- {{ if gt .Page.Offset 0 }} - {{ $prev := .Page.Previous }} + {{ if .Pagination.HasPreviousPage }} + {{ $prev := .Pagination.PreviousPage }} {{ i "chevron-left" "w-4 h-4" }} previous @@ -49,13 +49,12 @@
{{ end }} - {{ $next := .Page.Next }} - {{ if lt $next.Offset .Total }} - {{ $next := .Page.Next }} + {{ if .Pagination.HasNextPage }} + {{ $next := .Pagination.NextPage }} next {{ i "chevron-right" "w-4 h-4" }} diff --git a/appview/pages/templates/repo/issues/issues.html b/appview/pages/templates/repo/issues/issues.html index 5d073f0e..43993437 100644 --- a/appview/pages/templates/repo/issues/issues.html +++ b/appview/pages/templates/repo/issues/issues.html @@ -50,12 +50,12 @@ {{ $currentState = "open" }} {{ end }} - {{ if gt .Page.Offset 0 }} - {{ $prev := .Page.Previous }} + {{ if .Pagination.HasPreviousPage }} + {{ $prev := .Pagination.PreviousPage }} {{ i "chevron-left" "w-4 h-4" }} previous @@ -64,12 +64,12 @@
{{ end }} - {{ if eq (len .Issues) .Page.Limit }} - {{ $next := .Page.Next }} + {{ if .Pagination.HasNextPage }} + {{ $next := .Pagination.NextPage }} next {{ i "chevron-right" "w-4 h-4" }} diff --git a/appview/pagination/page.go b/appview/pagination/page.go index b3dc781b..37e738c1 100644 --- a/appview/pagination/page.go +++ b/appview/pagination/page.go @@ -1,31 +1,79 @@ package pagination -type Page struct { +type Pagination struct { Offset int // where to start from - Limit int // number of items in a page + Limit int // number of items on a single page + Total int // total number of items +} + +type Page struct { + No int // page number + Count int // number of items on this page +} + +func New(offset, limit, total int) Pagination { + return Pagination{ + Offset: offset, + Limit: limit, + Total: total, + } } -func FirstPage() Page { +func NewFromPage(page Page, total int) Pagination { + return New(page.No*page.Count, page.Count, total) +} + +func (p Pagination) FirstPage() Page { return Page{ - Offset: 0, - Limit: 30, + No: 0, + Count: p.Limit, } } -func (p Page) Previous() Page { +func (p Pagination) CurrentPage() Page { + return Page{ + No: p.Offset / p.Limit, + Count: p.Limit, + } +} + +func (p Pagination) LastPage() Page { + return Page{ + No: (p.Total - 1) / p.Limit, + Count: p.Limit, + } +} + +func (p Pagination) PreviousPage() Page { if p.Offset-p.Limit < 0 { - return FirstPage() + return p.FirstPage() } else { return Page{ - Offset: p.Offset - p.Limit, - Limit: p.Limit, + No: (p.Offset - p.Limit) / p.Limit, + Count: p.Limit, } } } -func (p Page) Next() Page { - return Page{ - Offset: p.Offset + p.Limit, - Limit: p.Limit, +func (p Pagination) NextPage() Page { + if p.Offset+p.Limit >= p.Total { + return p.LastPage() + } else { + return Page{ + No: (p.Offset + p.Limit) / p.Limit, + Count: p.Limit, + } } } + +func (p Pagination) HasPreviousPage() bool { + return p.CurrentPage().No > 0 +} + +func (p Pagination) HasNextPage() bool { + return p.CurrentPage().No+1 < p.TotalPageCount() +} + +func (p Pagination) TotalPageCount() int { + return (p.Total + p.Limit - 1) / p.Limit +} diff --git a/appview/repo/feed.go b/appview/repo/feed.go index 31d194e3..3bb3885b 100644 --- a/appview/repo/feed.go +++ b/appview/repo/feed.go @@ -27,7 +27,7 @@ func (rp *Repo) getRepoFeed(ctx context.Context, f *reporesolver.ResolvedRepo) ( issues, err := db.GetIssuesPaginated( rp.db, - pagination.Page{Limit: feedLimitPerType}, + pagination.Page{No: 0, Count: feedLimitPerType}, db.FilterEq("repo_at", f.RepoAt()), ) if err != nil { diff --git a/appview/state/gfi.go b/appview/state/gfi.go index b0106813..3ecbc4b7 100644 --- a/appview/state/gfi.go +++ b/appview/state/gfi.go @@ -20,7 +20,7 @@ func (s *State) GoodFirstIssues(w http.ResponseWriter, r *http.Request) { page, ok := r.Context().Value("page").(pagination.Page) if !ok { - page = pagination.FirstPage() + page = pagination.Page{No: 0, Count: 30} } goodFirstIssueLabel := fmt.Sprintf("at://%s/%s/%s", consts.TangledDid, tangled.LabelDefinitionNSID, "good-first-issue") @@ -37,7 +37,7 @@ func (s *State) GoodFirstIssues(w http.ResponseWriter, r *http.Request) { LoggedInUser: user, RepoGroups: []*models.RepoGroup{}, LabelDefs: make(map[string]*models.LabelDefinition), - Page: page, + Pagination: pagination.NewFromPage(page, 0), }) return } @@ -50,7 +50,8 @@ func (s *State) GoodFirstIssues(w http.ResponseWriter, r *http.Request) { allIssues, err := db.GetIssuesPaginated( s.db, pagination.Page{ - Limit: 500, + No: 0, + Count: 500, }, db.FilterIn("repo_at", repoUris), db.FilterEq("open", 1), @@ -98,8 +99,10 @@ func (s *State) GoodFirstIssues(w http.ResponseWriter, r *http.Request) { return sortedGroups[i].Repo.Name < sortedGroups[j].Repo.Name }) - groupStart := page.Offset - groupEnd := page.Offset + page.Limit + pagination := pagination.NewFromPage(page, len(sortedGroups)) + + groupStart := page.Count * page.No + groupEnd := page.Count * (page.No + 1) if groupStart > len(sortedGroups) { groupStart = len(sortedGroups) } @@ -145,7 +148,7 @@ func (s *State) GoodFirstIssues(w http.ResponseWriter, r *http.Request) { LoggedInUser: user, RepoGroups: paginatedGroups, LabelDefs: labelDefsMap, - Page: page, + Pagination: pagination, GfiLabel: labelDefsMap[goodFirstIssueLabel], }) } -- 2.43.0