Add graphql types to the Seo module #11213
Conversation
|
|
|
@sebastienros Dont who to ping to look at this but I think it's somewhat in a reviewable state now |
|
@dannyd89 is your PR ready or still WIP? |
|
@hishamco I removed the WIP label, this is ready for PR |
src/OrchardCore.Modules/OrchardCore.Seo/Indexes/SeoMetaPartIndex.cs
Outdated
Show resolved
Hide resolved
|
|
||
| namespace OrchardCore.Seo.GraphQL; | ||
|
|
||
| public class SeoMetaPartIndexAliasProvider : IIndexAliasProvider |
There was a problem hiding this comment.
Is it used in your custom code to retrieve the Indices names available to Query?
There was a problem hiding this comment.
Should I remove this? I just saw it being used in AutoPart for indexing thats why I created one for SeoMeta too
There was a problem hiding this comment.
You don't need to index these. There is no reason to query a content item by these properties IMO.
|
|
||
| namespace OrchardCore.Seo.Indexes; | ||
|
|
||
| public class SeoMetaPartContentIndexHandler : IContentItemIndexHandler |
There was a problem hiding this comment.
Ok, I don't really see when I will require to search in Lucene a Seo meta title. Though, can be useful if needed.
There was a problem hiding this comment.
I think it's fine to add it in the indices, might help for the full text search.
I'm not sure about that one, need to ask @carlwoodhouse but that would definitely make sense. |
This one is already fixed with the current commit, I'll work on your feedback as soon as I have some time on my hand, thanks |
agriffard
changed the title
|
|
||
| namespace OrchardCore.Seo.GraphQL; | ||
|
|
||
| public class SeoMetaPartIndexAliasProvider : IIndexAliasProvider |
There was a problem hiding this comment.
You don't need to index these. There is no reason to query a content item by these properties IMO.
src/OrchardCore.Modules/OrchardCore.Seo/GraphQL/SeoMetaInputObjectType.cs
Outdated
Show resolved
Hide resolved
src/OrchardCore.Modules/OrchardCore.Seo/GraphQL/SeoMetaQueryObjectType.cs
Show resolved
Hide resolved
|
|
||
| namespace OrchardCore.Seo.Indexes; | ||
|
|
||
| public class SeoMetaPartContentIndexHandler : IContentItemIndexHandler |
There was a problem hiding this comment.
I think it's fine to add it in the indices, might help for the full text search.
src/OrchardCore.Modules/OrchardCore.Seo/Indexes/SeoMetaPartIndex.cs
Outdated
Show resolved
Hide resolved
|
@sebastienros I addressed your feedback and removed all the indexing etc |
|
I recreated the internal SEO module with my code as an own module and we are currently using it successfully. Is there still something I can do to close this? Else we will stick with our own module. |
|
Waiting on release 1.4 before anything. |
Thanks for the reply, I'll wait and continue to use my module then for the time being |
|
Hi is there any progress on this issue? I would be very nice to have these fields in graphql? Maybe I can help to resolve the merge conflicts if you like to. |
|
I was still waiting on approvement for this, I would be happy if this PR could resolved/merged at some point |
Done!! |
|
GeoPointField also have same problems |
|
Please resolve the conflict |
dannyd89
force-pushed
the
dannyd89/11199-seo-module-add-graphql-types
branch
from
6ce580d
to
11b9cc7
Compare
|
Closed?!! |
|
Sorry for the confusion, should be good now |
This PR fixes #11199
Added ObjectGraphTypes for SeoMetaPart and needed to add one for MetaEntry, too.
I'm not sure if OrchardCore.Seo is the right place for the ObjectGraphType for MetaEntry though, so I would like some feedback on that.
Rest is working fine:
