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

Possible to set meta robots noindex/nofollow? #123

Closed
ianleeder opened this issue Oct 6, 2022 · 13 comments
Closed

Possible to set meta robots noindex/nofollow? #123

ianleeder opened this issue Oct 6, 2022 · 13 comments

Comments

@ianleeder
Copy link
Contributor

Very excited to find this package, it covers 99% of my needs! I was mildly surprised that it doesn't seem to allow setting a meta tag for robots, eg:

<meta name="robots" content="noindex">

https://developers.google.com/search/docs/crawling-indexing/robots-meta-tag

Have I missed something, or do you have another recommended method to accomplish this?
If I was implementing this myself, I was going to add 2 checkboxes to each document type settings. However given all the other SEO settings are now in a new page, I'd like to keep them grouped if possible.

I just found this promising teaser in the wiki:
image
Is it already possible, but just undocumented?

Regardless of your answer, love your work, and I'm excited to use this package.

@patrickdemooij9
Copy link
Owner

Hi @ianleeder

Yeah, robot tags aren't yet possible. I will see what I can do in the coming days, it shouldn't be hard to add this to the metafields package.

And good find with the documentation. I'll also see if I can improve on that, because it is possible to add fields already

@patrickdemooij9
Copy link
Owner

Part of this has been picked up with this PR: #126. That will give you the ability to set the robot tag value on document type level.

I'll also get it working for content level, but that will take a bit longer so I already merged this part. Expect this to be in the new version somewhere next week

@ianleeder
Copy link
Contributor Author

Any update on doing this at the content level @patrickdemooij9?

And similar to #140, is it possible to read these values through code? My customer wants to hide NOINDEX pages from the Sitemap.

For now I'll probably just add two checkboxes (NOINDEX/NOFOLLOW), and access them through the Sitemap notification (#145), but once content-level robot tags are available through the package, it would be nice to consolidate, and remove duplication..

@patrickdemooij9
Copy link
Owner

Has now been added in the latest release 2.3.0 & 3.1.0.
@ianleeder, I'll come back on the reading and writing of this in code tomorrow. I am going to see if I can extend the documentation for that

@ianleeder
Copy link
Contributor Author

Thanks @patrickdemooij9. I had a poke around in the source, and it looks like I need to use IMetaFieldsValueService.GetUserValues(). But I'm getting a NullReferenceException in GetCulture(). The accessor is non-null, but .VariationContext is null.

Am I on the wrong track?

@patrickdemooij9
Copy link
Owner

@ianleeder you are on the right track. Here is some example code of me reading & setting the title on a published event:

public class TestAfterPublishNotification : INotificationHandler<ContentPublishedNotification>
    {
        private readonly IMetaFieldsValueService _valueService;

        public TestAfterPublishNotification(IMetaFieldsValueService valueService)
        {
            _valueService = valueService;
        }

        public void Handle(ContentPublishedNotification notification)
        {
            var nodeId = notification.PublishedEntities.First().Id;

            //To retrieve the values
            var userValues = _valueService.GetUserValues(nodeId);
            userValues[SeoFieldAliasConstants.Title] = "This is after Published";

            //To write the values
            _valueService.AddValues(nodeId, userValues);
        }
    }

Though, hearing that the VariationContext is null is a bit weird to me. I assume you are trying to do this outside Umbraco context?
You should be able to inject a IVariationContextAccessor in your constructor and then set the VariationContext yourself. Here is the above example with that included:

public class TestAfterPublishNotification : INotificationHandler<ContentPublishedNotification>
    {
        private readonly IMetaFieldsValueService _valueService;
        private readonly IVariationContextAccessor _variationContextAccessor;

        public TestAfterPublishNotification(IMetaFieldsValueService valueService, IVariationContextAccessor variationContextAccessor)
        {
            _valueService = valueService;
            _variationContextAccessor = variationContextAccessor;
        }

        public void Handle(ContentPublishedNotification notification)
        {
            _variationContextAccessor.VariationContext = new VariationContext("en-GB");
            var nodeId = notification.PublishedEntities.First().Id;

            //To retrieve the values
            var userValues = _valueService.GetUserValues(nodeId);
            userValues[SeoFieldAliasConstants.Title] = "This is after Published";

            //To write the values
            _valueService.AddValues(nodeId, userValues);
        }
    }

