Losing part definitions in latest build (commit a62e580cdf8c)

[orchardbot]

GQAdonis created: https://orchard.codeplex.com/workitem/19944

I experienced the following issue:

After installing the latest build, I created a Content Part called "Sport", adding a single "Name" field as an Input Field. Saved it. I created a Content Type called "Sport", adding a "Body Part". I looked to add my "Sport" part to it (with the intention of having the type separate from the part, but it was not listed. I clicked on the "Content Parts" tab to look for the "Sport" part and did not find it. I tried to create it again, but I got an error that it already existed.

I clicked back on the Content Types tab and selected my Sport content type. I still could not find the "Sport" part to add, so I decided to add a "Name" field. When I did so, the "Technical Name" of the field appeared as "Name-1", which should be a mistake, as I have no conflicting "Name" field on the type--only the "Sport" part that I now cannot find.

Of course, I now have no way to delete the "Sport" part to get back to a good state.

This is probably a pretty serious error. Let me know if you have questions, and I will look into it myself.

[orchardbot]

@csurieux commented:

Yes, I agree this is a bug. It must has been here since long time(?) You could delete your Sport Content Item and your Sport Part should re-appear. Problem is Orchard is not accepting a contentype and part with same name...but it let you create the contentype with same name as an existing Part. Their should be a test before content item Create. As the Part Name can't be changed, may be the idea is to force each Part Name to end with Part -> SportPart in your case.

[orchardbot]

@csurieux commented:

Proposed patch here to add a test on Content Type creation.

I think we should also force the Part Name on Part creation, file ContentDefinitionService.cs

        public EditPartViewModel AddPart(CreatePartViewModel partViewModel) {
            var name = partViewModel.Name.Trim();
            if ( !name.EndsWith("Part",StringComparison.InvariantCultureIgnoreCase) )
                name = string.Format("{0}Part",name);

            if (_contentDefinitionManager.GetPartDefinition(name) != null)
                throw new OrchardException(T("Cannot add part named '{0}'. It already exists.", name));

            if (!String.IsNullOrEmpty(name)) {
                _contentDefinitionManager.AlterPartDefinition(name, builder => builder.Attachable());
                return new EditPartViewModel(_contentDefinitionManager.GetPartDefinition(name));
            }

            return null;
        }

[orchardbot]

@csurieux commented:

The missing patch previously announced (I hope well formatted....)

[orchardbot]

@csurieux commented:

(I hate the codeplex missing edit feature for already posted content here)

[orchardbot]

GQAdonis commented:

A quick question:

Should we not be able to have duplicate "part" and "type" names? This already exists elsewhere in built-in parts like "Comment". I would seem that coupling property value requirements for 2 different but related entity types (i.e. ContentTypeDefinition and ContentPartDefinition) might be problematic as a hard requirement.

[orchardbot]

@csurieux commented:

From what I understand ( let's be cautious :) ), when you create a content type, Orchard automatically reserve a 'hidden' part with the same name in order to attach potential content type fields (a field need a part).

The bug here is that if a part already exists whith same name you have no warning or message and it will no more be displayed becaused it is considered as an implicit part.

It could work if the intention is to have a real implicit part, but it's a kind of bet...and you no more can use your part. Very strange and marvelous ?

[orchardbot]

@csurieux commented:

This is the reason why I also proposed some change pour Part UI creation here https://orchard.codeplex.com/workitem/19945

don't hesitate to vote if you agree with me.

[orchardbot]

GQAdonis commented:

Actually, according to the code in the ContentDefinitionManager, the new content type should simply attach itself to an existing part of the same name if it exists, as referenced below:

private ContentPartDefinitionRecord Acquire(ContentPartDefinition contentPartDefinition) {
        var result = _partDefinitionRepository.Table.SingleOrDefault(x => x.Name == contentPartDefinition.Name);
        if (result == null) {
            result = new ContentPartDefinitionRecord { Name = contentPartDefinition.Name };
            _partDefinitionRepository.Create(result);
        }
        return result;
    }

The process of organizing the "content type namespace" might prove difficult if any requirements for naming are imposed. In addition, these restrictions would then have to be enforced for custom modules as well (however, it seems that the custom module documentation already defines the semantic you describe).

I keep thinking that perhaps we assure that he code above results in a valid set of records (which it seems to even with the reported issue) and fix the queries for displaying the parts list as well as the logic for assigning field names in the content management views. Thoughts?

[orchardbot]

@csurieux commented:

What I am proposing is not organising the 'content type namespace' but avoiding the bug you are reporting here. The proposed patch here is simply preventing the 'implict part effect': the part you have created event if attachable, is no more usable in another content type because it will be dedicated to contain the fields for one content type, only one. This is the reason why this part is filtered in ContentDefinitionService

        public IEnumerable<EditPartViewModel> GetParts(bool metadataPartsOnly) {
            var typeNames = GetTypes().Select(ctd => ctd.Name);

            // user-defined parts
            // except for those parts with the same name as a type (implicit type's part or a mistake)
            var userContentParts = _contentDefinitionManager
                .ListPartDefinitions()
                .Where(cpd => !typeNames.Contains(cpd.Name))
                .Select(cpd => new EditPartViewModel(cpd));

I think this filtering is Ok, considering that the Content Types fields are unique for one contentType, so you can no more attach your part in another content type, so it has to be removed from the potential parts. Ok ?

I am not sure that the average users want to create these implicit parts manually.

The other point only concerns

[orchardbot]

@csurieux commented:

Sorry for pushing before ending,... The other work item I am proposing concerns only parts and would be a simple security, no constraint on name.

[orchardbot]

GQAdonis commented:

Oh, okay. I understand. I think. I will try your patch and let you know what happens!

This interests me quite a bit, since I am in the middle of writing a module called Orchard.Api that will provide a REST interface for managing Orchard content (and type and part definitions).

It was in this development that I found this issue.

[orchardbot]

@csurieux commented:

Then could you put a +1 on my other suggestion which works in the same direction: cleaning and Disambiguation for Orchard.

Concerning your module I have seen, but Orchard already contains WebAPI ? Some guys from the comitte have already implemented extensions, especially concerning OData there is something coming..... I am also in the subject look for Datwendo on Git....

[orchardbot]

GQAdonis commented:

I +1'd your suggestion from above. As for the Orchard.WebApi effort, that is intended to provide a consistent API that mobile applications can use to interact with Orchard. I have started a repository at https://github.com/CloudMetal/Orchard.Api if you are interested in taking a look.

The vision for it is described in the following places:

https://orchard.codeplex.com/discussions/450170 https://github.com/CloudMetal/Orchard.Api/issues/1

I would welcome your input on it. It is still in pre-alpha stage, but it is coming together pretty fast. I would love input from everyone on the design of the JSON specification for POST/PUT operations as well as extensions allowed to that.

[orchardbot]

@jetski5822 commented:

From what I have seen you are imposing JSON as the transport type.. is this right? should you not let the client decide that?

[orchardbot]

AimOrchard commented:

@Jetski5822 What would you do then? Add support for XML? Again, wouldn't that impose only JSON & XML as the transport type, what if clients want more?

Sometimes choices have to be made ;)

[orchardbot]

GQAdonis commented:

@Jetski5822: The intention was indeed to ensure a JSON interface that is efficient for mobile applications to use. However, this is officially still REST-compliant, as content negotiation is supported. In the current implementation, we just say "no" when types other than JSON are requested via HTTP headers.

Eventually, it might be good to support XML and image MIME types (under certain circumstances), but I figure we can wait to see what the demand is after getting this first version out.

I will be updating again today sometime.

[urbanit]

Any news on this?

[thekaveman]

This is related to #3773