Skip to content

Commit 36e7476

Browse files
ChrisPennerChrisPenner
authored
Merge pull request #143 from unisoncomputing/cp/fix-branch-paging
Fix branch paging
2 parents e332966 + 4eae1e7 commit 36e7476

17 files changed

+338
-138
lines changed

‎src/Share/Postgres/Queries.hs‎

Lines changed: 126 additions & 107 deletions
Original file line numberDiff line numberDiff line change
@@ -707,59 +707,69 @@ listBranchesByProject ::
707707
BranchKindFilter ->
708708
ProjectId ->
709709
-- | (branch, contributorHandle)
710-
PG.Transaction e [(Branch CausalId, Maybe UserHandle)]
710+
PG.Transaction e (Paged (UTCTime, BranchId) (Branch CausalId, Maybe UserHandle))
711711
listBranchesByProject limit mayCursor mayBranchNamePrefix mayContributorQuery kind projectId = do
712-
let kindFilter = case kind of
713-
AllBranchKinds -> ""
714-
OnlyContributorBranches -> "AND b.contributor_id IS NOT NULL"
715-
OnlyCoreBranches -> "AND b.contributor_id IS NULL"
716-
let contributorFilter = case mayContributorQuery of
717-
Nothing -> mempty
718-
-- Allow null contributor here for the case where we're listing 'all' branch kinds.
719-
Just (Left contributorId) -> [PG.sql| AND (b.contributor_id IS NULL OR (b.contributor_id = #{contributorId})) |]
720-
Just (Right (Query contributorHandlePrefix)) -> [PG.sql| AND (contributor.handle IS NULL OR starts_with(contributor.handle, #{contributorHandlePrefix})) |]
721-
let branchNameFilter = case mayBranchNamePrefix of
722-
Nothing -> mempty
723-
Just (Query branchNamePrefix) -> [PG.sql| AND starts_with(b.name, #{branchNamePrefix}) |]
724-
let cursorFilter = case mayCursor of
725-
Nothing -> mempty
726-
Just (Cursor (beforeTime, branchId) Previous) -> [PG.sql| AND (b.updated_at, b.id) < (#{beforeTime}, #{branchId})|]
727-
Just (Cursor (afterTime, branchId) Next) -> [PG.sql| AND (b.updated_at, b.id) > (#{afterTime}, #{branchId})|]
728-
let sql =
729-
intercalateMap
730-
"\n"
731-
id
732-
[ ( [PG.sql|
733-
SELECT
734-
b.id,
735-
b.project_id,
736-
b.name,
737-
b.contributor_id,
738-
b.causal_id,
739-
b.merge_target_branch_id,
740-
b.created_at,
741-
b.updated_at,
742-
b.creator_id,
743-
contributor.handle
744-
FROM project_branches b
745-
LEFT JOIN users AS contributor ON contributor.id = b.contributor_id
746-
WHERE
747-
b.deleted_at IS NULL
748-
AND b.project_id = #{projectId}
712+
results <- query limit (mkCursorFilter mayCursor)
713+
let paged@(Paged {prevCursor, nextCursor}) =
714+
results
715+
& pagedOn (\(Branch {updatedAt, branchId}, _) -> (updatedAt, branchId))
716+
hasPrevPage <- not . null <$> query 1 (mkCursorFilter prevCursor)
717+
hasNextPage <- not . null <$> query 1 (mkCursorFilter nextCursor)
718+
pure $ guardPaged hasPrevPage hasNextPage paged
719+
where
720+
mkCursorFilter cursor = case cursor of
721+
Nothing -> mempty
722+
Just (Cursor (afterTime, branchId) Previous) -> [PG.sql| AND (b.updated_at, b.id) > (#{afterTime}, #{branchId})|]
723+
Just (Cursor (beforeTime, branchId) Next) -> [PG.sql| AND (b.updated_at, b.id) < (#{beforeTime}, #{branchId})|]
724+
kindFilter = case kind of
725+
AllBranchKinds -> ""
726+
OnlyContributorBranches -> "AND b.contributor_id IS NOT NULL"
727+
OnlyCoreBranches -> "AND b.contributor_id IS NULL"
728+
contributorFilter = case mayContributorQuery of
729+
Nothing -> mempty
730+
-- Allow null contributor here for the case where we're listing 'all' branch kinds.
731+
Just (Left contributorId) -> [PG.sql| AND (b.contributor_id IS NULL OR (b.contributor_id = #{contributorId})) |]
732+
Just (Right (Query contributorHandlePrefix)) -> [PG.sql| AND (contributor.handle IS NULL OR starts_with(contributor.handle, #{contributorHandlePrefix})) |]
733+
branchNameFilter = case mayBranchNamePrefix of
734+
Nothing -> mempty
735+
Just (Query branchNamePrefix) -> [PG.sql| AND starts_with(b.name, #{branchNamePrefix}) |]
736+
query :: Limit -> PG.Sql -> PG.Transaction e [(Branch CausalId, Maybe UserHandle)]
737+
query limit cursorFilter = do
738+
let sql =
739+
intercalateMap
740+
"\n"
741+
id
742+
[ ( [PG.sql|
743+
SELECT
744+
b.id,
745+
b.project_id,
746+
b.name,
747+
b.contributor_id,
748+
b.causal_id,
749+
b.merge_target_branch_id,
750+
b.created_at,
751+
b.updated_at,
752+
b.creator_id,
753+
contributor.handle
754+
FROM project_branches b
755+
LEFT JOIN users AS contributor ON contributor.id = b.contributor_id
756+
WHERE
757+
b.deleted_at IS NULL
758+
AND b.project_id = #{projectId}
759+
|]
760+
),
761+
kindFilter,
762+
contributorFilter,
763+
branchNameFilter,
764+
cursorFilter,
765+
( [PG.sql|
766+
ORDER BY b.updated_at DESC, b.id DESC
767+
LIMIT #{limit}
749768
|]
750-
),
751-
kindFilter,
752-
contributorFilter,
753-
branchNameFilter,
754-
cursorFilter,
755-
( [PG.sql|
756-
ORDER BY b.updated_at DESC, b.id DESC
757-
LIMIT #{limit}
758-
|]
759-
)
760-
]
761-
PG.queryListRows sql
762-
<&> fmap (\(branch PG.:. PG.Only contributorHandle) -> (branch, contributorHandle))
769+
)
770+
]
771+
PG.queryListRows sql
772+
<&> fmap (\(branch PG.:. PG.Only contributorHandle) -> (branch, contributorHandle))
763773

764774
-- | List all BranchHashes which are reachable within a given user's codebase.
765775
accessibleCausalsForUser ::
@@ -817,63 +827,72 @@ listContributorBranchesOfUserAccessibleToCaller ::
817827
Maybe (Cursor (UTCTime, BranchId)) ->
818828
Maybe Query ->
819829
Maybe ProjectId ->
820-
-- | (branch, project, projectOwnerHandle)
821-
PG.Transaction e [(Branch CausalId, Project, ProjectOwner)]
830+
PG.Transaction e (Paged (UTCTime, BranchId) (Branch CausalId, Project, ProjectOwner))
822831
listContributorBranchesOfUserAccessibleToCaller contributorUserId mayCallerUserId limit mayCursor mayBranchNamePrefix mayProjectId = do
823-
let branchNameFilter = case mayBranchNamePrefix of
824-
Nothing -> mempty
825-
Just (Query branchNamePrefix) -> [PG.sql| AND starts_with(b.name, #{branchNamePrefix}) |]
826-
let cursorFilter = case mayCursor of
827-
Nothing -> mempty
828-
Just (Cursor (beforeTime, branchId) Previous) -> [PG.sql| AND (b.updated_at, b.id) < (#{beforeTime}, #{branchId}) |]
829-
Just (Cursor (afterTime, branchId) Next) -> [PG.sql| AND (b.updated_at, b.id) > (#{afterTime}, #{branchId}) |]
830-
let projectFilter = case mayProjectId of
831-
Nothing -> mempty
832-
Just projId -> [PG.sql| AND b.project_id = #{projId} |]
833-
let sql =
834-
intercalateMap
835-
"\n"
836-
id
837-
[ [PG.sql|
838-
SELECT
839-
b.id,
840-
b.project_id,
841-
b.name,
842-
b.contributor_id,
843-
b.causal_id,
844-
b.merge_target_branch_id,
845-
b.created_at,
846-
b.updated_at,
847-
b.creator_id,
848-
project.id,
849-
project.owner_user_id,
850-
project.slug,
851-
project.summary,
852-
project.tags,
853-
project.private,
854-
project.created_at,
855-
project.updated_at,
856-
project_owner.handle,
857-
project_owner.name,
858-
EXISTS (SELECT FROM org_members WHERE org_members.organization_user_id = project.owner_user_id)
859-
FROM project_branches b
860-
JOIN projects project ON project.id = b.project_id
861-
JOIN users AS project_owner ON project_owner.id = project.owner_user_id
862-
WHERE
863-
b.deleted_at IS NULL
864-
AND b.contributor_id = #{contributorUserId}
865-
AND user_has_project_permission(#{mayCallerUserId}, b.project_id, #{ProjectView})
866-
|],
867-
branchNameFilter,
868-
cursorFilter,
869-
projectFilter,
870-
[PG.sql|
871-
ORDER BY b.updated_at DESC, b.id DESC
872-
LIMIT #{limit}
873-
|]
874-
]
875-
PG.queryListRows sql
876-
<&> fmap (\(branch PG.:. project PG.:. projectOwner) -> (branch, project, projectOwner))
832+
results <- query limit (mkCursorFilter mayCursor)
833+
let paged@(Paged {prevCursor, nextCursor}) =
834+
results
835+
& pagedOn (\(Branch {updatedAt, branchId}, _, _) -> (updatedAt, branchId))
836+
hasPrevPage <- not . null <$> query 1 (mkCursorFilter prevCursor)
837+
hasNextPage <- not . null <$> query 1 (mkCursorFilter nextCursor)
838+
pure $ guardPaged hasPrevPage hasNextPage paged
839+
where
840+
branchNameFilter = case mayBranchNamePrefix of
841+
Nothing -> mempty
842+
Just (Query branchNamePrefix) -> [PG.sql| AND starts_with(b.name, #{branchNamePrefix}) |]
843+
mkCursorFilter cursor = case cursor of
844+
Nothing -> mempty
845+
Just (Cursor (afterTime, branchId) Previous) -> [PG.sql| AND (b.updated_at, b.id) > (#{afterTime}, #{branchId}) |]
846+
Just (Cursor (beforeTime, branchId) Next) -> [PG.sql| AND (b.updated_at, b.id) < (#{beforeTime}, #{branchId}) |]
847+
projectFilter = case mayProjectId of
848+
Nothing -> mempty
849+
Just projId -> [PG.sql| AND b.project_id = #{projId} |]
850+
query :: Limit -> PG.Sql -> PG.Transaction e [(Branch CausalId, Project, ProjectOwner)]
851+
query limit cursorFilter = do
852+
let sql =
853+
intercalateMap
854+
"\n"
855+
id
856+
[ [PG.sql|
857+
SELECT
858+
b.id,
859+
b.project_id,
860+
b.name,
861+
b.contributor_id,
862+
b.causal_id,
863+
b.merge_target_branch_id,
864+
b.created_at,
865+
b.updated_at,
866+
b.creator_id,
867+
project.id,
868+
project.owner_user_id,
869+
project.slug,
870+
project.summary,
871+
project.tags,
872+
project.private,
873+
project.created_at,
874+
project.updated_at,
875+
project_owner.handle,
876+
project_owner.name,
877+
EXISTS (SELECT FROM org_members WHERE org_members.organization_user_id = project.owner_user_id)
878+
FROM project_branches b
879+
JOIN projects project ON project.id = b.project_id
880+
JOIN users AS project_owner ON project_owner.id = project.owner_user_id
881+
WHERE
882+
b.deleted_at IS NULL
883+
AND b.contributor_id = #{contributorUserId}
884+
AND user_has_project_permission(#{mayCallerUserId}, b.project_id, #{ProjectView})
885+
|],
886+
branchNameFilter,
887+
cursorFilter,
888+
projectFilter,
889+
[PG.sql|
890+
ORDER BY b.updated_at DESC, b.id DESC
891+
LIMIT #{limit}
892+
|]
893+
]
894+
PG.queryListRows sql
895+
<&> fmap (\(branch PG.:. project PG.:. projectOwner) -> (branch, project, projectOwner))
877896

878897
-- | Returns Project Owner information, including whether that user is an organization or not.
879898
projectOwnerByProjectId :: ProjectId -> PG.Transaction e (Maybe ProjectOwner)

‎src/Share/Web/Share/Branches/Impl.hs‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ import Share.Web.Share.Branches.API qualified as API
4343
import Share.Web.Share.Branches.Types (BranchKindFilter (..), ShareBranch (..))
4444
import Share.Web.Share.Branches.Types qualified as API
4545
import Share.Web.Share.CodeBrowsing.API qualified as API
46+
import Share.Web.Share.Contributions.Types
47+
import Share.Web.Share.DisplayInfo.Types
4648
import Share.Web.Share.Projects.Types (projectToAPI)
4749
import Share.Web.Share.Types
4850
import U.Codebase.HashTags (CausalHash)
@@ -535,7 +537,7 @@ listBranchesByProjectEndpoint (AuthN.MaybeAuthedUserID callerUserId) userHandle
535537
(mayNamePrefix, mayContributorFilter) <- computeSearchFilters
536538
branches <- PG.runTransaction do
537539
branches <- Q.listBranchesByProject limit mayCursor mayNamePrefix mayContributorFilter (fromMaybe defaultKindFilter mayKindFilter) projectId
538-
branchesWithContributions <-
540+
branchesWithContributions :: (Paged (UTCTime, BranchId) (Branch CausalId, [ShareContribution UserDisplayInfo], Maybe UserHandle)) <-
539541
branches
540542
& fmap (\(branch@(Branch {branchId}), contributorHandle) -> (branch, branchId, contributorHandle))
541543
& ContributionsQ.shareContributionsByBranchOf (traversed . _2)
@@ -549,10 +551,7 @@ listBranchesByProjectEndpoint (AuthN.MaybeAuthedUserID callerUserId) userHandle
549551
let branchShortHand = BranchShortHand {branchName, contributorHandle}
550552
in API.branchToShareBranch branchShortHand branch shareProject contributions
551553
)
552-
branches
553-
& pagedOn (\(Branch {updatedAt, branchId}, _, _) -> (updatedAt, branchId))
554-
& (\p -> p {items = shareBranches})
555-
& pure
554+
pure shareBranches
556555
where
557556
userIdForHandle handle = do
558557
fmap user_id <$> PG.runTransaction (UserQ.userByHandle handle)
@@ -628,10 +627,7 @@ listBranchesByUserEndpoint (AuthN.MaybeAuthedUserID callerUserId) contributorHan
628627
shareProject = projectToAPI projectOwnerHandle project
629628
in API.branchToShareBranch branchShortHand branch shareProject contributions
630629
)
631-
expandedBranches
632-
& pagedOn ((\(Branch {updatedAt, branchId}, _contr, _proj, _projOwner) -> (updatedAt, branchId)))
633-
& (\p -> p {items = shareBranches})
634-
& pure
630+
pure shareBranches
635631
where
636632
defaultLimit = Limit 20
637633
limit = fromMaybe defaultLimit mayLimit

‎transcripts/share-apis/branches/branch-list-by-self-name-prefix.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,8 @@
6161
"updatedAt": "<TIMESTAMP>"
6262
}
6363
],
64-
"nextCursor": "<CURSOR>",
65-
"prevCursor": "<CURSOR>"
64+
"nextCursor": null,
65+
"prevCursor": null
6666
},
6767
"status": [
6868
{

‎transcripts/share-apis/branches/branch-list-by-self-project-ref.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,8 @@
6161
"updatedAt": "<TIMESTAMP>"
6262
}
6363
],
64-
"nextCursor": "<CURSOR>",
65-
"prevCursor": "<CURSOR>"
64+
"nextCursor": null,
65+
"prevCursor": null
6666
},
6767
"status": [
6868
{

‎transcripts/share-apis/branches/branch-list-by-user-self.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,8 @@
8181
"updatedAt": "<TIMESTAMP>"
8282
}
8383
],
84-
"nextCursor": "<CURSOR>",
85-
"prevCursor": "<CURSOR>"
84+
"nextCursor": null,
85+
"prevCursor": null
8686
},
8787
"status": [
8888
{

‎transcripts/share-apis/branches/branch-list-contributor-filter.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,8 @@
101101
"updatedAt": "<TIMESTAMP>"
102102
}
103103
],
104-
"nextCursor": "<CURSOR>",
105-
"prevCursor": "<CURSOR>"
104+
"nextCursor": null,
105+
"prevCursor": null
106106
},
107107
"status": [
108108
{

‎transcripts/share-apis/branches/branch-list-contributor-prefix.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,8 @@
101101
"updatedAt": "<TIMESTAMP>"
102102
}
103103
],
104-
"nextCursor": "<CURSOR>",
105-
"prevCursor": "<CURSOR>"
104+
"nextCursor": null,
105+
"prevCursor": null
106106
},
107107
"status": [
108108
{

‎transcripts/share-apis/branches/branch-list-contributors-only.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,8 @@
6161
"updatedAt": "<TIMESTAMP>"
6262
}
6363
],
64-
"nextCursor": "<CURSOR>",
65-
"prevCursor": "<CURSOR>"
64+
"nextCursor": null,
65+
"prevCursor": null
6666
},
6767
"status": [
6868
{

‎transcripts/share-apis/branches/branch-list-core-only.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,8 @@
4242
"updatedAt": "<TIMESTAMP>"
4343
}
4444
],
45-
"nextCursor": "<CURSOR>",
46-
"prevCursor": "<CURSOR>"
45+
"nextCursor": null,
46+
"prevCursor": null
4747
},
4848
"status": [
4949
{

‎transcripts/share-apis/branches/branch-list-name-filter.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@
2222
"updatedAt": "<TIMESTAMP>"
2323
}
2424
],
25-
"nextCursor": "<CURSOR>",
26-
"prevCursor": "<CURSOR>"
25+
"nextCursor": null,
26+
"prevCursor": null
2727
},
2828
"status": [
2929
{

0 commit comments

Comments
 (0)