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

Support Preview flag in ContentByContentTypeQuery #229

Closed
jdpnielsen opened this issue Aug 30, 2023 · 4 comments
Closed

Support Preview flag in ContentByContentTypeQuery #229

jdpnielsen opened this issue Aug 30, 2023 · 4 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@jdpnielsen
Copy link

jdpnielsen commented Aug 30, 2023

Feature summary

Ideally, all 'basic queries' should support preview mode - this would make it much easier to build applications with preview support.

@jdpnielsen jdpnielsen added the enhancement New feature or request label Aug 30, 2023
@nikcio
Copy link
Owner

nikcio commented Aug 30, 2023

Hi @jdpnielsen

The reason ContentByContentTypeQuery doesn't support preview is because the underlying method that is called in Umbraco doesn't take a parameter of preview. So we would either need to have a new method added to Umbraco in order to support this or fetch the content data in some other way.

The underlying call is to IPublishedCache.GetByContentType(IPublishedContentType contentType);. See src\Nikcio.UHeadless.Content\Queries\ContentByContentTypeQuery.cs

@jdpnielsen
Copy link
Author

@nikcio That makes sense.

I just took a look at the underlying code for GetByContentType: https://github.com/umbraco/Umbraco-CMS/blob/v12/dev/src/Umbraco.Core/PublishedCache/PublishedCacheBase.cs#L91-L98 (hope this is the correct one anyway)

    public virtual IEnumerable<IPublishedContent> GetByContentType(IPublishedContentType contentType) =>

        // this is probably not super-efficient, but works
        // some cache implementation may want to override it, though
        GetAtRoot()
            .SelectMany(x => x.DescendantsOrSelf(_variationContextAccessor!))
            .Where(x => x.ContentType.Id == contentType.Id);

It seems the GetAtRoot() function take a preview parameter. Do you think it would make sense to implement a variant of this with preview support in this repo?

@nikcio
Copy link
Owner

nikcio commented Aug 31, 2023

@jdpnielsen I never knew the implementation of GetByContentType was so simple 😆

But yeah we can totally replace GetByContentType with this and pass a preview parameter.
But it would be a breaking change because we will need to inject the variation context accessor to the query and add the preview parameter. But I'm already working on v5 so it can be included in that 😄

Do you want to take a swing at opening a PR for it?

@nikcio nikcio added the good first issue Good for newcomers label Aug 31, 2023
nikcio added a commit that referenced this issue Apr 6, 2024
BREAKING CHANGE:
Adds VariationContextAccessor and preivew as parameters to the ContentByContentType Query

fixes: #229
@nikcio
Copy link
Owner

nikcio commented Apr 12, 2024

This will be included in version 5

@nikcio nikcio closed this as completed Apr 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants