Fix GetAsync not work when option is AllVersion

[hyzx86]

[sebastienros]

Can you describe what "doesn't work" so we can recommend the best approach? Maybe open an issue?

[Skrypt]

Retrieving AllVersions all at once with the ContentManager.

[jtkech]

Or as I understood, return any active version, maybe in priority the published version if it exists, or a draft version if it exists, that' why here the suggested code uses contentItemId && (x.Latest || x.Published).

But currently it then uses FirstOrDefaultAsync(), so it may return a draft whereas a published version exists.

Maybe it should use ListAsync() allowing to return a published version in priority.

[hyzx86]

Yes, it is the same problem as this one, https://github.com/OrchardCMS/OrchardCore/pull/11447

Because my program uses SPA mode, much of the data manipulation is done through the Api Initially I thought the draft state would be saved as the new ContentItemId, It turns out there may be something wrong with the data I'm transferring, so the new ContentItemId is generated because the POST API/content doesn't seem to have any validation. It might need to exclude two status fields (latest, published)(I can't remember), For simplicity, I've implemented an API similar to the Graphql mutation myself It can directly convert Graphql query results to Contentitem and then update the content

https://github.com/EasyOC/EasyOC/blob/eb1dc57f37491296823b633c30dc30806613e573/src/Modules/EasyOC.OrchardCore.ContentExtentions/AppServices/ContentManagementAppService.cs#L67

[hyzx86]

Can you describe what "doesn't work" so we can recommend the best approach? Maybe open an issue?

mean is this filter option is never used in this method, But now it seems that just changing this doesn't make much sense, because if no filtering conditions are attached, a ContentItem will be obtained randomly (or with a minimum ID), so at least one ContentVersionId needs to be specified here

[hyzx86]

Since this option doesn't serve any purpose, can we remove it? It only has one reference in the Graphql status filter This option does not return any data https://github.com/OrchardCMS/OrchardCore/blob/5d3cdca6cd2e04a404f130ca38a404a2699f37dc/src/OrchardCore/OrchardCore.ContentManagement.Abstractions/IContentManager.cs#L265

[jtkech]

Maybe we could have used AllVersions for the GetAsync() that returns a collection, currently we use a bool latest = false parameter, false meaning return all published, true all latest regardless there are published or not. So yes here we could have used AlVersions but we would need a new method to not break the existing one.

For the GetAsync that returns only one item, hmm maybe we could have used an AnyVersion with the behavior I described here https://github.com/OrchardCMS/OrchardCore/pull/11433#issuecomment-1082390420, but was just an idea, maybe too confusing ;)

So yes, if we keep the existing code I agree with you, maybe better to just get rid of the AllVersions option, or as said above and if we think it is worth, maybe use it for a GetAsync that returns a collection of items.

Note: Here we are talking about active versions (Published or Latest), Published and Latest are not versions in themselves but properties of a given version (related to a given contentItemVersionId), a content item having some versions but no active versions is a soft removed item.

[hyzx86]

https://github.com/OrchardCMS/OrchardCore/pull/11447

[hyzx86]

@jtkech This is a very embarrassing question for me, if I want to rewrite GetAsync(id,option) , I need call BuildNewVersionAsync But since it is not public, I have to copy this method to my ContentManager

https://github.com/OrchardCMS/OrchardCore/blob/4093d2e71fc9ef48c96afd609a6564826e17b803/src/OrchardCore/OrchardCore.ContentManagement/DefaultContentManager.cs#L239

But even if I copied it, I found ContentElement.Data is not open either

https://github.com/OrchardCMS/OrchardCore/blob/4093d2e71fc9ef48c96afd609a6564826e17b803/src/OrchardCore/OrchardCore.ContentManagement/DefaultContentManager.cs#L416

Finally, I wrote l index query directly by referring to the code in AdminController If so, what is the meaning of ContentManager

https://github.com/OrchardCMS/OrchardCore/blob/4093d2e71fc9ef48c96afd609a6564826e17b803/src/OrchardCore.Modules/OrchardCore.Contents/Controllers/AdminController.cs#L310

[jtkech]

Finally, I wrote l index query directly by referring to the code in AdminController If so, what is the meaning of ContentManager

It allows to call the contentManager LoadAsync() on each contentItem which call handlers to activate correctly items if not yet done by looking at the contentManagerSession cache.

[hyzx86]

Yes, I see. I mean, the current implementation of content manager is not complete enough

[jtkech]

As additional info, as it is done when using content manager GetAsync(), activating item meaning activating item parts/fields

[github-actions[bot]]

This pull request has merge conflicts. Please resolve those before requesting a review.

[Piedone]

We should rather remove VersionOptions.AllVersions, see comments from https://github.com/OrchardCMS/OrchardCore/issues/16027#issuecomment-2115840661