Replace Enumerable.First() with more defensive logic

[JanLenoch]

In ContactsController, HomeController, Home view, Coffee view, Cafe, and Product display templates, there is the Enumerable.First() being used.

Missing content can break the site.

Replace these calls with a more defensive logic.

(Internal reference: DEL-971)

[Steve-Fenton]

I notice that the listed areas are using FirstOrDefault now - is more required to close this issue down? If so, I'm happy to contribute.

[petrsvihlik]

The issue is still actual but let me rephrase it a little bit. The site should still be functional* if there are no (zero) content items of the following types:

• coffee
• brewer
• accessory
• article
• cafe
• fact about us
• grinder
• hero unit

*functional == the site doesn't throw any exceptions == all exceptions are either handled or prevented by null checks (the content is just not displayed)

[petrsvihlik]

if you want to pick it up @Steve-Fenton just leave a note here, please. thanks!

[Steve-Fenton]

Hi @petrsvihlik, I'll take a look. Any preference on the test project in terms of unit testing frameworks?

[petrsvihlik]

@Steve-Fenton We prefer nunit for .NET Framework and xunit for .NET Core.

If you need to reference any other library (e.g. for mocking) the rule of thumb is to use libraries that target .NET Standard. So e.g. Moq or FakeItEasy will do.

[Steve-Fenton]

Hi @petrsvihlik - I'm hitting a few problems creating mocks for some of the types, as they are sealed and have internal-only constructors, for example DeliveryItemListingResponse<Coffee>. Is there any chance of getting an interface over these types... or are there any suggestions for how I could mock these types?

Thanks,

Steve

[petrsvihlik]

Hi @Steve-Fenton , yes...it should be possible since yesterday :)

Read more here and here.

[Steve-Fenton]

@petrsvihlik brilliant stuff. I'll update the packages and get working on this. Thanks for the information.

[Steve-Fenton]

Hello,

I have been testing this using a web crawler against my local instance, as the amount of changes I would have to make in order to bring the views under test is monumental.

I ran a full crawl, then started un-publishing various content items from my copy of the demo data.

The errors I found from this testing are:

1. AboutUsController Line 21: null Facts -> foreach (var fact in response.Item.Facts)

Fix:

        public async Task<ActionResult> Index()
        {
            var response = await client.GetItemAsync<AboutUs>("about_us");

            var viewModel = new AboutUsViewModel
            {
                FactViewModels = MapFactsAboutUs(response)
            };

            return View(viewModel);
        }

        private IList<FactAboutUsViewModel> MapFactsAboutUs(DeliveryItemResponse<AboutUs> response)
        {
            var facts = new List<FactAboutUsViewModel>();

            if (response.Item == null)
            {
                return facts;
            }

            int i = 0;

            foreach (var fact in response.Item.Facts)
            {
                var factViewModel = new FactAboutUsViewModel
                {
                    Fact = (FactAboutUs)fact
                };

                if (i++ % 2 == 0)
                {
                    factViewModel.Odd = true;
                }

                facts.Add(factViewModel);
            }

            return facts;
        }

2. Contacts/Index.cshtml Line 10: null Roastery -> <li>@Html.DisplayFor(vm => vm.Roastery.Phone)</li>

Fixed by wrapping with:

@if (Model.Roastery != null) {

3. Shared/DisplayTemplates/PhoneNumber.cshtml: null string

Before

string strippedDashes = Model.Replace("-", string.Empty);

After

string strippedDashes = string.IsNullOrWhiteSpace(Model) ? string.Empty : Model.Replace("-", string.Empty);

I am currently dealing with a combination of VS2015/GitHub two-factor auth problems that is preventing this local change going up to my branch, and having to look at a live issue in my day job, so I don't know when the commit is coming for this.

[petrsvihlik]

@Steve-Fenton last thing before we close this issue. Could you please verify the following three scenarios?

• When home page is missing Hero image, it ends up with an unhandled exception
• When home page is missing content variants for personalization, it ends up with unhandled exception
• ContactsControler - If there are no Cafes, throws exception on .First()

They were reported some time ago so I'm not sure if they're still valid.

[Steve-Fenton]

• When home page is missing Hero image, it ends up with an unhandled exception

I have tested using "unpublish" on "Home page hero unit" and there are no errors.

• When home page is missing content variants for personalization, it ends up with unhandled except

I'm not sure which content to remove on this one. Can I have a hint?

• ContactsControler - If there are no Cafes, throws exception on .First()

Again, I unpublished all cafes. There is no error, although the page looks very blank.

[petrsvihlik]

👏 🎉 👌 thanks for your help, @Steve-Fenton !