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

Migrate nuPickers to v8 #199

Open
wants to merge 46 commits into
base: v8
Choose a base branch
from
Open

Migrate nuPickers to v8 #199

wants to merge 46 commits into from

Conversation

bielu
Copy link

@bielu bielu commented Aug 14, 2019

It is an initial version for v8, few things have to be still polished, but works! I am happy to continue to work on UI changes and/code optimizations. I think it is ready to be published as prerelease and check with other Umbraco developers if everything is working as intended

@bowserm
Copy link
Contributor

bowserm commented Nov 7, 2019

@bielu,

Any chance we could change the GetPropertyValueType method in the PickerPropertyValueConverter to the following? Right now, the property value converter in this pull request sets the type to string. This affects how ModelsBuilder generates the nupicker properties on its models.

public override Type GetPropertyValueType(IPublishedPropertyType propertyType) => typeof(Picker);

I forked off of your bielu/v8/develop branch and tried it out locally and confirmed that ModelsBuilder generates the nuPicker properties with type nuPickers.Picker after this change.

Thanks for all the hard work on this!

@bielu
Copy link
Author

bielu commented Nov 7, 2019

I will look at that's tomorrow morning 😜

@bielu
Copy link
Author

bielu commented Nov 8, 2019

@bowserm changed :)

@joepvtl
Copy link

joepvtl commented Jan 8, 2020

Hey @bielu , the saveFormat of the EnumPrefetchListPicker isn't getting saved. This is because the value is not being assigned.

-Joep

@bielu
Copy link
Author

bielu commented Jan 8, 2020

@joepvtl I will look into that tomorrow morning :)

@bowserm
Copy link
Contributor

bowserm commented Jan 16, 2020

I am seeing what I believe is the saveFormat error on the EnumRadioButtonPicker. When I look in the DB, this is what I am seeing as the prevalues for my datatype when I selected csv as the saveFormat:

{"useLabel":false,"dataSource":{"assemblyName":"ScientificGames.Core.dll","apiController":"EnumDataSourceApi","enumName":"ScientificGames.Core.Models.DataTypeDataSources.LeftRightTopBottomOrientation"},"customLabel":null,"layoutDirection":"horizontal","saveFormat":null,"items":[]}

I'll race you to a fix blielu :). I'll see if the same problem is happening to other editors.

@bowserm
Copy link
Contributor

bowserm commented Jan 16, 2020

@bielu I made a PR to the v8/develop branch on your fork. I think I have it working.

Ensure that saveFormat is saved as a string for every property editor
@bielu
Copy link
Author

bielu commented Jan 17, 2020

@bowserm thanks for the help, I reviewed that and merged code.
Actually I am quite little less active now as I have crazy time but I am happy to merge PRs.

@PezCo
Copy link

PezCo commented Jan 27, 2020

Is this Likely to be the definitive upgrade path for nuPickers? im a bit confused between this and https://github.com/uComponents/Pikachu which says its a v8 rebuild.

is there any "official" word on plans for v8 its almost a year old now.

@bielu
Copy link
Author

bielu commented Jan 28, 2020

@PezCo this is Upgrade of actual nuPickers which I started, as I needed use nuPickers in one of the projects, Pikachu was supposed to rebuild that for v8, not direct migration like that branch. @Hendy is responsible for the official part of that, but not sure what is his decision at all :).

@ronaldbarendse
Copy link

If you need an alternative to nuPickers for Umbraco 8, you might also be interested in Data List from @leekelleher: https://github.com/leekelleher/umbraco-contentment/blob/master/docs/editors/data-list.md.

@PezCo
Copy link

PezCo commented Jan 28, 2020

Ideally i want this branch.
I have a fairly large public Umbraco v7 site that i'm upgrading to v8 that makes use of nuPickers.
Unfortunately im not really experienced in umbraco plugins or its source code so not in much of a position to help here, im just looking for the least pain free way to migrate.

Ric Carey and others added 2 commits May 5, 2020 17:45
…n render macro by injecting UmbracoComponentRenderer
upgraded to latest version of Umbraco + fixed issue with null check o…
@jesperbrasmussen
Copy link

jesperbrasmussen commented Jun 5, 2020

Great work. I have made my own build of your branch to use on an Umbraco V8 installation (v. 8.6.1). But i ran into some problems with PanicExceptions getting thrown in PickerPropertyValueConverter when it tries to access the parent property.
It looks like it's related to NuCache, but i can't seem to solve it by just rebuilding the cache.
I'm using modelsbuilder and it happens when i try to get the values of the property that use the nupicker datatype. It's shows op fine in the Backoffice.

public override object ConvertIntermediateToObject(IPublishedElement owner, IPublishedPropertyType propertyType,
            PropertyCacheLevel referenceCacheLevel, object inter, bool preview)
        {
            int contextId = -1;
            int parentId = -1;

            IPublishedContent assignedContentItem = owner as IPublishedContent;

            if (assignedContentItem != null)
            {
                contextId = assignedContentItem.Id;

                if (assignedContentItem.Parent != null)
                {
                    parentId = assignedContentItem.Parent.Id;
                }
            }

            return new Picker(
                contextId,
                parentId,
                propertyType.EditorAlias,
                propertyType.DataType.Id,
                propertyType.Alias,
                inter);
        }

My workaround was to surround the failing code with a try/cacth and ignore the PanicException. It solves it in my case, but i have yet to discover if it has any sideeffects.

if (assignedContentItem != null)
            {
                contextId = assignedContentItem.Id;

                try
                {
                    if (assignedContentItem.Parent != null)
                    {
                        parentId = assignedContentItem.Parent.Id;
                    }
                }
                catch (PanicException e)
                {
                    //Ignore PanicException
                }
                
            }

