Skip to content

Commit 2fa2751

Browse files
ChrisPennerChrisPenner
authored
Merge pull request #95 from unisoncomputing/cp/mask-paging-cursors
2 parents b222bc7 + 6eb5829 commit 2fa2751

File tree

9 files changed

+134
-35
lines changed

9 files changed

+134
-35
lines changed

‎src/Share/Notifications/Impl.hs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import Share.Notifications.Types
1212
import Share.Postgres qualified as PG
1313
import Share.Postgres.Ops qualified as UserQ
1414
import Share.User (User (..))
15-
import Share.Utils.API (Cursor, Paged, pagedOn)
15+
import Share.Utils.API (Cursor, Paged)
1616
import Share.Web.App
1717
import Share.Web.Authorization qualified as AuthZ
1818
import Share.Web.Share.DisplayInfo.Types (UnifiedDisplayInfo)
@@ -93,9 +93,7 @@ getHubEntriesEndpoint userHandle callerUserId limit mayCursor mayStatusFilter =
9393
notifications <- PG.runTransaction $ do
9494
notifs <- NotificationQ.listNotificationHubEntryPayloads notificationUserId limit mayCursor (API.getStatusFilter <$> mayStatusFilter)
9595
forOf (traversed . traversed) notifs NotifOps.hydrateEvent
96-
notifications
97-
& pagedOn (\(NotificationHubEntry {hubEntryId, hubEntryCreatedAt}) -> (hubEntryCreatedAt, hubEntryId))
98-
& pure
96+
pure notifications
9997

10098
updateHubEntriesEndpoint :: UserHandle -> UserId -> API.UpdateHubEntriesRequest -> WebApp ()
10199
updateHubEntriesEndpoint userHandle callerUserId API.UpdateHubEntriesRequest {notificationStatus, notificationIds} = do

‎src/Share/Notifications/Queries.hs

Lines changed: 37 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ import Share.Postgres.Tickets.Queries qualified as TicketQ
4141
import Share.Postgres.Users.Queries qualified as UsersQ
4242
import Share.Prelude
4343
import Share.Ticket
44-
import Share.Utils.API (Cursor (..), CursorDirection (..))
44+
import Share.Utils.API (Cursor (..), CursorDirection (..), Paged (..), guardPaged, pagedOn)
4545
import Share.Web.Share.DisplayInfo.Queries qualified as DisplayInfoQ
4646
import Share.Web.Share.DisplayInfo.Types (UnifiedDisplayInfo)
4747

@@ -62,30 +62,45 @@ expectEvent eventId = do
6262
WHERE id = #{eventId}
6363
|]
6464

