Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do paged conversation lists; change the pager to be generic over types #105

Merged
merged 1 commit into from Oct 4, 2022

Conversation

lf-
Copy link
Contributor

@lf- lf- commented Oct 4, 2022

I need to be able to do paged conversation lists for slacklinker, since I expect I will have a lot of conversations to go through.

This meant that the existing pager stuff that was hardcoded to one response type needed to fixed. So I fixed it by deleting a bunch of duplicated code by making it generic.

Comment on lines +19 to +25
class PagedRequest a where
setCursor :: Maybe Cursor -> a -> a

class PagedResponse a where
type ResponseObject a
getResponseMetadata :: a -> Maybe ResponseMetadata
getResponseData :: a -> [ResponseObject a]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I designed it this way since there is no binding between request and response types; two requests may reuse the same response.

Comment on lines -22 to -53
import Web.Slack.Types (Cursor)
import Prelude

-- | Public only for testing.
conversationsHistoryAllBy ::
MonadIO m =>
-- | Response generator
(Conversation.HistoryReq -> m (Response Conversation.HistoryRsp)) ->
-- | The first request to send. _NOTE_: 'Conversation.historyReqCursor' is silently ignored.
Conversation.HistoryReq ->
-- | An action which returns a new page of messages every time called.
-- If there are no pages anymore, it returns an empty list.
m (LoadPage m Common.Message)
conversationsHistoryAllBy sendRequest initialRequest =
genericFetchAllBy
sendRequest
(\cursor -> initialRequest {Conversation.historyReqCursor = cursor})

-- | Public only for testing.
repliesFetchAllBy ::
MonadIO m =>
-- | Response generator
(Conversation.RepliesReq -> m (Response Conversation.HistoryRsp)) ->
-- | The first request to send. _NOTE_: 'Conversation.historyReqCursor' is silently ignored.
Conversation.RepliesReq ->
-- | An action which returns a new page of messages every time called.
-- If there are no pages anymore, it returns an empty list.
m (LoadPage m Common.Message)
repliesFetchAllBy sendRequest initialRequest =
genericFetchAllBy
sendRequest
(\cursor -> initialRequest {Conversation.repliesReqCursor = cursor})
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sicko_yes.jpg

Copy link

@dsh dsh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One non-blocking suggestion that I'm not even sure is a good idea.


instance NFData ResponseMetadata

$(deriveJSON (jsonOpts "responseMetadata") ''ResponseMetadata)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is simple enough write the instances without the Template Haskell overhead?

Or this is simple enough the Template Haskell overhead doesn't matter?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main motivation is that it was moved. I would rather take the TH overhead than do it inconsistently, I guess.

@lf- lf- merged commit 21741b9 into master Oct 4, 2022
@lf- lf- deleted the jadel/pager-improvements branch October 4, 2022 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants