Add a link to get only published entities in collections

Actually, thanks to the addition of \Drupal\Core\Entity\EntityPublishedInterface in #2789315: Create EntityPublishedInterface and use for Node and Comment, this is now totally feasible :)

I am working on this issue.

@midas587: that's great news! Also: welcome to drupal.org! 🤝

FWIW, I don't think we should be arbitrarily adding links to every response that are merely suggestions. Perhaps we can find a better way to solve this through documentation?

@e0ipso fair enough. Assigned.

#6: I agree, but in this case, we're adding a suggestion pointing users to what they probably want 99% of the time. I think this issue should perhaps be the sole exception?

Another example we should consider is translations.

… when we figure out how to handle translations. Core hasn't figured this out yet either: #2135829: EntityResource: translations support.

Yep :)

#2844375-38: Document excluded inaccessible entities for DX reasons was marked fixed, because documentation has been written. This would probably be even better, because it makes it more discoverable.

Isn't that what HATEOAS is about? As long as they are relevant I think they belong.

Not necessarily. To me, it's not about helpful links and/or permutations of filters. Or even improving DX.

HATEOAS links should be about communicating state, like "you have access to edit" or "this product is available for purchase." Links should ideally have mechanical relevance. Like a link that can be followed for the next page of results or an edit link suggesting to a frontend that the user has edit permissions and therefore should show an edit button in the UI.

We actually agree more than I'm making it sound like. I think we should add a helpful link. I just feel strongly that this shouldn't be a document level link. Let's put a link to a docs page on the error object.

Heck, let's even make sure there's an #anchor to a very specific suggestion. That would be in line with another open issue about adding error codes.

