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 whereClause = " where " + strings.Join(conditions, " and ") 99 } 100 101 - pLower := FilterGte("row_num", page.Offset+1) 102 - pUpper := FilterLte("row_num", page.Offset+page.Limit) 103 104 args = append(args, pLower.Arg()...) 105 args = append(args, pUpper.Arg()...) ··· 216 // get next issue_id 217 var newIssueId int 218 err := tx.QueryRow(` 219 - update repo_issue_seqs 220 - set next_issue_id = next_issue_id + 1 221 - where repo_at = ? 222 returning next_issue_id - 1 223 `, issue.RepoAt).Scan(&newIssueId) 224 if err != nil { ··· 244 } 245 246 func GetIssues(e Execer, filters ...filter) ([]models.Issue, error) { 247 - return GetIssuesPaginated(e, pagination.FirstPage(), filters...) 248 } 249 250 func GetIssue(e Execer, repoAt syntax.ATURI, issueId int) (*models.Issue, error) { ··· 261 whereClause = " where " + strings.Join(conditions, " and ") 262 } 263 264 - pLower := FilterGte("row_num", page.Count*page.No+1) 265 - pUpper := FilterLte("row_num", page.Count*(page.No+1)) 266 267 args = append(args, pLower.Arg()...) 268 args = append(args, pUpper.Arg()...) ··· 396 } 397 398 func GetIssues(e Execer, filters ...filter) ([]Issue, error) { 399 - return GetIssuesPaginated(e, pagination.Page{No: 0, Count: 10}, filters...) 400 } 401 402 func GetIssue(e Execer, repoAt syntax.ATURI, issueId int) (*Issue, error) {
··· 98 whereClause = " where " + strings.Join(conditions, " and ") 99 } 100 101 + pLower := FilterGte("row_num", page.Count*page.No+1) 102 + pUpper := FilterLte("row_num", page.Count*(page.No+1)) 103 104 args = append(args, pLower.Arg()...) 105 args = append(args, pUpper.Arg()...) ··· 216 // get next issue_id 217 var newIssueId int 218 err := tx.QueryRow(` 219 + update repo_issue_seqs 220 + set next_issue_id = next_issue_id + 1 221 + where repo_at = ? 222 returning next_issue_id - 1 223 `, issue.RepoAt).Scan(&newIssueId) 224 if err != nil { ··· 244 } 245 246 func GetIssues(e Execer, filters ...filter) ([]models.Issue, error) { 247 + return GetIssuesPaginated(e, pagination.Page{No: 0, Count: 30}, filters...) 248 } 249 250 func GetIssue(e Execer, repoAt syntax.ATURI, issueId int) (*models.Issue, error) { ··· 261 whereClause = " where " + strings.Join(conditions, " and ") 262 } 263 264 + pLower := FilterGte("row_num", page.Offset+1) 265 + pUpper := FilterLte("row_num", page.Offset+page.Limit) 266 267 args = append(args, pLower.Arg()...) 268 args = append(args, pUpper.Arg()...) ··· 396 } 397 398 func GetIssues(e Execer, filters ...filter) ([]Issue, error) { 399 + return GetIssuesPaginated(e, pagination.FirstPage(), filters...) 400 } 401 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 RepoGroups []*models.RepoGroup 321 LabelDefs map[string]*models.LabelDefinition 322 GfiLabel *models.LabelDefinition 323 - Page pagination.Page 324 } 325 326 func (p *Pages) GoodFirstIssues(w io.Writer, params GoodFirstIssuesParams) error { ··· 341 LoggedInUser *oauth.User 342 Notifications []*models.NotificationWithEntity 343 UnreadCount int 344 - Page pagination.Page 345 Total int64 346 } 347 ··· 866 RepoInfo repoinfo.RepoInfo 867 Active string 868 Issues []db.Issue 869 - Pagination pagination.Pagination 870 FilteringByOpen bool 871 } 872 ··· 968 Active string 969 Issues []models.Issue 970 LabelDefs map[string]*models.LabelDefinition 971 - Page pagination.Page 972 FilteringByOpen bool 973 } 974
··· 320 RepoGroups []*models.RepoGroup 321 LabelDefs map[string]*models.LabelDefinition 322 GfiLabel *models.LabelDefinition 323 + Pagination pagination.Pagination 324 } 325 326 func (p *Pages) GoodFirstIssues(w io.Writer, params GoodFirstIssuesParams) error { ··· 341 LoggedInUser *oauth.User 342 Notifications []*models.NotificationWithEntity 343 UnreadCount int 344 + Pagination pagination.Pagination 345 Total int64 346 } 347 ··· 866 RepoInfo repoinfo.RepoInfo 867 Active string 868 Issues []db.Issue 869 + Page pagination.Page 870 FilteringByOpen bool 871 } 872 ··· 968 Active string 969 Issues []models.Issue 970 LabelDefs map[string]*models.LabelDefinition 971 + Pagination pagination.Pagination 972 FilteringByOpen bool 973 } 974
+12 -13
appview/pages/templates/repo/issues/issues.html
··· 50 {{ $currentState = "open" }} 51 {{ end }} 52 53 - {{ if gt .Page.Offset 0 }} 54 - {{ $prev := .Page.Previous }} 55 <a 56 class="btn flex items-center gap-2 no-underline hover:no-underline dark:text-white dark:hover:bg-gray-700" 57 hx-boost="true" 58 - href = "/{{ $.RepoInfo.FullName }}/issues?state={{ $currentState }}&offset={{ $prev.Offset }}&limit={{ $prev.Limit }}" 59 > 60 {{ i "chevron-left" "w-4 h-4" }} 61 previous ··· 64 <div></div> 65 {{ end }} 66 67 - {{ if eq (len .Issues) .Page.Limit }} 68 - {{ $next := .Page.Next }} 69 <a 70 class="btn flex items-center gap-2 no-underline hover:no-underline dark:text-white dark:hover:bg-gray-700" 71 hx-boost="true" 72 - href = "/{{ $.RepoInfo.FullName }}/issues?state={{ $currentState }}&offset={{ $next.Offset }}&limit={{ $next.Limit }}" 73 > 74 next 75 {{ i "chevron-right" "w-4 h-4" }} ··· 87 {{ end }} 88 89 {{ define "pagination" }} 90 - {{ $currentPage := .Pagination.CurrentPage }} 91 <div class="flex justify-end mt-4 gap-2"> 92 {{ $currentState := "closed" }} 93 {{ if .FilteringByOpen }} 94 {{ $currentState = "open" }} 95 {{ end }} 96 97 - {{ if gt $currentPage.No 0 }} 98 - {{ $prev := .Pagination.PreviousPage }} 99 <a 100 class="btn flex items-center gap-2 no-underline hover:no-underline dark:text-white dark:hover:bg-gray-700" 101 hx-boost="true" 102 - href = "/{{ $.RepoInfo.FullName }}/issues?state={{ $currentState }}&page={{ add $prev.No 1 }}&count={{ $prev.Count }}" 103 > 104 {{ i "chevron-left" "w-4 h-4" }} 105 previous ··· 108 <div></div> 109 {{ end }} 110 111 - {{ if lt (add $currentPage.No 1) .Pagination.TotalPageCount }} 112 - {{ $next := .Pagination.NextPage }} 113 <a 114 class="btn flex items-center gap-2 no-underline hover:no-underline dark:text-white dark:hover:bg-gray-700" 115 hx-boost="true" 116 - href = "/{{ $.RepoInfo.FullName }}/issues?state={{ $currentState }}&page={{ add $next.No 1 }}&count={{ $next.Count }}" 117 > 118 next 119 {{ i "chevron-right" "w-4 h-4" }}
··· 50 {{ $currentState = "open" }} 51 {{ end }} 52 53 + {{ if .Pagination.HasPreviousPage }} 54 + {{ $prev := .Pagination.PreviousPage }} 55 <a 56 class="btn flex items-center gap-2 no-underline hover:no-underline dark:text-white dark:hover:bg-gray-700" 57 hx-boost="true" 58 + href = "/{{ $.RepoInfo.FullName }}/issues?state={{ $currentState }}&page={{ add $prev.No 1 }}&count={{ $prev.Count }}" 59 > 60 {{ i "chevron-left" "w-4 h-4" }} 61 previous ··· 64 <div></div> 65 {{ end }} 66 67 + {{ if .Pagination.HasNextPage }} 68 + {{ $next := .Pagination.NextPage }} 69 <a 70 class="btn flex items-center gap-2 no-underline hover:no-underline dark:text-white dark:hover:bg-gray-700" 71 hx-boost="true" 72 + href = "/{{ $.RepoInfo.FullName }}/issues?state={{ $currentState }}&page={{ add $next.No 1 }}&count={{ $next.Count }}" 73 > 74 next 75 {{ i "chevron-right" "w-4 h-4" }} ··· 87 {{ end }} 88 89 {{ define "pagination" }} 90 <div class="flex justify-end mt-4 gap-2"> 91 {{ $currentState := "closed" }} 92 {{ if .FilteringByOpen }} 93 {{ $currentState = "open" }} 94 {{ end }} 95 96 + {{ if gt .Page.Offset 0 }} 97 + {{ $prev := .Page.Previous }} 98 <a 99 class="btn flex items-center gap-2 no-underline hover:no-underline dark:text-white dark:hover:bg-gray-700" 100 hx-boost="true" 101 + href = "/{{ $.RepoInfo.FullName }}/issues?state={{ $currentState }}&offset={{ $prev.Offset }}&limit={{ $prev.Limit }}" 102 > 103 {{ i "chevron-left" "w-4 h-4" }} 104 previous ··· 107 <div></div> 108 {{ end }} 109 110 + {{ if eq (len .Issues) .Page.Limit }} 111 + {{ $next := .Page.Next }} 112 <a 113 class="btn flex items-center gap-2 no-underline hover:no-underline dark:text-white dark:hover:bg-gray-700" 114 hx-boost="true" 115 + href = "/{{ $.RepoInfo.FullName }}/issues?state={{ $currentState }}&offset={{ $next.Offset }}&limit={{ $next.Limit }}" 116 > 117 next 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 limit ? offset ? 69 `, whereClause) 70 71 - args = append(args, page.Limit, page.Offset) 72 73 rows, err := e.QueryContext(context.Background(), query, args...) 74 if err != nil { ··· 142 limit ? offset ? 143 `, whereClause) 144 145 - args = append(args, page.Limit, page.Offset) 146 147 rows, err := e.QueryContext(context.Background(), query, args...) 148 if err != nil { ··· 247 248 // GetNotifications retrieves notifications with filters 249 func GetNotifications(e Execer, filters ...filter) ([]*models.Notification, error) { 250 - return GetNotificationsPaginated(e, pagination.FirstPage(), filters...) 251 } 252 253 func CountNotifications(e Execer, filters ...filter) (int64, error) {
··· 68 limit ? offset ? 69 `, whereClause) 70 71 + args = append(args, page.Count, page.Count*page.No+1) 72 73 rows, err := e.QueryContext(context.Background(), query, args...) 74 if err != nil { ··· 142 limit ? offset ? 143 `, whereClause) 144 145 + args = append(args, page.Count, page.Count*page.No+1) 146 147 rows, err := e.QueryContext(context.Background(), query, args...) 148 if err != nil { ··· 247 248 // GetNotifications retrieves notifications with filters 249 func GetNotifications(e Execer, filters ...filter) ([]*models.Notification, error) { 250 + return GetNotificationsPaginated(e, pagination.Page{No: 0, Count: 30}, filters...) 251 } 252 253 func CountNotifications(e Execer, filters ...filter) (int64, error) {
+4 -3
appview/notifications/notifications.go
··· 34 35 r.Group(func(r chi.Router) { 36 r.Use(middleware.AuthMiddleware(n.oauth)) 37 - r.With(middleware.Paginate).Get("/", n.notificationsPage) 38 r.Post("/{id}/read", n.markRead) 39 r.Post("/read-all", n.markAllRead) 40 r.Delete("/{id}", n.deleteNotification) ··· 49 page, ok := r.Context().Value("page").(pagination.Page) 50 if !ok { 51 log.Println("failed to get page") 52 - page = pagination.FirstPage() 53 } 54 55 total, err := db.CountNotifications( ··· 61 n.pages.Error500(w) 62 return 63 } 64 65 notifications, err := db.GetNotificationsWithEntities( 66 n.db, ··· 84 LoggedInUser: user, 85 Notifications: notifications, 86 UnreadCount: unreadCount, 87 - Page: page, 88 Total: total, 89 }) 90 }
··· 34 35 r.Group(func(r chi.Router) { 36 r.Use(middleware.AuthMiddleware(n.oauth)) 37 + r.With(middleware.Paginate(30)).Get("/", n.notificationsPage) 38 r.Post("/{id}/read", n.markRead) 39 r.Post("/read-all", n.markAllRead) 40 r.Delete("/{id}", n.deleteNotification) ··· 49 page, ok := r.Context().Value("page").(pagination.Page) 50 if !ok { 51 log.Println("failed to get page") 52 + page = pagination.Page{No: 0, Count: 30} 53 } 54 55 total, err := db.CountNotifications( ··· 61 n.pages.Error500(w) 62 return 63 } 64 + paginate := pagination.NewFromPage(page, int(total)) 65 66 notifications, err := db.GetNotificationsWithEntities( 67 n.db, ··· 85 LoggedInUser: user, 86 Notifications: notifications, 87 UnreadCount: unreadCount, 88 + Pagination: paginate, 89 Total: total, 90 }) 91 }
+7 -7
appview/pages/templates/goodfirstissues/index.html
··· 130 </div> 131 {{ end }} 132 133 - {{ if or (gt .Page.Offset 0) (eq (len .RepoGroups) .Page.Limit) }} 134 <div class="flex justify-center mt-8"> 135 <div class="flex gap-2"> 136 - {{ if gt .Page.Offset 0 }} 137 - {{ $prev := .Page.Previous }} 138 <a 139 class="btn flex items-center gap-2 no-underline hover:no-underline dark:text-white dark:hover:bg-gray-700" 140 hx-boost="true" 141 - href="/goodfirstissues?offset={{ $prev.Offset }}&limit={{ $prev.Limit }}" 142 > 143 {{ i "chevron-left" "w-4 h-4" }} 144 previous ··· 147 <div></div> 148 {{ end }} 149 150 - {{ if eq (len .RepoGroups) .Page.Limit }} 151 - {{ $next := .Page.Next }} 152 <a 153 class="btn flex items-center gap-2 no-underline hover:no-underline dark:text-white dark:hover:bg-gray-700" 154 hx-boost="true" 155 - href="/goodfirstissues?offset={{ $next.Offset }}&limit={{ $next.Limit }}" 156 > 157 next 158 {{ i "chevron-right" "w-4 h-4" }}
··· 130 </div> 131 {{ end }} 132 133 + {{ if or .Pagination.HasPreviousPage .Pagination.HasNextPage }} 134 <div class="flex justify-center mt-8"> 135 <div class="flex gap-2"> 136 + {{ if .Pagination.HasPreviousPage }} 137 + {{ $prev := .Pagination.PreviousPage }} 138 <a 139 class="btn flex items-center gap-2 no-underline hover:no-underline dark:text-white dark:hover:bg-gray-700" 140 hx-boost="true" 141 + href="/goodfirstissues?page={{ add $prev.No 1 }}&count={{ $prev.Count }}" 142 > 143 {{ i "chevron-left" "w-4 h-4" }} 144 previous ··· 147 <div></div> 148 {{ end }} 149 150 + {{ if .Pagination.HasNextPage }} 151 + {{ $next := .Pagination.NextPage }} 152 <a 153 class="btn flex items-center gap-2 no-underline hover:no-underline dark:text-white dark:hover:bg-gray-700" 154 hx-boost="true" 155 + href="/goodfirstissues?page={{ add $next.No 1 }}&count={{ $next.Count }}" 156 > 157 next 158 {{ i "chevron-right" "w-4 h-4" }}
+6 -7
appview/pages/templates/notifications/list.html
··· 35 36 {{ define "pagination" }} 37 <div class="flex justify-end mt-4 gap-2"> 38 - {{ if gt .Page.Offset 0 }} 39 - {{ $prev := .Page.Previous }} 40 <a 41 class="btn flex items-center gap-2 no-underline hover:no-underline dark:text-white dark:hover:bg-gray-700" 42 hx-boost="true" 43 - href = "/notifications?offset={{ $prev.Offset }}&limit={{ $prev.Limit }}" 44 > 45 {{ i "chevron-left" "w-4 h-4" }} 46 previous ··· 49 <div></div> 50 {{ end }} 51 52 - {{ $next := .Page.Next }} 53 - {{ if lt $next.Offset .Total }} 54 - {{ $next := .Page.Next }} 55 <a 56 class="btn flex items-center gap-2 no-underline hover:no-underline dark:text-white dark:hover:bg-gray-700" 57 hx-boost="true" 58 - href = "/notifications?offset={{ $next.Offset }}&limit={{ $next.Limit }}" 59 > 60 next 61 {{ i "chevron-right" "w-4 h-4" }}
··· 35 36 {{ define "pagination" }} 37 <div class="flex justify-end mt-4 gap-2"> 38 + {{ if .Pagination.HasPreviousPage }} 39 + {{ $prev := .Pagination.PreviousPage }} 40 <a 41 class="btn flex items-center gap-2 no-underline hover:no-underline dark:text-white dark:hover:bg-gray-700" 42 hx-boost="true" 43 + href = "/notifications?page={{ add $prev.No 1 }}&count={{ $prev.Count }}" 44 > 45 {{ i "chevron-left" "w-4 h-4" }} 46 previous ··· 49 <div></div> 50 {{ end }} 51 52 + {{ if .Pagination.HasNextPage }} 53 + {{ $next := .Pagination.NextPage }} 54 <a 55 class="btn flex items-center gap-2 no-underline hover:no-underline dark:text-white dark:hover:bg-gray-700" 56 hx-boost="true" 57 + href = "/notifications?page={{ add $next.No 1 }}&count={{ $next.Count }}" 58 > 59 next 60 {{ i "chevron-right" "w-4 h-4" }}
+9 -6
appview/state/gfi.go
··· 20 21 page, ok := r.Context().Value("page").(pagination.Page) 22 if !ok { 23 - page = pagination.FirstPage() 24 } 25 26 goodFirstIssueLabel := fmt.Sprintf("at://%s/%s/%s", consts.TangledDid, tangled.LabelDefinitionNSID, "good-first-issue") ··· 37 LoggedInUser: user, 38 RepoGroups: []*models.RepoGroup{}, 39 LabelDefs: make(map[string]*models.LabelDefinition), 40 - Page: page, 41 }) 42 return 43 } ··· 50 allIssues, err := db.GetIssuesPaginated( 51 s.db, 52 pagination.Page{ 53 - Limit: 500, 54 }, 55 db.FilterIn("repo_at", repoUris), 56 db.FilterEq("open", 1), ··· 61 return 62 } 63 64 var goodFirstIssues []models.Issue 65 for _, issue := range allIssues { 66 if issue.Labels.ContainsLabel(goodFirstIssueLabel) { ··· 98 return sortedGroups[i].Repo.Name < sortedGroups[j].Repo.Name 99 }) 100 101 - groupStart := page.Offset 102 - groupEnd := page.Offset + page.Limit 103 if groupStart > len(sortedGroups) { 104 groupStart = len(sortedGroups) 105 } ··· 145 LoggedInUser: user, 146 RepoGroups: paginatedGroups, 147 LabelDefs: labelDefsMap, 148 - Page: page, 149 GfiLabel: labelDefsMap[goodFirstIssueLabel], 150 }) 151 }
··· 20 21 page, ok := r.Context().Value("page").(pagination.Page) 22 if !ok { 23 + page = pagination.Page{No: 0, Count: 30} 24 } 25 26 goodFirstIssueLabel := fmt.Sprintf("at://%s/%s/%s", consts.TangledDid, tangled.LabelDefinitionNSID, "good-first-issue") ··· 37 LoggedInUser: user, 38 RepoGroups: []*models.RepoGroup{}, 39 LabelDefs: make(map[string]*models.LabelDefinition), 40 + Pagination: pagination.NewFromPage(page, 0), 41 }) 42 return 43 } ··· 50 allIssues, err := db.GetIssuesPaginated( 51 s.db, 52 pagination.Page{ 53 + No: 0, 54 + Count: 500, 55 }, 56 db.FilterIn("repo_at", repoUris), 57 db.FilterEq("open", 1), ··· 62 return 63 } 64 65 + pagination := pagination.NewFromPage(page, len(allIssues)) 66 + 67 var goodFirstIssues []models.Issue 68 for _, issue := range allIssues { 69 if issue.Labels.ContainsLabel(goodFirstIssueLabel) { ··· 101 return sortedGroups[i].Repo.Name < sortedGroups[j].Repo.Name 102 }) 103 104 + groupStart := page.Count * page.No 105 + groupEnd := page.Count * (page.No + 1) 106 if groupStart > len(sortedGroups) { 107 groupStart = len(sortedGroups) 108 } ··· 148 LoggedInUser: user, 149 RepoGroups: paginatedGroups, 150 LabelDefs: labelDefsMap, 151 + Pagination: pagination, 152 GfiLabel: labelDefsMap[goodFirstIssueLabel], 153 }) 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
ptr.pet submitted #2
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.