Goals:

• Adding activation for user/tenant creation
• Implement Product API

Done:

• Type class for DB query monad for finer grained separation of effects
• Interface for product queries

How queries on products will work

Servant provides an API to accept QueryParams in the URL

type ProductAPI = "products" :> QueryParams "filter" ProductFilter :> Get '[JSON] [Product]

A handler for this API can be implemented like so:

productHandler :: [ProductFilter] -> App Product
productHandler [] = ..
productHandler xs = let x = mconcat xs in ...

This is possible because ProductFilter, like ProductView and ProductComparator compose and thus form a monoid. Now the user can query like this:

/products?filter=title:sometitle&filter=type:physical

and mconcat will compose all the filters supplied to give you a new filter with which we can write the SQL query.

The mechanism is the same for ProductView and ProductComparator. See ProductQuery.hs for more details on how this is handled.

@wz1000 has the ProductQuery infra been hooked up to any handler? If not, can you please hook it up, so the flow is easier to understand?

Also, if I understand the overall gist of ProductQuery correctly, it is supposed to parse multiple query params, bearing the same name, in the following format:

paramName=keyName:value

into a list of type [Filter DBProduct], where Filter is Persistent's internal data structure to hold SQL where clauses? (In the current case, the resultant data structure is more complex, but this is the basic idea, right?)

I'm not completely able to understand how you're using the Monoid property of ProductFilter to built a list of filters as you parse subsequent query params. Neither of the parseQueryParam functions you have defined are in any sort of Monad/Monoid. To rephrase, who exactly is calling mappend on the ProductFilter?

While I will take a little more time to understand the mechanics of this idea, as usual, you seem to have solved this problem very elegantly. Is there any way to abstract this even further so any such API endpoint is much easier to write (basically, reduce the boilerplate)?

And I assume, this is possible for any DB library. In the case of Opaleye one would parse the URL params to an Opaleye specific type? Btw, I'm unable to figure out what type that would be? https://hackage.haskell.org/package/opaleye-0.5.1.1/docs/Opaleye-Operators.html

@saurabhnanda

I'm not completely able to understand how you're using the Monoid property of ProductFilter to built a list of filters as you parse subsequent query params. Neither of the parseQueryParam functions you have defined are in any sort of Monad/Monoid. To rephrase, who exactly is calling mappend on the ProductFilter?

parseQueryParam only defines how to parse a single ProductFilter/ProductView/ProductComparator.

When we use the QueryParams combinator, Servant automatically gives us a list of ProductFilter/ProductView/ProductComparator. We can then call mconcat(which uses mappend) on this list.

Also, if I understand the overall gist of ProductQuery correctly, it is supposed to parse multiple query params, bearing the same name, in the following format:

paramName=keyName:value

This is the format for ProductFilter. ProductView works a little differently.

The API would be called like this:

 /products?fields=name&fields=description&fields=currency...

Each field=somefield generates a ProductView. A ProductView is essentially a function from a product to a JSON object containing a subset of the products fields. I've defined a new JSON type that is a newtype wrapper of aeson's JSON type, and implemented a monoid instance for it.

