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

Implemented domain content #3

Merged
merged 2 commits into from
Sep 29, 2018
Merged

Implemented domain content #3

merged 2 commits into from
Sep 29, 2018

Conversation

bdunogier
Copy link
Owner

@bdunogier bdunogier commented Oct 13, 2016

Allows query of content using their identifier and fields directly. The content types groups are used as root, with the content types as leafs. It can be accessed at /graphql/domain and /graphiql/domain.

{
  content {
    articles {
      name: title { text }
    }
    folders(query: { Text: "HTML" }) {
      name { text }
    }
  }
  media {
    images {
      name { text }
      image {
        large_image: variations(identifier: "large") { uri }
        small_image: variations(identifier: "large") { uri }
      }
    }
  }
}

It relies on a Custom GraphQL types for ContentTypes of the Repository, such as ArticleContent or BlogPostContent. Those have GraphQL fields for values of their fields values, allowing very quick access to actual content items values: Those types also have a _content field that gives access to the underlying
content item.

The schema for those is generated using a Command script. When executed, it writes the schema to src/AppBundle/Resources/config/graphql.

TODO

  • Think about pluralization of content type identifiers (blog_post => blog_posts)
  • More generally, think about resources that must be exposed. The current resolver does a simple search by ContentType identifier, but we can expose any kind of route. Some configuration could do wonders here (an occasion to re-use QueryTypes maybe).

@janit
Copy link
Contributor

janit commented Oct 14, 2016

Works great for a pile-of-content and takes out the complexity of knowing how complex field types like Image and RichText work internally as they're exposed easily.

Here are some comments:

  • I would expect the query {domain} to resolve to a list of types by default, instead of error
  • Term "domain" is not descriptive and potentially conflicting to example.com, foobar.com...
  • This works for a pile of content, but not if I want to leverage the location tree. Would it be possible to add search options for location or is mixing objects/locations like oil/water?
  • Is it feasible to have the schema update dynamically when adding / modifying Content Types in the future?

@bdunogier
Copy link
Owner Author

bdunogier commented Oct 14, 2016

I would expect the query {domain} to resolve to a list of types by default, instead of error

I'm not sure we can do that. If it is an object, and it is if you have dumped the domain schema, then you need to select something from it.

Term "domain" is not descriptive and potentially conflicting to example.com, foobar.com...

It is a term that has been around in our, well, (engineering) domain. As in Domain Driven Design. Maybe there are better alternatives, but this one is pretty solid imho (ping @andrerom). Naming is hard :)

This works for a pile of content, but not if I want to leverage the location tree. Would it be possible to add search options for location or is mixing objects/locations like oil/water?

We can do plenty of things. About location(s), what would you expect in regards to those ? I was thinking that the UrlAlias was maybe an important property.

Is it feasible to have the schema update dynamically when adding / modifying items in the future?

Complicated, we're gonna have to dig: the yaml schema is compiled in the container.

@janit
Copy link
Contributor

janit commented Oct 14, 2016

I would expect the query {domain} to resolve to a list of types by default, instead of error

I'm not sure we can do that. If it is an object, and it is if you have dumped the domain schema, then you need to select something from it.

Ok, I am just trying to think how this would be as consistent as possible. Actually now comparing to {location}, {content}, {user}... the behaviour is consistent, but {domain} contains the error message, which made me think it works somehow else. So this is fine.

Term "domain" is not descriptive and potentially conflicting to example.com, foobar.com...

It is a term that has been around in our, well, (engineering) domain. Maybe there are better alternatives. Naming is hard :)

Naming is hard, for sure as in this case there's already content and contentType. Maybe concatenate this to contentTypeDomain?

This works for a pile of content, but not if I want to leverage the location tree. Would it be possible to add search options for location or is mixing objects/locations like oil/water?

We can do plenty of things. About location(s), what would you expect in regards to those ? I was thinking that the UrlAlias was maybe an important property.

