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

Extended EntryInterface fields #184

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

djfarly
Copy link

@djfarly djfarly commented Oct 31, 2018

Hej, thank you for this plugin! We're using it to make craft truly headless.

We ran into a few problems that we needed to solve:

  • The uri field is "__home__" for the home page which is not particularly useful if you're not craft
  • The uri field does not contain the url prefix from sites (which are used for languages in our case [/en, /de])
  • The url field does contain the language prefix, but also contains the full url including "https://something.bla/", which again is not particularly helpful since the frontend is not necessarily hosted where the cms is
  • There is no way (coming from an Entry) to figure out to which site it belongs
  • There is no way (coming from an Entry) to figure out what the entries with the same id's for other sites are. (Example: "I'm the English version of an Entry, what are the Entries in other Languages related to me.")

To solve this I've added some fields:

  • fullUri contains a fully qualified url no matter what.
  • site is just the site the entry belongs to
  • supportedSites is the array of supportedSites (copied from craft)
  • alternateEntries is an Array of a new Type called EntryInterfaceAlternate that just contains an Entry and a boolean isSelf. The entries are the ones with the same id in all sites where the entry is active. Think of this as enabling you to simply make a site-(/language-)switcher based on that data.
  • language is just a convenience field that pulls the language from the site. (Yea... kinda unnecessary — I'm actually indifferent about this one)

This code is more or less a proof of concept and I'm almost certain that my implementation is not very nice and might have some quirks. Nevertheless I'd love to make CraftQL more helpful for us.

Do you think those (or some of those) additions make sense?

How can I improve them?

If some additions do not make sense... is there a way to extend the Schema like this without needing to fork CraftQL?

Let's talk :)

Screenshot of how it looks currently:
Entry Additions

@markhuot
Copy link
Owner

Loves,

  • I love the addition of site on the EntryInterface
  • supportedSites makes sense to me but I'm not sure why you need the custom resolver, won't the native code do the same thing?
  • Grabbing alternate entries is a neat idea. Does Craft support this natively through the .twig interfaces? If so then it's definitely worth bring in here. If it's not supported by .twig then this may be better suited for the EntryConnection type.

Some critiques,

  • language seems redundant if you can get the same info out of EntryInterface.site.language I'm also worried about language conflicting with a custom field a user created.

A few more questions about the URLs,

  • I understand __home__ not playing well outside of Craft, that's probably worth adjusting
  • If you're in a .twig template and you call {{ entry.url }} does it suffer from the same issues you describe above? If so I'd rather push this upstream and deal with it in Craft, itself?

Regarding extending the schema, this is sort of possible today but I've been quiet about it because I'd rather get these features fixed in CraftQL core and not require uses to patch them in, themselves. Basically, this should all be resolved in core since it's a core bug so thanks for raising it!

@djfarly
Copy link
Author

djfarly commented Oct 31, 2018

Loves,

  • I love the addition of site on the EntryInterface

thanks ❤️

  • supportedSites makes sense to me but I'm not sure why you need the custom resolver, won't the native code do the same thing?

I thought the same but for some reason $root['supportedSites'] contains an array containing assoc arrays of siteId and enabledByDefault:

Array
(
        [0] => Array
        (
                [siteId] => 1
            [enabledByDefault] => 1
        )

    [1] => Array
        (
                [siteId] => 2
            [enabledByDefault] => 1
        )

)

That seems to trip up the automatic resolver. I don't have enough experience to know why. :)

  • Grabbing alternate entries is a neat idea. Does Craft support this natively through the .twig interfaces? If so then it's definitely worth bring in here. If it's not supported by .twig then this may be better suited for the EntryConnection type.

I'm not aware of Craft exposing that in the twig api directly. How would you solve this with the EntryConnection type? I just added the Wrapper type because i wanted to carry the isSelf info somewhere.

Some critiques,

  • language seems redundant if you can get the same info out of EntryInterface.site.language I'm also worried about language conflicting with a custom field a user created.

Nevermind, let's drop it 😄

A few more questions about the URLs,

  • I understand __home__ not playing well outside of Craft, that's probably worth adjusting
  • If you're in a .twig template and you call {{ entry.url }} does it suffer from the same issues you describe above? If so I'd rather push this upstream and deal with it in Craft, itself?

Mhh… if you deal with url in twig/craft itself then if your Craft runs on https://foobar.org/admin then url = 'https://foobar.org/my-cool-page' is correct as well because the hostname is the same.

That's not a given if you use craft as a headless solution. You could have https://foobar.org (running craft + craftql) and https://example.com (running some kind of app consuming the api). Still your url field contains "https://foobar.org"... which you're not interested in.

To achieve the desired result it might be better to remove the hostname from url field instead of rebuilding it from scratch with some replace magic.

Regarding extending the schema, this is sort of possible today but I've been quiet about it because I'd rather get these features fixed in CraftQL core and not require uses to patch them in, themselves. Basically, this should all be resolved in core since it's a core bug so thanks for raising it!

That makes sense. 😁

@markhuot
Copy link
Owner

Thanks @djfarly!

  • I see what's up with the default resolver now and, yea, we'll need the custom resolver there.
  • I'll need to think on the URL bit a little. I really want to try to keep CraftQL 1:1 with craft.entries and if Craft returns the full URL in entry.url then I'd like to keep CraftQL doing the same. However, I get the struggle and it makes sense we'd need more flexibility in a headless approach. One question, does the native uri field not work for you in this case?

@markhuot
Copy link
Owner

Nevermind, I see why .uri isn't working for you. I wonder if we could fix that with directives? Maybe something like this?

{
  entries {
    uri @withLocale @normalize
  }
}

@djfarly
Copy link
Author

djfarly commented Oct 31, 2018

smiles, while deleting the explanation for the uri… 😄

That sounds like a super nice idea. I think it would need to be @withSitePrefix (or something) because you could use sites for something other than localization... (I guess).

What would @normalize do?

@markhuot
Copy link
Owner

markhuot commented Nov 1, 2018

Oh, @normalize could convert __home__ to /. Or maybe if there's other "Craft-specific" things in the URI, @normalize could convert them…

@djfarly
Copy link
Author

djfarly commented Nov 2, 2018

Nice :)

Just trying to figure out what this would require:

  • creating a Directive (src/Directives/Uri.php) based on src/Directives/Date.php
  • creating a Builder (src/Builders/Uri.php) based on src/Builders/Date.php

Do I need to wire it up somewhere else in the code?

@juliansthl
Copy link

juliansthl commented Apr 30, 2019

@markhuot @djfarly is there any progress on this? or any way I can help to push this forward?
The addition of site on the EntryInterface and having a way to access alternate Entries (or even better: a way to query entries across sites) would be super helpful to us.

Edit: CraftCMS 3.2 now allows querying entries across sites: craftcms/cms@d017022

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