What the monoid essentially does is take two JSON objects, and combines them to give a JSON object with the fields of both the objects(if any fields overlap, the first object's fields are kept).

The monoid instance for functions is defined if the result type forms a monoid. Thus, when a function that takes a product and returns a JSON object containing its name is mappended to another function that takes a product and returns a JSON object containing its description, the result is a function that takes a product and returns a JSON object containing both the name and the description.

Each field=somefield generates a ProductView. A ProductView is essentially a function from a product to a JSON object containing a subset of the products fields. I've defined a new JSON type that is a newtype wrapper of aeson's JSON type, and implemented a monoid instance for it. What the monoid essentially does is take two JSON objects, and combines them to give a JSON object with the fields of both the objects(if any fields overlap, the first object's fields are kept).

Wow! I had missed that completely.

We definitely need both of these wired-up to Servant handlers to complete the story!

@wz1000 please confirm if you're wiring this up to Servant handlers.

@saurabhnanda Done

@saurabhnanda As persistent does not support joins, the definition for dbGetProductList I've written is pretty inefficient. I'll fix this by using esqueleto in the future.

A few basic questions:

• Is ProductComparator even getting used?
• Does QueryParams result in the params being parsed to a [x] (i.e. list) or have you done something special to make Servant behave this way? Ref: https://github.com/vacationlabs/haskell-webapps/pull/47/files#diff-14ecfaa0391ebb8c841c4d276243fb7fR30
• Servant is basically parsing the query params individually without calling mconcat on them, right? The only reason we have defined the Monoid instance for ProductFilter is to be able to run mconcat here. Theoretically, this could be replaced by a foldl' as well, right?

At a conceptual level, the pattern/architecture that you seem to be going towards is the following: transform the HTTP request (incoming JSON, query parameters, incoming patch/diff, etc), to functions/data-structures that represent SQL operations as closely as possible.

Are you specifically aiming for this, or things just happen to be lining-up neatly this way?

Is ProductComparator even getting used?

Neither ProductView nor ProductComparator are getting used right now.

ProductComparator doesn't even need to fit into the parse a list and then mconcat model. I just put it in because we get the extra power for free. For example, we can now order by costPrice and then comparisionPrice. If the costPrices are equal mconcat will automatically take care of ordering by comparisionPrice.

Does QueryParams result in the params being parsed to a [x](i.e. list) or have you done something special to make Servant behave this way?

This is the way QueryParams behaves by default.

Theoretically, this could be replaced by a foldl' as well, right?

Yes, but by explicitly stating that it is a Monoid, we get to use a nice interface(mconcat) which behaves in a mathematically consistent way, making it easier to reason about.

At a conceptual level, the pattern/architecture that you seem to be going towards is the following: transform the HTTP request (incoming JSON, query parameters, incoming patch/diff, etc), to functions/data-structures that represent SQL operations as closely as possible.

Only ProductFilter is defined using a persistent specific interface.

Only ProductFilter is defined using a persistent specific interface.

I'm sure if you think hard enough you'll be able to state type-safe updates in terms of a Persistent interface, as well :)

To me, both these approaches have something in common (which is significantly different from the standard Rails way of doing things) but I'm unable to put a finger of what exactly that is.

Neither ProductView nor ProductComparator are getting used right now.

What would it do to the Servant API signatures, if you use ProductView completely?

@saurabhnanda

When I took my first stab at writing ProductFilter, I implemented it using a simple Haskell function

newtype ProductFilter = { getFilter :: DBProduct -> All } deriving (Monoid)

where All(defined in Data.Monoid) is the standard Bool monoid over (&&).

Once I had this, realised to use it I would have to load all the products in the database and then filter over that list. That's when I realised that the persistent [Filter DBProduct] type forms pretty much the same monoidal interface as xs ++ ys where xs and ys are [Filter DBProduct] is persistent for and xs ys

What would it do to the Servant API signatures, if you use ProductView completely?

Not much. The return type would just change from Product to AppJSON.

So, is this sprint complete?

Adding activation for user/tenant creation

Tenant creation code still has some undefined, right? Also, storing the activation key in the DB?

Implement Product API

Product creation as well, or just fetch and filter products?

Yes, but by explicitly stating that it is a Monoid, we get to use a nice interface(mconcat) which behaves in a mathematically consistent way, making it easier to reason about.

Was thinking more about this comment. Can you elaborate how mconcat is better compared to a fold?

Also, do you want to add anything to this PR?

Was thinking more about this comment. Can you elaborate how mconcat is better compared to a fold?

First, on the issue of correctness, there is the property that the product of two monoids is also a monoid. The monoid instances for ProductView and ProductFilter flow directly as a result of this. There is only one canonical way to make a monoid out of the product of two monoids. On the other hand, there are infinite ways to write a foldl/foldr, and so infinitely many ways by which to get it wrong.

Second, monoid composition is associative. That means that (a <> b) <> c is equivalent to a <> (b <> c). Using mconcat indicates that we do not care about the evaluation order. The compiler is free to evaluate result however it likes. It can evaluate mconcat [a,b,c,d] as a <> (b <> (c <> d)) or ((a <> b) <> c) <> d) or even something bizarre like (a <> (b <> c)) <> d

