In #3094046: Set 'assignment_restriction' in Condition and RulesAction contexts the majority of actions and conditions which need an assignment restrictor were updated. However, there are two conditions which got missed. The "DataIsEmpty" and "DataListCountIs" conditions both need to be set to 'Selector' and the description in each also states this. In the UI pressing the 'data selection' button for DataIsEmpty does actually work and you can switch. However, for the 'List Count' condition you get an InvalidArgument Exception "Cannot set a list with a non-array value. "

Patch for both of these to follow

CommentFileSizeAuthor
#3 3159876-2.selector-DataIsEmpty-listCount.patch1.59 KBjonathan1055
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jonathan1055 created an issue. See original summary.

jonathan1055’s picture

jonathan1055 CreditAttribution: jonathan1055 as a volunteer commented
Title: Set assignment restrictor » Set assignment restrictor for list and data conditions
Status: Active » Needs review

Patch for the two identified so far. As in comment #10 on #3094046-10: Set 'assignment_restriction' in Condition and RulesAction contexts I will now check all the List conditions and actions.

jonathan1055’s picture

jonathan1055 CreditAttribution: jonathan1055 as a volunteer commented
FileSize
3159876-2.selector-DataIsEmpty-listCount.patch1.59 KB
jonathan1055’s picture

jonathan1055 CreditAttribution: jonathan1055 as a volunteer commented
Title: Set assignment restrictor for list and data conditions » Set assignment restrictor for two list and data conditions

I've checked all List and Data actions and conditions in D7 using UI not looking at the source code. With the two addtions in the patch above, D8 now matches D7 for all of the settings for picking the list or data item.

For specifying the list item to add or remove or check against, D7 has these restricted to selector only, but in D8 we allow both direct input and selector. I think this is probably OK, and is an improvement in functionality.

FYI, I found these missing selectors due to my current work in expanding the ActionFormTest and ConditionFormTest to actually cover entering and saving data for every action and condition. These enhancements could be added here.

TR’s picture

TR CreditAttribution: TR commented

If you want, you can add the tests just for these two conditions, but I don't want to mix in unrelated stuff.

I'm also fine with just leaving the tests out of this and and opening a new issue for your test case refactoring/improvement.

jonathan1055’s picture

jonathan1055 CreditAttribution: jonathan1055 as a volunteer commented

Thanks TR. Yes it's better to not fix up the issues. I have added all the expanded tests on #3160067: Refactor and expand ActionsFormTest and ConditionsFormTest

I will work on that issue, and when ready you can commit all the tests (with the failing ones commented out) then we can demonstrate here that this patch fixes it.

  • TR committed 06f71d4 on 8.x-3.x authored by jonathan1055 
    Issue #3159876 by jonathan1055: Set assignment restrictor for two list...
TR’s picture

TR CreditAttribution: TR commented
Status: Needs review » Fixed

Thanks. Committed.

jonathan1055’s picture

jonathan1055 CreditAttribution: jonathan1055 as a volunteer commented

Thank you. I have uncommented the test for this on #3160067: Refactor and expand ActionsFormTest and ConditionsFormTest

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.