Monorepo for Tangled tangled.org

appview: improve pagination.Page usage #519

closed opened by ptr.pet targeting master from [deleted fork]: pipeline-paginated
Labels

None yet.

assignee

None yet.

Participants 2
AT URI
at://did:plc:dfl62fgb7wtjj3fcbb72naae/sh.tangled.repo.pull/3lwubheua2u22
+54 -52
Interdiff #1 โ†’ #2
+9 -9
appview/db/issues.go
··· 98 98 whereClause = " where " + strings.Join(conditions, " and ") 99 99 } 100 100 101 - pLower := FilterGte("row_num", page.Offset+1) 102 - pUpper := FilterLte("row_num", page.Offset+page.Limit) 101 + pLower := FilterGte("row_num", page.Count*page.No+1) 102 + pUpper := FilterLte("row_num", page.Count*(page.No+1)) 103 103 104 104 args = append(args, pLower.Arg()...) 105 105 args = append(args, pUpper.Arg()...) ··· 216 216 // get next issue_id 217 217 var newIssueId int 218 218 err := tx.QueryRow(` 219 - update repo_issue_seqs 220 - set next_issue_id = next_issue_id + 1 221 - where repo_at = ? 219 + update repo_issue_seqs 220 + set next_issue_id = next_issue_id + 1 221 + where repo_at = ? 222 222 returning next_issue_id - 1 223 223 `, issue.RepoAt).Scan(&newIssueId) 224 224 if err != nil { ··· 244 244 } 245 245 246 246 func GetIssues(e Execer, filters ...filter) ([]models.Issue, error) { 247 - return GetIssuesPaginated(e, pagination.FirstPage(), filters...) 247 + return GetIssuesPaginated(e, pagination.Page{No: 0, Count: 30}, filters...) 248 248 } 249 249 250 250 func GetIssue(e Execer, repoAt syntax.ATURI, issueId int) (*models.Issue, error) { ··· 261 261 whereClause = " where " + strings.Join(conditions, " and ") 262 262 } 263 263 264 - pLower := FilterGte("row_num", page.Count*page.No+1) 265 - pUpper := FilterLte("row_num", page.Count*(page.No+1)) 264 + pLower := FilterGte("row_num", page.Offset+1) 265 + pUpper := FilterLte("row_num", page.Offset+page.Limit) 266 266 267 267 args = append(args, pLower.Arg()...) 268 268 args = append(args, pUpper.Arg()...) ··· 396 396 } 397 397 398 398 func GetIssues(e Execer, filters ...filter) ([]Issue, error) { 399 - return GetIssuesPaginated(e, pagination.Page{No: 0, Count: 10}, filters...) 399 + return GetIssuesPaginated(e, pagination.FirstPage(), filters...) 400 400 } 401 401 402 402 func GetIssue(e Execer, repoAt syntax.ATURI, issueId int) (*Issue, error) {
appview/issues/issues.go

This patch was likely rebased, as context lines do not match.

appview/issues/router.go

This patch was likely rebased, as context lines do not match.

appview/middleware/middleware.go

This file has not been changed.

