Merge pull request #101 from jml/todo-tweaks

Revise TODOs

Author: jml

Diff for: src/GraphQL/Internal/Validation.hs

@@ -494,10 +494,8 @@ validateSelection schema selection =
     AST.SelectionFragmentSpread (AST.FragmentSpread name directives) ->
       SelectionFragmentSpread <$> (UnresolvedFragmentSpread name <$> validateDirectives directives)
     AST.SelectionInlineFragment (AST.InlineFragment typeCond directives ss) ->
-      SelectionInlineFragment <$> (InlineFragment  -- TODO: fix the case statement
-                                    <$> (case typeCond of
-                                           Nothing -> pure Nothing
-                                           Just tC -> Just <$> validateTypeCondition schema tC)
+      SelectionInlineFragment <$> (InlineFragment
+                                    <$> traverse (validateTypeCondition schema) typeCond
                                     <*> validateDirectives directives
                                     <*> childSegments ss)
   where

Diff for: src/GraphQL/Resolver.hs

@@ -198,29 +198,31 @@ instance forall m. (Applicative m) => HasResolver m Bool where
   resolve handler Nothing =  map (ok . toValue) handler
   resolve _ (Just ss) = throwE (SubSelectionOnLeaf ss)
 
--- XXX: jml really doesn't understand this. What happens to the selection set? What if it's a nullable object?
-instance forall m hg. (HasResolver m hg, Functor m, ToValue (Maybe hg)) => HasResolver m (Maybe hg) where
-  type Handler m (Maybe hg) = m (Maybe hg)
-  resolve handler _ =  map (ok . toValue) handler
-
 instance forall m hg. (Monad m, Applicative m, HasResolver m hg) => HasResolver m (API.List hg) where
   type Handler m (API.List hg) = m [Handler m hg]
   resolve handler selectionSet = do
     h <- handler
     let a = traverse (flip (resolve @m @hg) selectionSet) h
     map aggregateResults a
 
-
 instance forall m ksN enum. (Applicative m, API.GraphQLEnum enum) => HasResolver m (API.Enum ksN enum) where
   type Handler m (API.Enum ksN enum) = enum
   resolve handler Nothing = (pure . ok . GValue.ValueEnum . API.enumToValue) handler
   resolve _ (Just ss) = throwE (SubSelectionOnLeaf ss)
 
+-- TODO: This is our handler for `Maybe a`, which is currently used to
+-- implement nullable types. It's *probably* broken, in that it's discarding
+-- the selection set. <https://github.com/jml/graphql-api/issues/102>
+instance forall m hg. (HasResolver m hg, Functor m, ToValue (Maybe hg)) => HasResolver m (Maybe hg) where
+  type Handler m (Maybe hg) = m (Maybe hg)
+  resolve handler _ =  map (ok . toValue) handler
+
 -- TODO: A parametrized `Result` is really not a good way to handle the
 -- "result" for resolveField, but not sure what to use either. Tom liked the
 -- tuple we had before more because it didn't imply any other structure or
 -- meaning. Maybe we can just create a new datatype. jml thinks we should
 -- extract some helpful generic monad, ala `Validator`.
+-- <https://github.com/jml/graphql-api/issues/98>
 type ResolveFieldResult = Result (Maybe GValue.Value)
 
 -- Extract field name from an argument type. TODO: ideally we'd run

Diff for: src/GraphQL/Value.hs

@@ -110,6 +110,7 @@ type Value = Value' ConstScalar
 
 -- TODO: These next two definitions are quite internal. We should move this
 -- module to Internal and then re-export the bits that end-users will use.
+-- <https://github.com/jml/graphql-api/issues/99>
 
 -- | A GraphQL value which might contain some variables. These variables are
 -- not yet associated with

Diff for: src/GraphQL/Value/ToValue.hs

@@ -22,7 +22,7 @@ instance ToValue a => ToValue [a] where
   toValue = toValue . List' . map toValue
 
 -- TODO - tom still thinks that using Maybe for nullable is maybe not
--- the best idea.
+-- the best idea. <https://github.com/jml/graphql-api/issues/100>
 instance ToValue a => ToValue (Maybe a) where
   toValue Nothing = ValueNull
   toValue (Just v) = toValue v

Diff for: tests/EndToEndTests.hs

@@ -312,6 +312,7 @@ tests = testSpec "End-to-end tests" $ do
         -- very simple function to turn a JSON value / object into the
         -- variable map that we desire. Alternatively, we should have APIs
         -- like Aeson does.
+        -- <https://github.com/jml/graphql-api/issues/96>
         let Right varName = makeName "whichCommand"
         let vars = Map.singleton (Variable varName) (toValue Sit)
         response <- executeQuery  @QueryRoot root query Nothing vars