Ideally I would like to mask the concepts of objectand location from the casual API consumer, who might not be familiar with eZ at all. If they would start with this API and not be limited to content searches, I would like them to have basic access to location search capabilities:

  • Set parent location IDs (default can be 2)
  • Sort by priority

I think this more thought and better specification, but simply not to cause immediate frustration / if you would like to tap into locations in addition to content. Locations are a key feature in eZ Platform that many CaaS services (including Contentful) do not have.

@andrerom
Copy link

andrerom commented Oct 14, 2016

Cool to see protoype of this, get it a bit better now. But shouldn't this align with concepts in API? And if this needs custom mapping, I still think some form of native domain types might be better for this then having to map up things manually here tough (somewhat like what was attempted to be introduced here: https://github.com/ezsystems/ezpublish-kernel/pull/955/files).

@bdunogier
Copy link
Owner Author

bdunogier commented Oct 15, 2016

But shouldn't this align with concepts in API?

Well, I'm not sure. The Repository's API is covered by the initial GraphQL implementation (content, location, etc). I see this approach as one level aboveIt could be a Site API approach, even though for this we need an intermediate layer (like what netgen has done). But being given PHP objects structured after the Content's type is kind of what is done in the User/UserGroup prototype is it not ?

@andrerom
Copy link

But being given PHP objects structured after the Content's type is kind of what is done in the User/UserGroup prototype is it not ?

?

@bdunogier
Copy link
Owner Author

The PR has been rebased to make more sense. See the new syntax in the PR's description.

Ideally I would like to mask the concepts of objectand location from the casual API consumer, who might not be familiar with eZ at all. If they would start with this API and not be limited to content searches, I would like them to have basic access to location search capabilities:

folders will list folders, but can be provided with an extra query. That extra query will accept location based criteria, such as ParentLocationId:

{
  content {
    folders(query: {ParentLocationId: 2}) {
      name { text }
    }
  }
}

Details about the location can be accessed using the _ properties: _location (main), _allLocations (as well as _content for the content item itself).

@bdunogier
Copy link
Owner Author

Note: I intend to merge this very soon. It is getting tedious to maintain it.

1 or 2 reviews would be appreciated.

@bdunogier
Copy link
Owner Author

bdunogier commented Sep 22, 2018

I'm now prototype something about children. To get a tree of all folders, as well as the titles of the articles they contain:

content {
  folders {
    name { text }
    _children(mainLocationOnly: false) {
      ... on ArticleContent {
        title { text }
      }
    }
  }
}

By default, it loads all children from all the content's locations. The idea is to have a mainLocationOnly option, as well as a Subtree option. Other ideas to work with multiple locations are welcome. It could also be made recursive. It will use the search service for filtering anyway.

The _children property will only show up for containers.

@bdunogier
Copy link
Owner Author

Ping @andrerom @janit, and other people you think would have interest in this.

@janit
Copy link
Contributor

janit commented Sep 24, 2018

I'll take a look this week, as I'll also need to try how #17 works :)

@bdunogier
Copy link
Owner Author

bdunogier commented Sep 26, 2018

I'll take a look this week

Thank you, I really appreciate. In any case, I intend to merge it soon, because:

  • it is quite an important addition, and it is getting in the way of further improvements
  • it doesn't, or at least shouldn't, affect the current endpoint (/graphql still refers to the same endpoint)
  • a 0.2.0 has been tagged recently, meaning that we can break master ™

I'd say we are fairly safe.

as I'll also need to try how #17 works :)

It works really well ;)

Some more work is needed to provide support custom field templates arguments. There's also a grey area when it comes to custom templates, as we currently don't have a way to provide custom parameters to html.

Bertrand Dunogier added 2 commits September 29, 2018 12:30
- refactored query input parsing
- added extra criteria
@bdunogier
Copy link
Owner Author

I'm merging this, for the reasons explained in this comment.

@bdunogier bdunogier changed the title Prototyped domain content Implemented domain content Sep 29, 2018
@bdunogier bdunogier merged commit 2836617 into master Sep 29, 2018
@bdunogier bdunogier deleted the domain_content branch October 26, 2018 07:55
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.

3 participants