And, if you insist ;) add a link with the filter on it too!(although I think it would actually be less helpful because it wouldn't have any context). Just do it on the error object :P

Then, in an ideal world, perhaps some Drupal JS client could log the errors in the console in development mode and it could put that link to "more info" in the log message for all errors where we've made this improvement, mechanically :)

The spec even has a built-in errors.links.about for exactly that purpose.

Look ma! I made docs and a link!

Thanks for the argument/explanation in #15, that convinced me :)

The only remaining question then is: how do we determine when to (automatically) add this link to the error object? We don't necessarily know when access is not allowed due to involvement of the status field.

how do we determine when to (automatically) add this link to the error object? We don't necessarily know when access is not allowed due to involvement of the status field.

I personally don't think we should. I think the link to the docs page is enough. We don't need to include a link with a filter status, because it's not mechanically actionable. A human will be able to interpret the docs page just fine.

However, for the sake of being thorough, I imaging we could (1) look for the PublishableInterface and (2) inspect the "reason" string for the permission strings: bypass node access, administer nodes or view own unpublished content

To be clear, I wouldn't be in favor of doing that lol. But I wouldn't filibuster it either.

I think the link to the docs page is enough.

A hardcoded link to the error object, on every error object? It's not clear to me where that link that you're proposing would live.

Ah, my bad. I posted a lot of context which you may not have seen. The error object I'm imagining can be seen in the code example under "So, what's next" in this comment: #2943176-34: Spec Compliance: when error object 'id' key exists, it contains the individual resource URI, instead of "a unique identifier for this particular occurrence of the problem"

Summary: I don't think we should a single error per omitted resource in a collection. Just a single error object when any object has been omitted because of access. The link I'm talking about would be a hardcoded link to the documentation page I made and would live on that single error object under links.about.

If you're concerned about including a hardcoded link to a d.o docs page, perhaps we could link to a line in the jsonapi.api.php using the repository browser.

Oh! Well then let's postpone this on #2943176: Spec Compliance: when error object 'id' key exists, it contains the individual resource URI, instead of "a unique identifier for this particular occurrence of the problem", otherwise this is going to be too confusing. Because we'd be discussing multiple options in parallel, depending on what we do there.

@midas587, do you still want this one assigned to you?

6 days w/o a response, I think it's fair to un-assign.

#2943176: Spec Compliance: when error object 'id' key exists, it contains the individual resource URI, instead of "a unique identifier for this particular occurrence of the problem" was closed, so #21 is no longer true.

As long as they are relevant I think they belong.

HATEOAS links should be about communicating state, like "you have access to edit" or "this product is available for purchase." Links should ideally have mechanical relevance. Like a link that can be followed for the next page of results or an edit link suggesting to a frontend that the user has edit permissions and therefore should show an edit button in the UI.

I think you're both right.

HATEOAS is about state. For individual resources, I think the "mechanical relevance" (i.e. links for state mutations) makes sense.

But what about collections? In case of a collection, filtering it down to a subset seems reasonable too; it doesn't change application state, but it does change the "application state" per se, but it does change the "collection state", in that it's a subset. Is that permissible by HATEOAS?
(Note that http://jsonapi.org/format/ has zero mentions of HATEOAS.)

I think links providing access to different subsets of the collection is a powerful DX feature.

For example: \Drupal\Core\Entity\EntityPublishedInterface implies an entity type is (un)publishable. That allows us to automatically surface both published and unpublished links.

This has gone on long enough (and that's my fault!).

It won't hurt anything and @Wim Leers makes a fair point about getting to a subset of a collection.

I like the idea of the links are available conditionally based on EntityPublishedEntityInterface and toggling between published and unpublished.

Let's get it done and move on :)

Of course, it'd be great if this was a precursor to or at least forward-compatible with a good solution for customizable/extendable HATEAOS links. But let's not that block this.

This still needs lots of clean-up, but shows the principle.

One question, is the published information enforced to live in a field named status?

No, it isn’t, hence”needs lots of clean-up” :) I’ll update this patch to remove the inappropriately hardcoded bits. I just wanted architectural feedback from both of you before spending more time on this!

#2995960: Add a Link and LinkCollection class to support RFC8288 web linking. and #3014704: Expose API to allow links handling for entities from other modules would make this a lot simpler.

This is #29 rebased on top of #2995960-54: Add a Link and LinkCollection class to support RFC8288 web linking..

#2995960: Add a Link and LinkCollection class to support RFC8288 web linking. landed.

That means this patch can move forward.

I wanted to expand this to ?resourceVersion=rel:working-copy per … but then I remembered that collection-level support for that was removed in #2992833-282: Add a version negotiation to revisionable resource types.

#36 implements exactly the scope of what the issue title says.

I think it's fine to hardcode this for now. It addresses a very common need. With literally 9 LoC. That are trivial to retain BC for if needed. Eventually, this would be implemented using #2994193: [META] The Last Link: A Hypermedia Story. But doing this now addresses the current need now.

The only thing that I think really needs further attention is the relation name.

Wow, our tests are impressively tight.

That will make it easy to add explicit tests to verify these links are present when they should be :)

Affected entity types: Term, Node, MenuLinkContent, Media, Comment, BlockContent.

_{d.o-- for automatically testing #35 even though nobody ever asked it to :(}

1. +++ b/src/Controller/EntityResource.php
   @@ -890,6 +891,14 @@ class EntityResource {
   +    $entity_type_class = \Drupal::entityTypeManager()->getDefinition($resource_type->getEntityTypeId())->getClass();
   

   The entity type manager is already injected.

2. +++ b/src/Controller/EntityResource.php
   @@ -890,6 +891,14 @@ class EntityResource {
   +    if (is_subclass_of($entity_type_class, EntityPublishedInterface::class)) {
   

   Should we add a condition that omits these links if a filter is already on the request?

   If filters were already on the request, I think I would expect that the published link would add to or alter any existing filters, not lose them.

3. +++ b/src/Controller/EntityResource.php
   @@ -890,6 +891,14 @@ class EntityResource {
   +        ->withLink('filter-published', new Link(new CacheableMetadata(), $this->linkManager->getRequestLink($request, array_merge($query, ['filter' => ['status' => 1]])), ['filter-published']))
   +        ->withLink('filter-unpublished', new Link(new CacheableMetadata(), $this->linkManager->getRequestLink($request, array_merge($query, ['filter' => ['status' => 0]])), ['filter-unpublished']));
   

   Why filter-* and not just (un)published?

1. Hah, that's new since I first did this. Easy fix :)
2. That sounds exactly like #3023508: In the entry point, mirror undefined (but valid) query parameters on every resource type collection link. Can we consider that out-of-scope here and leave it for that issue?
3. Because no reason. This is exactly why I wrote:
   

   The only thing that I think really needs further attention is the relation name.

   in #37.

   I think we'd want to be careful to either use https://www.iana.org/assignments/link-relations/link-relations.xhtml link relation types or otherwise not conflict with them. Sadly, https://jsonapi.org/format/#document-links doesn't say anything about this yet.

   Looking forward to your thoughts :)

1. 👍

2. I think you misread this. I asked to omit the links, not to mirror the links. Mirroring the links is definitely out of scope. Does that change your response?

3. So, technically, the spec doesn't say that link keys have to be valid relation names. Ofc, it's not a coincidence that they are in the spec. I would be very frustrated if future version of the spec made this a requirement (since it precludes having >1 relation name). However, we can't be sure of that.

I proposed a format to extension relation names in #2994193: [META] The Last Link: A Hypermedia Story and we just saw the first adoption of one in #3027238: Design of the new iteration of the module (3.x):

drupal://jsonapi/extensions/consumer_image_styles/links/relation-types/#derivative

So, following that pattern, this one would be:

drupal://jsonapi/links/relation-types/#published

We can elide the extensions/<module_name> in our own link relations, obviously.

2. Oh so you're saying "suggesting potential filters is especially valuable when no filters are present yet, since that is the exploratory state"? But wouldn't that contradict with the idea behind hypermedia, i.e. that interesting/relevant links are always present for a user agent to follow?
3. Hm… I have concerns about that pattern. Why drupal:// and not drupal:? Why drupal: instead of https:? Why so verbose?

Oh so you're saying "suggesting potential filters is especially valuable when no filters are present yet, since that is the exploratory state"?

I agree with this statement (without agreeing with conclusion that follows it).

But wouldn't that contradict with the idea behind hypermedia, i.e. that interesting/relevant links are always present for a user agent to follow?

I don't see how saying suggesting some links are more valuable at different points in an interaction than others contradicts the idea behind hypermedia. HATEOAS does not require that "links are always present".

HATEOAS suggest that links should drive interaction based on state. Thus, if there are already filters, then the state of the interaction is different than it would be without them. I think it's counterproductive if a client says "I want all the red or blue things and not any yellow things" and we say "here's a link for some green things!" or "here's a link for everything except green things!" as opposed to "here's a link for the what you have AND green things!". The latter acknowledges the state of the interaction, the former does not.

So, what I'm trying to say is that we shouldn't do the mirroring thing in this issue. We should simply do nothing for now. So that we can do the better thing later. If we do the simple implementation right now, then we won't be able to the mirroring one later for fear of breaking BC. That's why we should just omit the links when there is a state that we can't yet handle gracefully.

Why so verbose?

Extension link relation types are supposed to be URIs. I did say: "technically, the spec doesn't say that link keys have to be valid relation names."

Which was me saying, "I agree that this is super verbose for a key name, we could probably get away with just doing published and adding an official link relation later (when there's a mechanism to have >1 link relation per link key in the spec).

Why drupal: instead of https:?

When you mint your own URIs with a custom syntax, you are supposed to use your own prefix.

Why drupal:// and not drupal:?

Double slashes indicate a hierarchical syntax.

Why so verbose?

I wanted to create a URI namespace that any Drupal module could eventually use, without conflict. There are lots of reasons to mint URIs for things that are not link relations. For example, an (unregistered) JSON:API profile. Think: Commerce might have a JSON:API profile that's not generic enough to be registered and the profile URI would be drupal://jsonapi/extensions/commerce/profiles/variations which would make productVariation a required resource object meta member and define special handling for it.

RE: conditional presence of these links. You're absolutely right :) I was merely playing devil's advocate to ensure we though through all aspects — AFAICT we have!

When you mint your own URIs with a custom syntax, you are supposed to use your own prefix.

So core's HAL module got this completely wrong then? (See \Drupal\hal\LinkManager\TypeLinkManager::getTypeUri() for example.)

So core's HAL module got this completely wrong then?

Sort of.

HAL decided to make every link relation type unique to each instance of a website. IOW: http://wimleers.com/rest/type/node/article !== http://sullice.com/rest/type/node/article. That's okay for entity types, but it's not okay if you want to modules to be able to provide features via hypermedia and have client libraries that are able to build on those features.

For example, an HTTP client library should be able to hardcode drupal://jsonapi/extensions/commerce/links/relation-types/#add-to-cart so that a JS library/plugin could be written that works for any Drupal Commerce site, not just wimleers.com.

Because we want modules to be able to define their own relations and we don't want to maintain our own registry of link relations (we'd need to vet, approve and prevent conflicts) we need to define a federated URI scheme so that modules can provide link relations that are globally recognizable without our permission. That's how I came up with this format:
jsonapi/extensions/{module_name}/links/relation-types/#{name_of_relation}

^ This says: "this is a link relation type for JSON:API, provided by {module_name}"

^{🤔Maybe we could drop the first jsonapi segment, it might be superfluous}

So, since this is a new specification for how identifiers are assigned, we shouldn't reuse the http scheme.

#48: Sorry, I read this but forgot to respond. That all sounds logical, thanks. Now that we have the rationale at least documented here on the issue, I'm fine with proceeding in that direction.

What do we want to do here?

Should we just write a simple contrib module that does this, and uses the API provided by https://www.drupal.org/project/jsonapi_hypermedia?

I'm fine the proposal in #50.

Project name: jsonapi_published?

#52: Sure 🤷🏼‍♂️. TBH, if someone needs this kind of a link, can't they just add it themselves in a custom module? I personally wouldn't want to create a full project for such a small bit of functionality.

The reason I bring it up is because people keep running into this time and time again. We've gotten this question so many times.

But … actually … in the past 6 months or so, I don't think we've gotten this question even once, despite the usage of the JSON:API Drupal module growing significantly. So perhaps it's now sufficiently clear, the community has learned this, and the need for this has evaporated?

Since that's my observation today (compared to when this was opened nearly 3 years ago 🙃), I am tempted to mark this Closed (outdated). But only if @gabesullice and @e0ipso agree.

+1

Thanks @gabesullice, one more +1 needed to close this.