+4 -4
appview/pages/pages.go
··· 320 320 RepoGroups []*models.RepoGroup 321 321 LabelDefs map[string]*models.LabelDefinition 322 322 GfiLabel *models.LabelDefinition 323 - Page pagination.Page 323 + Pagination pagination.Pagination 324 324 } 325 325 326 326 func (p *Pages) GoodFirstIssues(w io.Writer, params GoodFirstIssuesParams) error { ··· 341 341 LoggedInUser *oauth.User 342 342 Notifications []*models.NotificationWithEntity 343 343 UnreadCount int 344 - Page pagination.Page 344 + Pagination pagination.Pagination 345 345 Total int64 346 346 } 347 347 ··· 866 866 RepoInfo repoinfo.RepoInfo 867 867 Active string 868 868 Issues []db.Issue 869 - Pagination pagination.Pagination 869 + Page pagination.Page 870 870 FilteringByOpen bool 871 871 } 872 872 ··· 968 968 Active string 969 969 Issues []models.Issue 970 970 LabelDefs map[string]*models.LabelDefinition 971 - Page pagination.Page 971 + Pagination pagination.Pagination 972 972 FilteringByOpen bool 973 973 } 974 974
+12 -13
appview/pages/templates/repo/issues/issues.html
··· 50 50 {{ $currentState = "open" }} 51 51 {{ end }} 52 52 53 - {{ if gt .Page.Offset 0 }} 54 - {{ $prev := .Page.Previous }} 53 + {{ if .Pagination.HasPreviousPage }} 54 + {{ $prev := .Pagination.PreviousPage }} 55 55 <a 56 56 class="btn flex items-center gap-2 no-underline hover:no-underline dark:text-white dark:hover:bg-gray-700" 57 57 hx-boost="true" 58 - href = "/{{ $.RepoInfo.FullName }}/issues?state={{ $currentState }}&offset={{ $prev.Offset }}&limit={{ $prev.Limit }}" 58 + href = "/{{ $.RepoInfo.FullName }}/issues?state={{ $currentState }}&page={{ add $prev.No 1 }}&count={{ $prev.Count }}" 59 59 > 60 60 {{ i "chevron-left" "w-4 h-4" }} 61 61 previous ··· 64 64 <div></div> 65 65 {{ end }} 66 66 67 - {{ if eq (len .Issues) .Page.Limit }} 68 - {{ $next := .Page.Next }} 67 + {{ if .Pagination.HasNextPage }} 68 + {{ $next := .Pagination.NextPage }} 69 69 <a 70 70 class="btn flex items-center gap-2 no-underline hover:no-underline dark:text-white dark:hover:bg-gray-700" 71 71 hx-boost="true" 72 - href = "/{{ $.RepoInfo.FullName }}/issues?state={{ $currentState }}&offset={{ $next.Offset }}&limit={{ $next.Limit }}" 72 + href = "/{{ $.RepoInfo.FullName }}/issues?state={{ $currentState }}&page={{ add $next.No 1 }}&count={{ $next.Count }}" 73 73 > 74 74 next 75 75 {{ i "chevron-right" "w-4 h-4" }} ··· 87 87 {{ end }} 88 88 89 89 {{ define "pagination" }} 90 - {{ $currentPage := .Pagination.CurrentPage }} 91 90 <div class="flex justify-end mt-4 gap-2"> 92 91 {{ $currentState := "closed" }} 93 92 {{ if .FilteringByOpen }} 94 93 {{ $currentState = "open" }} 95 94 {{ end }} 96 95 97 - {{ if gt $currentPage.No 0 }} 98 - {{ $prev := .Pagination.PreviousPage }} 96 + {{ if gt .Page.Offset 0 }} 97 + {{ $prev := .Page.Previous }} 99 98 <a 100 99 class="btn flex items-center gap-2 no-underline hover:no-underline dark:text-white dark:hover:bg-gray-700" 101 100 hx-boost="true" 102 - href = "/{{ $.RepoInfo.FullName }}/issues?state={{ $currentState }}&page={{ add $prev.No 1 }}&count={{ $prev.Count }}" 101 + href = "/{{ $.RepoInfo.FullName }}/issues?state={{ $currentState }}&offset={{ $prev.Offset }}&limit={{ $prev.Limit }}" 103 102 > 104 103 {{ i "chevron-left" "w-4 h-4" }} 105 104 previous ··· 108 107 <div></div> 109 108 {{ end }} 110 109 111 - {{ if lt (add $currentPage.No 1) .Pagination.TotalPageCount }} 112 - {{ $next := .Pagination.NextPage }} 110 + {{ if eq (len .Issues) .Page.Limit }} 111 + {{ $next := .Page.Next }} 113 112 <a 114 113 class="btn flex items-center gap-2 no-underline hover:no-underline dark:text-white dark:hover:bg-gray-700" 115 114 hx-boost="true" 116 - href = "/{{ $.RepoInfo.FullName }}/issues?state={{ $currentState }}&page={{ add $next.No 1 }}&count={{ $next.Count }}" 115 + href = "/{{ $.RepoInfo.FullName }}/issues?state={{ $currentState }}&offset={{ $next.Offset }}&limit={{ $next.Limit }}" 117 116 > 118 117 next 119 118 {{ i "chevron-right" "w-4 h-4" }}
appview/pagination/page.go

This patch was likely rebased, as context lines do not match.

appview/repo/feed.go

This patch was likely rebased, as context lines do not match.

