search

thoughtbot / carnival

An unobtrusive, developer-friendly way to add comments
MIT License
499 stars 30 forks source link

Make all actions site-specific #144

Closed pbrisbin closed 9 years ago

pbrisbin commented 9 years ago

Note: aace81a is a big refactor commit to remove all direct uses of the UserComment constructor, replaced by a smart builder and record accessors. With that done, eac57a3 (adding a Site field to it) was a much smaller change. I suggest reviewing those two separately and in isolation.

With this, Carnival fully supports multiple sites run from the same installed instance.

jferris commented 9 years ago

This looks good to me.

We have a lot of handlers which are SiteId -> x, Is there a way to abstract that into a SiteHandler type or something?

pbrisbin commented 9 years ago

We have a lot of handlers which are SiteId -> x, Is there a way to abstract that into a SiteHandler type or something?

Sounds like a case for ReaderT, except that I don't think we can change the type of Handler functions since they're "auto"-invoked by Yesod as part of routing.

pbrisbin commented 9 years ago

Hmm. Thinking about this more, I think you're right. The Handler types wouldn't change, but they'd look something like:

type SiteHandler = ReaderT Handler (Entity Site)

withSite :: SiteHandler a -> SiteId -> Handler a
withSite f siteId = do
    site <- runDB $ get404 siteId

    runReader f (Entity siteId site)

postComment :: SiteId -> Handler Value
postComment = withSite $ do
    -- ...

    site <- ask

    -- ...

    lift $ sendResponse -- ...

EDIT: thought through the code example a bit more.

I'm not sure where this would lead, but it could be worth exploring. I think I'll merge this as-is but add an Issue to investigate that -- is should be done in an isolated PR anyway.