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

Fixes NullReferenceException with recursive GetVortoValue #100

Merged
merged 2 commits into from
Aug 17, 2018
Merged

Fixes NullReferenceException with recursive GetVortoValue #100

merged 2 commits into from
Aug 17, 2018

Conversation

drangus
Copy link
Contributor

@drangus drangus commented Nov 21, 2017

Fix for NullReferenceException when calling GetVortoValue : allow return of defaultValue or if the recursive parameter set true traversal up the tree.

drangus and others added 2 commits November 21, 2017 20:06
Fix for NullReferenceException when calling GetVortoValue : allow return of defaultValue or if the recursive parameter set true traversal up the tree.
Whitespacing formatting, to match current code style
@leekelleher
Copy link
Contributor

Thanks @drangus! 🎉 I've updated the PR with whitespace formatting - to match the current code style.

Once @mattbrailsford is happy, we can merge it in 👍

@leekelleher leekelleher changed the title Update IPublishedContentExtensions.cs Fixes NullReferenceException with recursive GetVortoValue Nov 23, 2017
@sniffdk
Copy link

sniffdk commented Feb 16, 2018

Was just about to submit a PR for this as well. This change should also be added to DoInnerHasVortoValue.

Copy link

@sniffdk sniffdk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Imho the unnecessary nesting should be replaced with something like
if (prop == null)
{
return defaultValue;
}

@tristanjthompson
Copy link

Was just going to raise an issue/PR for this! Might it be approved/merged/released soon? Anything I can do to help?

@mattbrailsford mattbrailsford merged commit 17ba60a into umco:develop Aug 17, 2018
mattbrailsford added a commit that referenced this pull request Aug 17, 2018
@mattbrailsford
Copy link
Collaborator

This has now been merged in (and tweaked as per @sniffdk's suggestions) and is available via the build server https://ci.appveyor.com/project/UMCO/umbraco-vorto/build/1.6.1.114/artifacts or via MyGet https://www.myget.org/gallery/umbraco-packages.

I'll have a chat with @leekelleher to see if we push a release out now, or try and address any of the open issues first.

@drangus
Copy link
Contributor Author

drangus commented May 29, 2019

The applied patch doesn't allow recursion!

Change to:

if (prop == null)
{
    // PR #100 - Prevent generation of NullReferenceException: allow return of defaultValue or traversal up the tree if prop is null
    //return defaultValue;
    return recursive && content.Parent != null? content.Parent.DoGetVortoValue<T>(propertyAlias, cultureName, recursive, defaultValue) : defaultValue;
}

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

Successfully merging this pull request may close these issues.

5 participants