Thanks for the quick primer on Monoids. Any comments on:

Also, do you want to add anything to this PR?

@wz1000 update, please.

@saurabhnanda

As of the commit I just pushed, the domain API is pretty much complete, other than photos and product updates.

Comments:

• Is there any way to run custom SQL statements in-sync with the Persistent migration? For example, if I want to add a custom CHECK CONSTRAINT on a table, how do I do it?
• A user can belong to many roles. The current schema restricts a user to belong to a single role only.
• Why have HasTimestamp instances not been defined for all relevant tables?
• Shouldn't dbUpdateTenant return the updated tenant record back? Shouldn't activateTenant return the updated tenant record back? Is there an efficient version of INSERT... RETURNING or UPDATE...RETURNING in Persistent?
  • Shouldn't all DB APIs inserting/updating DB rows, return the updated/inserted rows?
• Any way to write a wrapper on insert and update to make them take care of createdAt and updatedAt fields automagically? (I see you already have applyUpdate, but that's not being used everywhere)
• I removed the requirePermission (EditUser uid) expression from dbUpdateUsers, and the code compiled! Is there a way to make the code compilation fails if someone forgets to put a call to requirePermission?
• Is it possible to model the Product in such a way that we don't need the following runtime checks? Basically make illegal states unrepresentable?

     case piType of
       Phys -> when (any (\VariantI{..} -> isNothing viWeightInGrams
                                        || isNothing viWeightDisplayUnit)
                         piVariants) $
                 throwError PhysicalProductFieldsMissing
       Dig -> when (any (\VariantI{..} -> isJust viWeightInGrams
                                       || isJust viWeightDisplayUnit)
                         piVariants) $
                 throwError DigitalProductExtraFields

(contd)

I'm missing something in the updation infrastructure again. Why is the following erroring out? What exactly does one need to pass to an Updater '[x]?

:t (dbUpdateUser (toSqlKey 10) (\u -> u & userFirstName .~ "Saurabh")) 

<interactive>:1:30: error:
    • Couldn't match expected type ‘Updater
                                      '[Types.HasHumanName, Types.HasContactDetails]’
                  with actual type ‘UserBase userType00 -> UserBase userType00’
    • The lambda expression ‘\ u -> u & userFirstName .~ "Saurabh"’
      has one argument,
      but its type ‘UserUpdater’ has none
      In the second argument of ‘dbUpdateUser’, namely
        ‘(\ u -> u & userFirstName .~ "Saurabh")’
      In the expression:
        (dbUpdateUser
           (toSqlKey 10) (\ u -> u & userFirstName .~ "Saurabh"))

Also, just thinking aloud, if we have the Updater x infra working properly, do we really need the *Input pattern (eg. UserInput, TenantInput, etc)? Can't we apply the same principles and get a Creator x infra in place?

@saurabhnanda

I'm missing something in the updation infrastructure again. Why is the following erroring out? What exactly does one need to pass to an Updater '[x]?

You forgot to wrap the function in a U constructor. Also, userFirstNames type is too monomorphic. Instead, firstName should be used.

:t (dbUpdateUser (toSqlKey 10) (U $ \u -> u & firstName .~ "Saurabh")) 

Also, the lambda is pretty redundant in this case. You can simply write

:t dbUpdateUser (toSqlKey 10) (U $ firstName .~ "Saurabh") 

@saurabhnanda

I removed the requirePermission (EditUser uid) expression from dbUpdateUsers, and the code compiled! Is there a way to make the code compilation fails if someone forgets to put a call to requirePermission?

Not really, since OperationT m is a monad. All monads have return defined. Consider return "something" :: OperationT IO String. The only "reasonable" definition of return creates an OperationT m which requires no permissions.

Since the compiler has no way to check if the permissions you've required for any given operation are actually the permissions that operation requires, this shouldn't be that big of a tradeoff.

Why have HasTimestamp instances not been defined for all relevant tables?

HasTimestamp is only really required for updates, and I haven't gotten around to writing update operations for all the entities. Once I get to that, the compiler will force me to define the relevant instances.