65-
listNotificationHubEntryPayloads :: UserId -> Maybe Int -> Maybe (Cursor GetHubEntriesCursor) -> Maybe (NESet NotificationStatus) -> Transaction e [NotificationHubEntry UnifiedDisplayInfo HydratedEventPayload]
65+
listNotificationHubEntryPayloads :: UserId -> Maybe Int -> Maybe (Cursor GetHubEntriesCursor) -> Maybe (NESet NotificationStatus) -> Transaction e (Paged GetHubEntriesCursor (NotificationHubEntry UnifiedDisplayInfo HydratedEventPayload))
6666
listNotificationHubEntryPayloads notificationUserId mayLimit mayCursor statusFilter = do
67-
let limit = clamp (0, 1000) . fromIntegral @Int @Int32 . fromMaybe 50 $ mayLimit
68-
let statusFilterList = Foldable.toList <$> statusFilter
69-
let cursorFilter = case mayCursor of
67+
let inflatedLimit = clamp (0, 1000) . fromIntegral @Int @Int32 . fromMaybe 50 . (fmap (+ 1)) $ mayLimit
68+
let mkCursorFilter = \case
7069
Nothing -> mempty
71-
Just (Cursor (beforeTime, entryId) Previous) -> [PG.sql| AND (hub.created_at, hub.id) < (#{beforeTime}, #{entryId})|]
72-
Just (Cursor (afterTime, entryId) Next) -> [PG.sql| AND (hub.created_at, hub.id) > (#{afterTime}, #{entryId})|]
73-
dbNotifications <-
74-
queryListRows @(NotificationHubEntry UserId NotificationEventData)
75-
[sql|
76-
SELECT hub.id, hub.status, hub.created_at, event.id, event.occurred_at, event.scope_user_id, event.actor_user_id, event.resource_id, event.topic, event.data
77-
FROM notification_hub_entries hub
78-
JOIN notification_events event ON hub.event_id = event.id
79-
WHERE hub.user_id = #{notificationUserId}
80-
AND (#{statusFilterList} IS NULL OR hub.status = ANY(#{statusFilterList}::notification_status[]))
81-
-- By default omit notifications that are from the user themself.
82-
AND event.actor_user_id <> #{notificationUserId}
83-
^{cursorFilter}
84-
ORDER BY hub.created_at DESC
85-
LIMIT #{limit}
86-
|]
70+
Just (Cursor (beforeTime, entryId) Previous) -> [PG.sql| AND (hub.created_at, hub.id) > (#{beforeTime}, #{entryId})|]
71+
Just (Cursor (afterTime, entryId) Next) -> [PG.sql| AND (hub.created_at, hub.id) < (#{afterTime}, #{entryId})|]
72+
dbNotifications <- query inflatedLimit (mkCursorFilter mayCursor)
8773
hydratedPayloads <- PG.pipelined $ forOf (traversed . traversed) dbNotifications hydrateEventPayload
88-
hydratedPayloads & DisplayInfoQ.unifiedDisplayInfoForUserOf (traversed . hubEntryUserInfo_)
74+
results <- hydratedPayloads & DisplayInfoQ.unifiedDisplayInfoForUserOf (traversed . hubEntryUserInfo_)
75+
let hasNextPage = length results == fromIntegral inflatedLimit
76+
let paged@(Paged {prevCursor}) =
77+
results
78+
& (take (fromIntegral inflatedLimit - 1))
79+
& pagedOn (\(NotificationHubEntry {hubEntryId, hubEntryCreatedAt}) -> (hubEntryCreatedAt, hubEntryId))
80+
hasPrevPage <- not . null <$> query 1 (mkCursorFilter prevCursor)
81+
pure $ guardPaged hasPrevPage hasNextPage paged
82+
where
83+
statusFilterList :: Maybe [NotificationStatus]
84+
statusFilterList = Foldable.toList <$> statusFilter
85+
query ::
86+
(QueryA m) =>
87+
Int32 ->
88+
PG.Sql ->
89+
m [NotificationHubEntry UserId NotificationEventData]
90+
query limit cursorFilter =
91+
queryListRows @(NotificationHubEntry UserId NotificationEventData)
92+
[sql|
93+
SELECT hub.id, hub.status, hub.created_at, event.id, event.occurred_at, event.scope_user_id, event.actor_user_id, event.resource_id, event.topic, event.data
94+
FROM notification_hub_entries hub
95+
JOIN notification_events event ON hub.event_id = event.id
96+
WHERE hub.user_id = #{notificationUserId}
97+
AND (#{statusFilterList} IS NULL OR hub.status = ANY(#{statusFilterList}::notification_status[]))
98+
-- By default omit notifications that are from the user themself.
99+
AND event.actor_user_id <> #{notificationUserId}
100+
^{cursorFilter}
101+
ORDER BY hub.created_at DESC
102+
LIMIT #{limit}
103+
|]
89104

90105
hasUnreadNotifications :: UserId -> Transaction e Bool
91106
hasUnreadNotifications notificationUserId = do

‎src/Share/Utils/API.hs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ module Share.Utils.API
1313
emptySetUpdate,
1414
Cursor (..),
1515
Paged (..),
16+
guardPaged,
1617
Query (..),
1718
Limit (..),
1819
(:++) (..),
@@ -229,6 +230,13 @@ instance (ToJSON a, ToJSON cursor) => ToJSON (Paged cursor a) where
229230
"nextCursor" .= nextCursor
230231
]
231232

233+
guardPaged :: Bool -> Bool -> Paged cursor a -> Paged cursor a
234+
guardPaged hasPrev hasNext paged@(Paged {prevCursor, nextCursor}) =
235+
paged
236+
{ prevCursor = if hasPrev then prevCursor else Nothing,
237+
nextCursor = if hasNext then nextCursor else Nothing
238+
}
239+
232240
-- | The maximum page size for pageable endpoints
233241
maxLimit :: Int64
234242
maxLimit = 100

‎transcripts/share-apis/notifications/list-notifications-read-transcripts.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,8 @@
5050
"status": "read"
5151
}
5252
],
53-
"nextCursor": "<CURSOR>",
54-
"prevCursor": "<CURSOR>"
53+
"nextCursor": null,
54+
"prevCursor": null
5555
},
5656
"status": [
5757
{

‎transcripts/share-apis/notifications/list-notifications-test-paging.json

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
{
2+
"body": {
3+
"items": [
4+
{
5+
"createdAt": "<TIMESTAMP>",
6+
"event": {
7+
"actor": {
8+
"info": {
9+
"avatarUrl": "https://www.gravatar.com/avatar/205e460b479e2e5b48aec07710c08d50?f=y&d=retro",
10+
"handle": "transcripts",
11+
"name": "Transcript User",
12+
"userId": "U-<UUID>"
13+
},
14+
"kind": "user"
15+
},
16+
"data": {
17+
"kind": "project:ticket:comment",
18+
"link": "http://<HOST>:1234/@test/publictestproject/tickets/1",
19+
"payload": {
20+
"author": {
21+
"avatarUrl": "https://www.gravatar.com/avatar/205e460b479e2e5b48aec07710c08d50?f=y&d=retro",
22+
"handle": "transcripts",
23+
"name": "Transcript User",
24+
"userId": "U-<UUID>"
25+
},
26+
"commentId": "CMT-<UUID>",
27+
"content": "This is a new comment on the ticket",
28+
"createdAt": "<TIMESTAMP>",
29+
"project": {
30+
"projectId": "P-<UUID>",
31+
"projectOwnerHandle": "test",
32+
"projectOwnerUserId": "U-<UUID>",
33+
"projectShortHand": "@test/publictestproject",
34+
"projectSlug": "publictestproject"
35+
},
36+
"ticket": {
37+
"author": {
38+
"avatarUrl": null,
39+
"handle": "test",
40+
"name": null,
41+
"userId": "U-<UUID>"
42+
},
43+
"createdAt": "<TIMESTAMP>",
44+
"description": "I want the code to solve all my problems but it does not. Please fix.\n\n## Things I need:\n\n* It should tie my *shoes*\n* It should make me _coffee_\n* It should do my taxes\n",
45+
"number": 1,
46+
"status": "open",
47+
"ticketId": "T-<UUID>",
48+
"title": "Bug Report"
49+
}
50+
}
51+
},
52+
"id": "EVENT-<UUID>",
53+
"occurredAt": "<TIMESTAMP>",
54+
"scope": {
55+
"info": {
56+
"avatarUrl": null,
57+
"handle": "test",
58+
"name": null,
59+
"userId": "U-<UUID>"
60+
},
61+
"kind": "user"
62+
}
63+
},
64+
"id": "NOT-<UUID>",
65+
"status": "unread"
66+
}
67+
],
68+
"nextCursor": "<CURSOR>",
69+
"prevCursor": null
70+
},
71+
"status": [
72+
{
73+
"status_code": 200
74+
}
75+
]
76+
}

‎transcripts/share-apis/notifications/list-notifications-test.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -266,8 +266,8 @@
266266
"status": "unread"
267267
}
268268
],
269-
"nextCursor": "<CURSOR>",
270-
"prevCursor": "<CURSOR>"
269+
"nextCursor": null,
270+
"prevCursor": null
271271
},
272272
"status": [
273273
{

‎transcripts/share-apis/notifications/list-notifications-transcripts.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,8 +121,8 @@
121121
"status": "unread"
122122
}
123123
],
124-
"nextCursor": "<CURSOR>",
125-
"prevCursor": "<CURSOR>"
124+
"nextCursor": null,
125+
"prevCursor": null
126126
},
127127
"status": [
128128
{

‎transcripts/share-apis/notifications/list-notifications-unread-test.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -203,8 +203,8 @@
203203
"status": "unread"
204204
}
205205
],
206-
"nextCursor": "<CURSOR>",
207-
"prevCursor": "<CURSOR>"
206+
"nextCursor": null,
207+
"prevCursor": null
208208
},
209209
"status": [
210210
{

‎transcripts/share-apis/notifications/run.zsh

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,8 @@ test_notification_id=$(fetch_data_jq "$test_user" GET list-notifications-test '/
102102
transcripts_notification_id=$(fetch_data_jq "$transcripts_user" GET list-notifications-transcripts '/users/transcripts/notifications/hub' '.items[0].id')
103103

104104
fetch "$test_user" GET list-notifications-test '/users/test/notifications/hub'
105+
# If we pass a limit, we should get a next cursor
106+
fetch "$test_user" GET list-notifications-test-paging '/users/test/notifications/hub?limit=1'
105107

106108
fetch "$transcripts_user" GET list-notifications-transcripts '/users/transcripts/notifications/hub'
107109

@@ -120,7 +122,7 @@ fetch "$transcripts_user" PATCH mark-notifications-read-transcripts '/users/tran
120122
]
121123
}"
122124

123-
# Show only unread notifications (none should be unread):
125+
# Show only unread notifications:
124126
fetch "$test_user" GET list-notifications-unread-test '/users/test/notifications/hub?status=unread'
125127

126128
# Show only read notifications (the one we just marked as read):

0 commit comments

Comments
 (0)