U4-11161 - Content Picker 2 suddenly allows multiple nodes to be selected (inadvertent breaking change)

Created by Dan Booth 28 Mar 2018, 14:29:21 Updated by Shannon Deminick 05 Apr 2018, 11:27:23

Tags: PR Regression

Relates to: U4-11058

What did you do?

Added a Content Picker to a document type in Umbraco 7.10 - this is the picker with alias Umbraco.ContentPicker2 Left Start Node empty

What did you expect to happen?

I expected it to function as previously and allow only a single content node to be picked.

What actually happened?

It now features an "Add" button allowing multiple content nodes to be picked (this seems to be unlimited).

If more than one item is picked then this breaks the conversion back to a single IPublishedContent and thus the property value converter returns nothing. This can be site breaking where people have relied on it only picking a single value and is mandatory.

This also happens if you upgrade an existing site that uses ContentPicker2.

3 Attachments

Comments

Dave Woestenborghs 28 Mar 2018, 15:14:42

PR created for this one : https://github.com/umbraco/Umbraco-CMS/pull/2550

If you are in hurry to fix this have a look at the changes in the PR


Shannon Deminick 29 Mar 2018, 05:59:57

Is this purely a 7.10 issue? If so, i need to figure out where/when it went wrong so we can review that


Dave Woestenborghs 29 Mar 2018, 06:27:02

@Shandem i think this commit caused it. The check for showing the add button in the view changed. https://github.com/umbraco/Umbraco-CMS/commit/f2a3a269ba1c3f53dc3e3c4db2c29bada8154bbc


Dave Woestenborghs 29 Mar 2018, 06:55:34

The linked in issue is in the release notes of 7.9.3 https://our.umbraco.org/download/releases/793/

Will do a quick test to confirm


Dave Woestenborghs 29 Mar 2018, 07:04:52

I can confirm this happens in 7.9.3 also.

Also this issues occur in 7.9.3 http://issues.umbraco.org/issue/U4-11155 http://issues.umbraco.org/issue/U4-11149


Dan Booth 29 Mar 2018, 08:41:41

@Shandem It's not an issue in 7.9.2 so it must have originated in 7.9.3 based on Dave's comment.


Shannon Deminick 29 Mar 2018, 11:06:24

Yup, it's showing up in the release notes now because I updated the original task, it wasn't tagged before today unfortunately


Warren Buckley 05 Apr 2018, 09:46:07

Regression

The cause of this was a change to the Angular view that controls the logic for showing the add button, where we use model.config.maxNumber

However this commit did not have any change to the controller, so no default was ever set & assumed the property editor would always be configured and have a maxNumber set in the property editor/datatype configuration.

The original commit/change that caused the regression was from this issue and PR http://issues.umbraco.org/issue/U4-11058 https://github.com/umbraco/Umbraco-CMS/pull/2509

It just seems unfortunate with testing that a scenario was missed, where no configuration/values have been set, that caused this regression issue, as far as I can tell.

Test Notes

I have tested @dawoe changes and it all works as expected.

Tested with the starter kit before the PR applied and was able to set more than one link for the call to action on the home node in the starter kit which does throw a YSOD with the PropertyValueConvertor stuff.

With the PR applied all was working good again.

Merge Notes

  • Been merged into patch-v7 aka 7.10.2
  • New branch created patch-7.9.4 & commits cherry picked, version bumped & pushed
  • Up-merged patch-v7 into dev-v7
  • Up-merged patch-v7 into dev-v8


Dave Woestenborghs 05 Apr 2018, 10:03:02

Hi @warren.buckley 7.10.2 ? Is 7.10.1 released yet ? Did I miss something ?

Dave


Warren Buckley 05 Apr 2018, 10:21:26

Hi @dawoe Out today - see https://twitter.com/umbracoproject/status/981837305886461953


Dave Woestenborghs 05 Apr 2018, 10:23:14

Just saw it. Why is this one postponed for a version ? There are also some other IMHO breaking bugs that have a PR that could have been included.

Dave


Shannon Deminick 05 Apr 2018, 11:27:23

@dawoe the point is we needed this out now as it's affecting new customers on cloud, this doesn't mean that another version isn't on it's way. We'll have 7.10.2 out on tuesday, and of course there are always more bugs to included but it's a matter of time and priority. We'll continue to release bug fixes often. Perhaps more often than we've previously done.


Priority: Major

Type: Bug

State: Fixed

Assignee:

Difficulty: Normal

Category: Architecture

Backwards Compatible: True

Fix Submitted: Pull request

Affected versions: 7.10.0, 7.9.3

Due in version: 7.10.2, 7.9.4

Sprint: Sprint 82

Story Points: 1

Cycle: 9