OrchardCMS / Orchard

Orchard is a free, open source, community-focused Content Management System built on the ASP.NET MVC platform.
https://orchardproject.net
BSD 3-Clause "New" or "Revised" License
2.38k stars 1.12k forks source link

Taxonomy Fields do not save state correctly where there is more than 1 taxonomy field in a content type (with fix) #4012

Closed orchardbot closed 9 years ago

orchardbot commented 11 years ago

Jimasp created: https://orchard.codeplex.com/workitem/20184

Problem:

When a content item has 2 taxonomy fields, filling in one taxonomy field, but not the other, does not re-display the submitted page state to the user. (Note that I tested this with Autocomplete fields, but I expect it effects the checkbox tree the same way).

Recreate:

Create 2 separate taxonomies. Add a couple of terms to each. Edit a Content Definition for a content type, and add 2 taxonomy fields to the definition, one for each taxonomy. Select the required and autocomplete options for both fields. Do not select any other options. Create a new content item with the content type you chose at (2). Fill in one of the taxonomy fields but NOT the other. Hit Publish.

Observe that the state of the filled-in taxonomy field is lost. (NOTE - If you do not see the problem using the above steps, this may be due to the call order, so leave the other taxonomy field blank and fill in the alternate one, and re-test).

Fix:

In the file ..\Orchard.Web\Modules\Orchard.Taxonomies\Drivers\TaxonomyFieldDriver.cs: .

a. Change this line: if (updater.TryUpdateModel(viewModel, GetPrefix(field, part), null, null))

To this: var modelUpdated = updater.TryUpdateModel(viewModel, GetPrefix(field, part), null, null);

field.TermsField.Value = viewModel.Terms
    .Where(t => (t.IsChecked || t.Id == viewModel.SingleTermId))
    .Select(t => GetOrCreateTerm(t, viewModel.TaxonomyId, field))
    .Where(t => t != null).Distinct(new TermPartComparer()).ToList();

if (modelUpdated)

b. Remove this line: var checkedTerms = viewModel.Terms .Where(t => (t.IsChecked || t.Id == viewModel.SingleTermId)) .Select(t => GetOrCreateTerm(t, viewModel.TaxonomyId, field)) .Where(t => t != null).ToList();

c. Change this line: if (settings.Required && !checkedTerms.Any())

To this: if (settings.Required && !field.TermsField.Value.Any())

d. Change this line: _taxonomyService.UpdateTerms(part.ContentItem, checkedTerms, field.Name);

To this: _taxonomyService.UpdateTerms(part.ContentItem, field.TermsField.Value, field.Name);

e. Change this line: var appliedTerms = _taxonomyService.GetTermsForContentItem(part.ContentItem.Id, field.Name).Distinct(new TermPartComparer()).ToDictionary(t => t.Id, t => t);

To this: var appliedTerms = new Dictionary<int, TermPart>(); if (field.TermsField.Value != null) { appliedTerms = field.TermsField.Value.ToDictionary(t => t.Id, t => t); }

Assuming you are using the same 1.7.1 source unchanged, then the final code for the 2 methods should look like this: protected override DriverResult Editor(ContentPart part, TaxonomyField field, dynamic shapeHelper) { return ContentShape("Fields_TaxonomyField_Edit", GetDifferentiator(field, part), () => { var settings = field.PartFieldDefinition.Settings.GetModel();

            var appliedTerms = new Dictionary<int, TermPart>();
            if (field.TermsField.Value != null) {
                appliedTerms = field.TermsField.Value.ToDictionary(t => t.Id, t => t);
            }

            var taxonomy = _taxonomyService.GetTaxonomyByName(settings.Taxonomy);
            var terms = taxonomy != null
                ? _taxonomyService.GetTerms(taxonomy.Id).Where(t => !string.IsNullOrWhiteSpace(t.Name)).Select(t => t.CreateTermEntry()).ToList()
                : new List<TermEntry>(0);

            terms.ForEach(t => t.IsChecked = appliedTerms.ContainsKey(t.Id));

            var viewModel = new TaxonomyFieldViewModel
            {
                DisplayName = field.DisplayName,
                Name = field.Name,
                Terms = terms,
                Settings = settings,
                SingleTermId = terms.Where(t => t.IsChecked).Select(t => t.Id).FirstOrDefault(),
                TaxonomyId = taxonomy != null ? taxonomy.Id : 0
            };

            var templateName = settings.Autocomplete ? "Fields/TaxonomyField.Autocomplete" : "Fields/TaxonomyField";
            return shapeHelper.EditorTemplate(TemplateName: templateName, Model: viewModel, Prefix: GetPrefix(field, part));
        });
    }

    protected override DriverResult Editor(ContentPart part, TaxonomyField field, IUpdateModel updater, dynamic shapeHelper)
    {
        var viewModel = new TaxonomyFieldViewModel { Terms = new List<TermEntry>() };

        var modelUpdated = updater.TryUpdateModel(viewModel, GetPrefix(field, part), null, null);

        field.TermsField.Value = viewModel.Terms
            .Where(t => (t.IsChecked || t.Id == viewModel.SingleTermId))
            .Select(t => GetOrCreateTerm(t, viewModel.TaxonomyId, field))
            .Where(t => t != null).Distinct(new TermPartComparer()).ToList();

        if (modelUpdated)
        {
            var settings = field.PartFieldDefinition.Settings.GetModel<TaxonomyFieldSettings>();
            if (settings.Required && !field.TermsField.Value.Any())
            {
                updater.AddModelError(GetPrefix(field, part), T("The field {0} is mandatory.", T(field.DisplayName)));
            }
            else
                _taxonomyService.UpdateTerms(part.ContentItem, field.TermsField.Value, field.Name);
        }

        return Editor(part, field, shapeHelper);
    }
orchardbot commented 11 years ago

hansenf commented:

The problem might be related to a bug in UpdateTerms

 // removing current terms for specific field
var fieldIndexes = termsPart.Terms
  .Where(t => t.Field == field)
  .Select((t, i) => i)
  .OrderByDescending(i => i)
  .ToList();

I believe the above code is calculating the wrong indexes when a content item has multiple fields. A possible fix is shown below.

// removing current terms for specific field
 var fieldIndexes = termsPart.Terms
  .Select((t, i) => new { Index = i, Item = t })
  .Where(j => j.Item.Field == field)
  .Select((j, i) => j.Index)
  .OrderByDescending(i => i)
  .ToList();
orchardbot commented 11 years ago

hansenf commented:

Hadn't read 20204 :-)

orchardbot commented 11 years ago

@sebastienros commented:

https://orchard.codeplex.com/workitem/20204 (https://github.com/OrchardCMS/Orchard/issues/4032)

orchardbot commented 11 years ago

@sebastienros commented:

\ Closed by sebastienros 11/07/2013 11:43AM

orchardbot commented 11 years ago

Jimasp commented:

20204 will not fix this - I should know, as I was the person that raised both issues!

Also, I just pulled latest version of the code and the bug is still there.

Please can you re-open this issue?

Thanks.

orchardbot commented 11 years ago

Jimasp commented:

(P.S. the codeplex "reopen issue" button doesn't work for me, it just brings up a green box saying "working" and just sits there spinning).

orchardbot commented 11 years ago

@sebastienros commented:

This one is closed as Duplicate thus it ill remain closed. I am reopening the other one.

orchardbot commented 10 years ago

Jimasp commented:

Thanks Seb.

orchardbot commented 10 years ago

Jimasp commented:

If anyone has problems recreating this in 1.7.1 - see the video in comments on 20204. The problem is still there.