PersistNat should capture User as a Parameter for the natural transformation.

[andorp]

Instead of passing the current user around for addDB, modifyDB, we could turn persistNat to a function that produces a natural transformation. Please take a look on the following snippet.

data ActionEnv r = ActionEnv
    { _persistNat :: AUID User -> (r :~> IO)
    , _config     :: Config
    }

instance PersistM r => ActionPersist r (Action r) where
    persistent r = do
        user <- currentUser
        view persistNat >>= \nt -> let (Nat rp) = nt user in liftIO $ rp r

What do you think?

[andorp]

I would like to implement this change.

[np]

What impact would this have to the interfaces? I used to be in favor of a change to help pass the current user around but not anymore. For the currentUserAddDb solves this already.So I'd like to understand better what you are aiming to solve.

[andorp]

I've encountered use cases of the current user as a parameter for the persistent layer.

• Adding data in middle of a transaction, lead the persist broken into 3 transactions, in the favor of currentUserAddDb function
• modifyDB should record the user who touched the data

[fisx]

I actually think we should have a PersistCtx that provides more than the user id. We also need a system time ~~and a pure source of passwords~~. This is true at least in #333, but I suspect it also holds for any other working solution, even #328 once it's fixed.

[fisx]

Updated last comment: source of password should be an explicit argument, of course.

[fisx]

@np: could we argue about this some more? you said that understanding currentUserAddDb may help me to like it. Here is a point that @andorp makes: it's not compositional.

Consider two transactions, AddIdea and VoteOnIdea for the former. They can be easily melted into a new transaction AddAndVoteOnIdea, but the type of the new transaction will not match the one needed by currentUserAddDb. So we need a new wrapper.

Have you thought of a solution for this? (We don't need to solve this before #340 has landed, and maybe not this month. But it can't hurt to talk about it now that the experience is fresh.)

[andorp]

An easy workaround for this is to create a similar function to addWithUser which has the following type:

modifiesWithUser :: ... -> (User -> AUpdate a) -> m a

and something similar to

currentUserModifiesDB :: ... (User -> AUpdate a) -> m a

Which is kind of half way between the reader monad and the current solution and it does not involves TemplateHaskell magic, but it has the same functionality.

[np]

I was indeed expecting new wrappers such as the two proposed by @andorp. However one might need at least one more to combine two updates into one. I think that this is the simplest solution.

I'm also seeing a potential monad Reader solution. For instance addIdea has type AddDb Idea which means: EnvWithProto Idea -> AUpdate (), it could have type: Proto Idea -> Reader Env (AUpdate ()). Of course we would give a name to Reader Env (AUpdate ()), the template haskell module would have to use runReader to expose Proto Idea -> Env -> AUpdate () to the acid-state layer. In a way this is to say, I think I do the change I tried yesterday (not the impossible one) but hey, I've failed before on that so...

[andorp]

If you feel so to try that implementation it is good for me. But I would be totally happy with the User -> AUpdate a version, as we have to use only in few places.

[Mikolaj]

I'd be fine with User -> AUpdate a, as well, but that's just my personal preference for KISS (in this case, avoiding premature refactorings), even at the price of more verbose calls.