@bielu
Copy link
Author

bielu commented Jun 5, 2020

@jesperbrasmussen can you add which version of v8 are you using? and I will look into that issue when I will have time

@jesperbrasmussen
Copy link

@jesperbrasmussen can you add which version of v8 are you using? and I will look into that issue when I will have time

Yes it's v. 8.6.1

@bielu
Copy link
Author

bielu commented Jun 5, 2020

@jesperbrasmussen can you also paste exception log as maybe I will be find out that just by exception as I check Umbraco source code and not much change here about ConvertIntermediateToObject and I will need to recreate but would prefer be sure it is same exception if I will cause any :)

@jesperbrasmussen
Copy link

I have just tested it on the lastest version of Umbraco which is v8.6.2, and I still get get the same error.

This is the first part of the exception:

<Error>
<Message>An error has occurred.</Message>
<ExceptionMessage>invalid item type</ExceptionMessage>
<ExceptionType>Umbraco.Core.Exceptions.PanicException</ExceptionType>
<StackTrace> at Umbraco.Web.PublishedCache.NuCache.PublishedContent.GetGetterById() in D:\a\1\s\src\Umbraco.Web\PublishedCache\NuCache\PublishedContent.cs:line 123 at Umbraco.Web.PublishedCache.NuCache.PublishedContent.get_Parent() in D:\a\1\s\src\Umbraco.Web\PublishedCache\NuCache\PublishedContent.cs:line 261 at nuPickers.PickerPropertyValueConverter.ConvertIntermediateToObject(IPublishedElement owner, IPublishedPropertyType propertyType, PropertyCacheLevel referenceCacheLevel, Object inter, Boolean preview) in S:\Solutions (external)\nuPickers\source\nuPickers\PickerPropertyValueConverter.cs:line 118 at Umbraco.Core.Models.PublishedContent.PublishedPropertyType.ConvertInterToObject(IPublishedElement owner, PropertyCacheLevel referenceCacheLevel, Object inter, Boolean preview) in D:\a\1\s\src\Umbraco.Core\Models\PublishedContent\PublishedPropertyType.cs:line 219 at Umbraco.Web.PublishedCache.NuCache.Property.GetValue(String culture, String segment) in D:\a\1\s\src\Umbraco.Web\PublishedCache\NuCache\Property.cs:line 211 at Umbraco.Web.PublishedPropertyExtension.Value[T](IPublishedProperty property, String culture, String segment, Fallback fallback, T defaultValue) in D:\a\1\s\src\Umbraco.Web\PublishedPropertyExtension.cs:line 39 at Umbraco.Web.PublishedContentExtensions.Value[T](IPublishedContent content, String alias, String culture, String segment, Fallback fallback, T defaultValue) in D:\a\1\s\src\Umbraco.Web\PublishedContentExtensions.cs:line 163 at MyProject.UmbracO2.Models.CustomerMember.get_AllowedTypesForMember() in S:\Solutions\MyProject\MyProject.com_v8\MyProject.UmbracO2\Models\CustomerMember.generated.cs:line 60 at MyProject.UmbracO2.Models.CustomerMember.MyProject.Core.Interfaces.Member.ICustomerMember.get_AllowedTypesForMember() in S:\Solutions\MyProject\MyProject.com_v8\MyProject.UmbracO2\Models\CustomerMember.cs:line 16 at MyProject.UmbracO2.Services.UmbracoMembershipService.GetAllowedTypes(Int32 memberId) in S:\Solutions\MyProject\MyProject.com_v8\MyProject.UmbracO2\Services\UmbracoMembershipService.cs:line 63 at MyProject.UmbracO2.Services.Member.MemberAdministrationService.GetAllowedTypes(Int32 memberId) in S:\Solutions\MyProject\MyProject.com_v8\MyProject.UmbracO2\Services\Member\MemberAdministrationService.cs:line 347 at MyProject.UmbracO2.Controllers.Backoffice.Section.AdminToolController.<>c__DisplayClass6_0.<ListUsers>b__3(IMember group) in S:\Solutions\MyProject\MyProject.com_v8\MyProject.UmbracO2\Controllers\Backoffice\Section\AdminToolController.cs:line 71 at System.Linq.Lookup`2.Create[TSource](IEnumerable`1 source, Func`2 keySelector, Func`2 elementSelector, IEqualityComparer`1 comparer) at System.Linq.Enumerable.ToLookup[TSource,TKey,TElement](IEnumerable`1 source, Func`2 keySelector, Func`2 elementSelector) at MyProject.UmbracO2.Controllers.Backoffice.Section.AdminToolController.ListUsers(Boolean includeInactive) in 

@bielu
Copy link
Author

bielu commented Jun 8, 2020

@jesperbrasmussen thanks that error log is helpful! Will look over weekend how to fix that issue! :)

@bielu
Copy link
Author

bielu commented Jun 20, 2020

@jesperbrasmussen I tried on clear umbraco 8.6.2 today and can't reproduce that issue,

@jesperbrasmussen
Copy link

@jesperbrasmussen I tried on clear umbraco 8.6.2 today and can't reproduce that issue,

Ok. I'll try to find time to do a test on a clean umbraco 8.6.2, and see if i can find a way to reproduce the error

niekvanderreest and others added 2 commits November 9, 2020 13:37
When a nuPicker is added to a custum MemberType and an instcance of that membertype is loaded through use of the memberservice i.e. Umbraco.Web.Composing.Current.UmbracoHelper.Member(id), the PickerPropertyValueConverter throws an error on line: 117.

The check assignedContentItem.Parent != null appears not to be a valid check for IMember eventhough it's successfully cast to IPublishedContent, the fix works around the check by getting the parent id from the Path property.
Fixed issue for nuPickers added to Membertypes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

8 participants