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

WIP: Call Doctrine QueryExtension on Subresource request #3046

Closed

Conversation

ottaviano
Copy link

Q A
Bug fix? no
New feature? yes
BC breaks? yes (I think)
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT
Doc PR

Hi,
What do you think about calling the ItemExtensions for the parent resource query on Subrequest operation?

It can be useful if the parent resource has an ItemExtension like:

class EnabledExtension implements QueryItemExtensionInterface, QueryCollectionExtensionInterface
{
    public function applyToItem(...)
    {
        if (is_subclass_of($resourceClass, EnabledInterface::class)) {
            $queryBuilder->andWhere(
                sprintf('%s.isEnabled = 1', $queryBuilder->getRootAliases()[0])
            );
        }
    }
}

otherwise, I find no way to change the parent subquery...

@teohhanhui
Copy link
Contributor

Can you share a use case so that it's easier to understand this?

@ottaviano
Copy link
Author

yes, for example, I have two entities: Article and Comment

/**
 * @ApiResource
 */
class Article 
{
  /**
   * @ORM\Column(type="boolean")
   */
  private $published;

  /**
   * @ApiSubresource
   * 
   * @ORM\OneToMany(...)
   */
  private $comments;
}

I add a QueryExtension which modifies Article query for adding article.published = true condition.

class ArticlePublishedExtension implements QueryItemExtensionInterface, QueryCollectionExtensionInterface
{
    public function applyToItem(...)
    {
        if (Article::class === $resourceClass) {
            $queryBuilder->andWhere(
                sprintf('%s.published = true', $queryBuilder->getRootAliases()[0])
            );
        }
    }
}

The queryEtension is well called for these routes:

/articles
/articles/{id}

but not for Comment subresource query /articles/{id}/comments.

SubresourceDataProvider will generate this kind of query:

SELECT 
 comment.*
FROM comment
WHERE 
  comment.article_id IN (
      SELECT id FROM articles WHERE id = :article_id
  )

I suggest calling the queryExtension for Subresource queries too, so the generated query will be:

SELECT 
 comment.*
FROM comment
WHERE 
  comment.article_id IN (
      SELECT id FROM articles WHERE id = :article_id AND published = true
  )

@soyuka
Copy link
Member

soyuka commented Sep 13, 2019

I think that this kinda makes sense but it may break too many current implementations because we will run the actual item extensions... Subresources definitely need some refactor to ease everything 😿

Base automatically changed from master to main January 23, 2021 21:59
@stale
Copy link

stale bot commented Nov 5, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Nov 5, 2022
@stale
Copy link

stale bot commented Jan 4, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jan 4, 2023
@stale stale bot closed this Jan 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants