GetByContentType(IPublishedContentType contentType) appears to be broken

craigs100 commented 1 year ago

Which Umbraco version are you using? (Please write the exact version, example: 10.1.0)

10.5.1

Bug summary

Noted Vendr Checkout not sending Confirmation Emails. Looking into it further, the cause appears to be that the Umbraco method IEnumerable GetByContentType(IPublishedContentType contentType) isn't finding valid nodes other than the root. Therefore, in Vendr, not being able to find the Checkout node and therefore it's Ancestor holding the storeId.

Specifics

Umb V10.5.1, Vendr V3.0.12, Vendr Checkout V3.0.0 & Vendr Deploy V3.0.0

References containing full details:

Discord: https://discord.com/channels/869656431308189746/1126807476080222369

Our Forum: https://our.umbraco.com/forum/using-umbraco-and-getting-started/112199-unable-to-send-vendr-order-confirmation-emails

Steps to reproduce

Build Vendr Store including Vendr Checkout with the following schema and versions detailed above:-

Home (with store picker)
    Shop
        Checkout

Set up SMTP transport (SendGrid, MailTrap, etc.)

Attempt to purchase an item and watch the exception occur on line 29 of \App_Plugins\VendrCheckout\views\emails\VendrCheckoutOrderConfirmationEmail.cshtml (detailed in references)

Expected result / actual result

Expected the vendrCheckoutCheckoutPage node to be found and email to be sent.

github-actions[bot] commented 1 year ago

Hi there @craigs100!

Firstly, a big thank you for raising this issue. Every piece of feedback we receive helps us to make Umbraco better.

We really appreciate your patience while we wait for our team to have a look at this but we wanted to let you know that we see this and share with you the plan for what comes next.

We'll assess whether this issue relates to something that has already been fixed in a later version of the release that it has been raised for.
If it's a bug, is it related to a release that we are actively supporting or is it related to a release that's in the end-of-life or security-only phase?
We'll replicate the issue to ensure that the problem is as described.
We'll decide whether the behavior is an issue or if the behavior is intended.

We wish we could work with everyone directly and assess your issue immediately but we're in the fortunate position of having lots of contributions to work with and only a few humans who are able to do it. We are making progress though and in the meantime, we will keep you in the loop and let you know when we have any questions.

Thanks, from your friendly Umbraco GitHub bot :robot: :slightly_smiling_face:

redmorello commented 1 year ago

I am also experiencing this issue (Umbraco 10.6.1), but with other document types.

I've been doing some testing this morning with the method GetByContentType and I'm getting some strange results:

{
  "gaitSessions": 44,
  "gaitSessionsOther": 44,
  "devices": 0,
  "devicesOther": 5,
  "deviceFamilies": 0,
  "deviceFamiliesOther": 9
}

The first result for each document type is using GetByContentType, the second "other" value is from GetByXPath

Some document types are returning no results, when clearly they do exist (as XPath works)

The only difference between the document types are: GaitSessions are single language DeviceFamily is setup for multiple languages Devices are a child of DeviceFamily but setup for single language

Example code:

using var ctx = _umbracoContextFactory.EnsureUmbracoContext();
var content = ctx.UmbracoContext.Content;
var gaitSessionContentType = GaitSession.GetModelContentType(_publishedSnapshotAccessor); // (this just returns a string value of the doc type alias)
var gaitSessions = content?.GetByContentType(gaitSessionContentType);

var gaitSessionsOther = content?.GetByXPath($"//{GaitSession.ModelTypeAlias}");

craigs100 commented 1 year ago

The nodes in my site were all multi-lingual, if it makes a difference.

redmorello commented 1 year ago

this is the offending method in V10:

public virtual IEnumerable<IPublishedContent> GetByContentType(IPublishedContentType contentType)
        {
            // this is probably not super-efficient, but works
            // some cache implementation may want to override it, though
            return GetAtRoot()
                .SelectMany(x => x.DescendantsOrSelf(_variationContextAccessor!))
                .Where(x => x.ContentType.Id == contentType.Id);
        }

I'm guessing it has something to do with language variants then, as the only docs that don't get found are those with multi language or children of multi langauge

craigs100 commented 1 year ago

I don't have any variants. It looks like the same method though.

bergmania commented 1 year ago

I have a pretty hard time reproducing this. Any of you that can make a minimal setup that shows the error?

craigs100 commented 1 year ago

I nearly made a quick demo but the changes to allow Vendr Checkout to be installed on Linux appear to have been reverted in 3.1.0-beta-0007 so it ended there. However, the problem is the same on Windows as well. If you followed my instructions in the original post, you should see it happen when you debug the Order Confirmation email template. You might have to move the templates into your own Views and edit the Vendr Email Template path in the back office though to debug them.

umbraco / Umbraco-CMS