+3 -3
appview/db/notifications.go
··· 68 68 limit ? offset ? 69 69 `, whereClause) 70 70 71 - args = append(args, page.Limit, page.Offset) 71 + args = append(args, page.Count, page.Count*page.No+1) 72 72 73 73 rows, err := e.QueryContext(context.Background(), query, args...) 74 74 if err != nil { ··· 142 142 limit ? offset ? 143 143 `, whereClause) 144 144 145 - args = append(args, page.Limit, page.Offset) 145 + args = append(args, page.Count, page.Count*page.No+1) 146 146 147 147 rows, err := e.QueryContext(context.Background(), query, args...) 148 148 if err != nil { ··· 247 247 248 248 // GetNotifications retrieves notifications with filters 249 249 func GetNotifications(e Execer, filters ...filter) ([]*models.Notification, error) { 250 - return GetNotificationsPaginated(e, pagination.FirstPage(), filters...) 250 + return GetNotificationsPaginated(e, pagination.Page{No: 0, Count: 30}, filters...) 251 251 } 252 252 253 253 func CountNotifications(e Execer, filters ...filter) (int64, error) {
+4 -3
appview/notifications/notifications.go
··· 34 34 35 35 r.Group(func(r chi.Router) { 36 36 r.Use(middleware.AuthMiddleware(n.oauth)) 37 - r.With(middleware.Paginate).Get("/", n.notificationsPage) 37 + r.With(middleware.Paginate(30)).Get("/", n.notificationsPage) 38 38 r.Post("/{id}/read", n.markRead) 39 39 r.Post("/read-all", n.markAllRead) 40 40 r.Delete("/{id}", n.deleteNotification) ··· 49 49 page, ok := r.Context().Value("page").(pagination.Page) 50 50 if !ok { 51 51 log.Println("failed to get page") 52 - page = pagination.FirstPage() 52 + page = pagination.Page{No: 0, Count: 30} 53 53 } 54 54 55 55 total, err := db.CountNotifications( ··· 61 61 n.pages.Error500(w) 62 62 return 63 63 } 64 + paginate := pagination.NewFromPage(page, int(total)) 64 65 65 66 notifications, err := db.GetNotificationsWithEntities( 66 67 n.db, ··· 84 85 LoggedInUser: user, 85 86 Notifications: notifications, 86 87 UnreadCount: unreadCount, 87 - Page: page, 88 + Pagination: paginate, 88 89 Total: total, 89 90 }) 90 91 }
+7 -7
appview/pages/templates/goodfirstissues/index.html
··· 130 130 </div> 131 131 {{ end }} 132 132 133 - {{ if or (gt .Page.Offset 0) (eq (len .RepoGroups) .Page.Limit) }} 133 + {{ if or .Pagination.HasPreviousPage .Pagination.HasNextPage }} 134 134 <div class="flex justify-center mt-8"> 135 135 <div class="flex gap-2"> 136 - {{ if gt .Page.Offset 0 }} 137 - {{ $prev := .Page.Previous }} 136 + {{ if .Pagination.HasPreviousPage }} 137 + {{ $prev := .Pagination.PreviousPage }} 138 138 <a 139 139 class="btn flex items-center gap-2 no-underline hover:no-underline dark:text-white dark:hover:bg-gray-700" 140 140 hx-boost="true" 141 - href="/goodfirstissues?offset={{ $prev.Offset }}&limit={{ $prev.Limit }}" 141 + href="/goodfirstissues?page={{ add $prev.No 1 }}&count={{ $prev.Count }}" 142 142 > 143 143 {{ i "chevron-left" "w-4 h-4" }} 144 144 previous ··· 147 147 <div></div> 148 148 {{ end }} 149 149 150 - {{ if eq (len .RepoGroups) .Page.Limit }} 151 - {{ $next := .Page.Next }} 150 + {{ if .Pagination.HasNextPage }} 151 + {{ $next := .Pagination.NextPage }} 152 152 <a 153 153 class="btn flex items-center gap-2 no-underline hover:no-underline dark:text-white dark:hover:bg-gray-700" 154 154 hx-boost="true" 155 - href="/goodfirstissues?offset={{ $next.Offset }}&limit={{ $next.Limit }}" 155 + href="/goodfirstissues?page={{ add $next.No 1 }}&count={{ $next.Count }}" 156 156 > 157 157 next 158 158 {{ i "chevron-right" "w-4 h-4" }}
+6 -7
appview/pages/templates/notifications/list.html
··· 35 35 36 36 {{ define "pagination" }} 37 37 <div class="flex justify-end mt-4 gap-2"> 38 - {{ if gt .Page.Offset 0 }} 39 - {{ $prev := .Page.Previous }} 38 + {{ if .Pagination.HasPreviousPage }} 39 + {{ $prev := .Pagination.PreviousPage }} 40 40 <a 41 41 class="btn flex items-center gap-2 no-underline hover:no-underline dark:text-white dark:hover:bg-gray-700" 42 42 hx-boost="true" 43 - href = "/notifications?offset={{ $prev.Offset }}&limit={{ $prev.Limit }}" 43 + href = "/notifications?page={{ add $prev.No 1 }}&count={{ $prev.Count }}" 44 44 > 45 45 {{ i "chevron-left" "w-4 h-4" }} 46 46 previous ··· 49 49 <div></div> 50 50 {{ end }} 51 51 52 - {{ $next := .Page.Next }} 53 - {{ if lt $next.Offset .Total }} 54 - {{ $next := .Page.Next }} 52 + {{ if .Pagination.HasNextPage }} 53 + {{ $next := .Pagination.NextPage }} 55 54 <a 56 55 class="btn flex items-center gap-2 no-underline hover:no-underline dark:text-white dark:hover:bg-gray-700" 57 56 hx-boost="true" 58 - href = "/notifications?offset={{ $next.Offset }}&limit={{ $next.Limit }}" 57 + href = "/notifications?page={{ add $next.No 1 }}&count={{ $next.Count }}" 59 58 > 60 59 next 61 60 {{ i "chevron-right" "w-4 h-4" }}
+9 -6
appview/state/gfi.go
··· 20 20 21 21 page, ok := r.Context().Value("page").(pagination.Page) 22 22 if !ok { 23 - page = pagination.FirstPage() 23 + page = pagination.Page{No: 0, Count: 30} 24 24 } 25 25 26 26 goodFirstIssueLabel := fmt.Sprintf("at://%s/%s/%s", consts.TangledDid, tangled.LabelDefinitionNSID, "good-first-issue") ··· 37 37 LoggedInUser: user, 38 38 RepoGroups: []*models.RepoGroup{}, 39 39 LabelDefs: make(map[string]*models.LabelDefinition), 40 - Page: page, 40 + Pagination: pagination.NewFromPage(page, 0), 41 41 }) 42 42 return 43 43 } ··· 50 50 allIssues, err := db.GetIssuesPaginated( 51 51 s.db, 52 52 pagination.Page{ 53 - Limit: 500, 53 + No: 0, 54 + Count: 500, 54 55 }, 55 56 db.FilterIn("repo_at", repoUris), 56 57 db.FilterEq("open", 1), ··· 61 62 return 62 63 } 63 64 65 + pagination := pagination.NewFromPage(page, len(allIssues)) 66 + 64 67 var goodFirstIssues []models.Issue 65 68 for _, issue := range allIssues { 66 69 if issue.Labels.ContainsLabel(goodFirstIssueLabel) { ··· 98 101 return sortedGroups[i].Repo.Name < sortedGroups[j].Repo.Name 99 102 }) 100 103 101 - groupStart := page.Offset 102 - groupEnd := page.Offset + page.Limit 104 + groupStart := page.Count * page.No 105 + groupEnd := page.Count * (page.No + 1) 103 106 if groupStart > len(sortedGroups) { 104 107 groupStart = len(sortedGroups) 105 108 } ··· 145 148 LoggedInUser: user, 146 149 RepoGroups: paginatedGroups, 147 150 LabelDefs: labelDefsMap, 148 - Page: page, 151 + Pagination: pagination, 149 152 GfiLabel: labelDefsMap[goodFirstIssueLabel], 150 153 }) 151 154 }

History

4 rounds 2 comments
sign up or login to add to the discussion
1 commit
expand
eda039f3
appview: improve pagination API
expand 0 comments
closed without merging
1 commit
expand
93bdc6bc
appview: improve pagination API
expand 0 comments
1 commit
expand
1af31ddd
appview: improve pagination API
expand 1 comment

i went with a page number api and made pagination produce pages instead, i think it fits it better (pagination still uses offset / limit / total though ofc)

ptr.pet submitted #0
1 commit
expand
8ee73a8a
appview: improve pagination.Page usage
expand 1 comment

this helper is quite strange. can we use a slightly better API? the page size does not belong in the FirstPage method (although i understand it was previously hardcoded here).

we can do something like p := NewPagination(WithSize(N), WithOffset(0), WithTotal(...)) and have p.FirstPage() perhaps.