I also think that the service should have a method where you can pass the culture, so I made an issue for that: #162. Though I think you should be able to get it working with the VariationContext. Let me know if you still have any issues with this!

@ianleeder
Copy link
Contributor Author

Yeah, that's pretty much what I'm doing. The customer wants to hide pages from the sitemap if the robots meta value is set to NOINDEX.

public class HideFromSiteMapNotificationHandler : INotificationHandler<GenerateSitemapNodeNotification>
{
    private IMetaFieldsValueService _metaFieldsValueService;

    public HideFromSiteMapNotificationHandler(IMetaFieldsValueService metaFieldsValueService)
    {
        _metaFieldsValueService = metaFieldsValueService;
    }

    public void Handle(GenerateSitemapNodeNotification notification)
    {
        notification.Node.HideFromSitemap |= notification.Node.Content.Value<bool>("hideFromSitemap");

        if(!notification.Node.HideFromSitemap)
        {
            Dictionary<string, object> values = _metaFieldsValueService.GetUserValues(notification.Node.Content.Id);
            // Check Robots NOINDEX value
        }
    }
}

It's a valid ID, I've also tried hard-coding one. I had to create a debug build and step into the code to find out why it's throwing NullReferenceException.

I don't need my own IVariationContextAccessor. The problem was in GetUserValues which calls GetCulture, and in there VariationContext is null. If it's not something I'm doing wrong, want me to create a new issue?

@patrickdemooij9
Copy link
Owner

@ianleeder, I think I see what is going on then. Am I right in assuming that you have a website with just one language?

@ianleeder
Copy link
Contributor Author

ianleeder commented Feb 9, 2023 via email

@ianleeder
Copy link
Contributor Author

Ok, so I have two languages installed:
image

However I get the same issue if I delete US and leave just AU language:
image

And if I restore US and set it as the fallback for AU:
image

Even setting it to US only still gives the same NullReferenceException:
image

It doesn't seem to be related to the installed languages.

@ianleeder
Copy link
Contributor Author

ianleeder commented Feb 28, 2023

So I was having a play with this today @patrickdemooij9 . The top variable (_valueService) is inside the ContentPublishedNotification handler (the example you gave above). The bottom variable (_metaFieldsValueService) is inside the GenerateSitemapNodeNotification handler (the code I posted above).

image

The VariationContext is not available for a SiteMap request, but it is on a publish event? Same null value when I try to inject the IVariationContextAccessor manually.

It looks like #162 is required to bypass this limitation.

@dederuhi
Copy link

dederuhi commented Nov 4, 2024

@ianleeder have you find a solution for hiding contents with NOINDEX selected?

@ianleeder
Copy link
Contributor Author

ianleeder commented Nov 5, 2024

@dederuhi Patrick's solution above works after the fix in #166. For reference here is my complete class:

using SeoToolkit.Umbraco.MetaFields.Core.Interfaces.Services;
using SeoToolkit.Umbraco.Sitemap.Core.Notifications;
using Umbraco.Cms.Core.Composing;
using Umbraco.Cms.Core.DependencyInjection;
using Umbraco.Cms.Core.Events;
using Umbraco.Extensions;

namespace MyProject.Notifications;

// https://seotoolkit.gitbook.io/useotoolkit/extensions/notifications#generatesitemapnodenotification

public class HideFromSiteMapComposer : IComposer
{
    public void Compose(IUmbracoBuilder builder)
    {
        builder.AddNotificationHandler<GenerateSitemapNodeNotification, HideFromSiteMapNotificationHandler>();
    }
}

public class HideFromSiteMapNotificationHandler : INotificationHandler<GenerateSitemapNodeNotification>
{
    private readonly IMetaFieldsValueService _metaFieldsValueService;

    public HideFromSiteMapNotificationHandler(IMetaFieldsValueService metaFieldsValueService)
    {
        _metaFieldsValueService = metaFieldsValueService;
    }

    public void Handle(GenerateSitemapNodeNotification notification)
    {
        // Get settings for this content node
        Dictionary<string, object> values = _metaFieldsValueService.GetUserValues(notification.Node.Content.Id);
        // If settings include robots noindex, hide from sitemap
        if (values.TryGetValue("robots", out object? robotsObject) && robotsObject is string robotsString)
        {
            notification.Node.HideFromSitemap |= robotsString.Split(',').Contains("noindex");
        }
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants