Port "List contains item (list_contains)" condition to 8.x

Assigning to me as this is a requirement for #2317193: Port "Add an item to a list" action to D8 which I am working on.

Code for this is now up at https://github.com/fago/rules/pull/99

I was working on this on the TGV. Doing the "run tests & see if I can figure out the error messages enough" route - started learning about typed data by reading the code & interfaces as couldn't tether on the train. Got as far as getting this error - I think I'll figure it out once I understand typed data but understand if you need to move it forward - I've only one example to go from at the mo (the one which fago mentioned) but understanding more minute by minute ;) Would like to continue but understand if you need to move it ahead though, I'll read the solution with interest.

There was 1 error:

1) Drupal\Tests\rules\Integration\Condition\ListContainsTest::testRefiningContextDefinitions
Drupal\Component\Plugin\Exception\ContextException: The list provided context is not valid.

/Users/steve/Sites/d8dev/modules/rules/src/Context/ContextProviderTrait.php:55
/Users/steve/Sites/d8dev/modules/rules/tests/src/Integration/Condition/ListContainsTest.php:156

FAILURES!
Tests: 130, Assertions: 275, Errors: 1.

This is what I've put in DataListContains:

class DataListContains extends RulesConditionBase {

/**
   * {@inheritdoc}
   */
  public function refineContextDefinitions() {
    if ($list = $this->getContextValue('list')) {
      //print_r($list);
      print_r($this->pluginDefinition['context']['list']->getDataType());
      $this->pluginDefinition['context']['list']->setDataType("list");
      print_r($this->pluginDefinition['context']['list']->getDataType());
    }
    /*
    if ($item = $this->getContextValue('item')) {
      $this->pluginDefinition['context']['item']->setDataType("list:$item");
    }
    */
  }

  /**
   * {@inheritdoc}
   */
  public function summary() {

...and this in ListContainsTest (which I'm guessing should be renamed DataListContainsTest?):

  /**
   * @covers ::refineContextDefinitions
   */
  public function testRefiningContextDefinitions() {

    // Test array of string values
    $list = ['One','Two','Three'];

    $this->condition
      ->setContextValue('list', $list)
      ->setContextValue('item', 'Zero');
    $this->condition->refineContextdefinitions();
    $this->assertEquals(
      $this->condition->getProvidedContextDefinition('list')
        ->getDataType(), 'list:list'
    );
  }

I know the code doesn't make much logic at the moment because I'm half way between copy & paste and "oh... d'oh" moments.

Thanks for an awesome week, have set some time aside this week to look into the derivatives to see if I can understand then document that too.

Trying to keep momentum going this time!

OK, current question I have at the moment because I've been reading about typed data api over at #1794140: Typed Data API overview:

- the example in EntityFetchByField enables refining of the provides = {"entity_fetched" whereas there is no provides in DataListContains, it just returns the resultant value of in_array($item, $list). Does this mean it shouldn't just return a value but set a provided context so we can then add the refine context?

...but that doesn't make much sense, because why would you want to change a boolean result? Was trying to loop through the list earlier but didn't really 'get' why you would want to change the type of each list item but perhaps you would.

Oh, hang on, item is a list too, isn't it - d'oh.
And you probably would want to change the type of a list item - d'oh.

Maybe, just thinking out loud as you said update... I generally don't as you can see the threads get long and often embarrassing for me ;)

Another day and it's like a new brain. Reading the example again I see it's seeing if there's a type sent through in the context, and if so then sets the type of the entity provided. That's because that's what that rules action does - this rules condition takes a list and an item and returns whether or not the item is in the list. The contexts of any of these could be refined, so the code needs to get the info using the Typed Data API. The doc on that is sparse but links to the more thorough Entity API docs at https://www.drupal.org/node/1795854 which I'm going through.

So I may laugh at this in time to come, but lists are complex structures so presumably I'll need to step through the list with the refine context function, and once on the item (which isn't a list as I said above, but I could still change my mind...)

OK so I can see I can use getProvidedContextDefinition('item')->getDataType(); to get the type of the item, but now confused as to what I would then set - the example is clearer as type is an actual context. Now less sure I'll need to do anything to the list.

Guess I should hop on IRC at some point, I'll RTFM more first - must say I'm impressed with the quality of the Drupal 8 Entity API documentation there is in the code, makes it a pleasure to read, thanks!

OK this is what I have so far for item, working on list:

class DataListContains extends RulesConditionBase {

  /**
   * {@inheritdoc}
   */
  public function refineContextdefinitions() {

    // Refine the item context
    if($item = $this->getContextValue('item')) {
      if ($item instanceof EntityInterface && $type = $item->getTypedData('type')) {
        $this->pluginDefinition['context']['item']->setDataType("entity:$type");
      }
    };

  }

  /**

  /**
   * @covers ::refineContextDefinitions
   */
  public function testRefiningContextDefinitions() {

    // Create array of mock entities
    $entity_zero = $this->getMock('Drupal\Core\Entity\EntityInterface');
    $entity_zero->expects($this->any())
      ->method('id')
      ->will($this->returnValue('entity_zero_id'));

    $entity_one = $this->getMock('Drupal\Core\Entity\EntityInterface');
    $entity_one->expects($this->any())
      ->method('id')
      ->will($this->returnValue('entity_one_id'));

    // Test array of entities
    $entity_list = [$entity_zero, $entity_one];

    $this->condition
      ->setContextValue('list', $entity_list)
      ->setContextValue('item', $entity_zero);
    $this->condition->refineContextdefinitions();
    $this->assertEquals(
      $this->condition->getContextValue('item'), $entity_zero);
  }

That's one of the questions I mentioned in my ramblings above, good to see I wasn't too far off in my thinking.

Enjoyed my little trip around the entity api, it's slowly making more sense!

Good point, well made. I'll RTF7M ;)

The code in 7 uses the same info alter for add and remove actions, posting here whilst I get lost in setting & getting context definitions!

/**
 * Info alteration callback for the "Add and Remove a list item" actions.
 */
function rules_data_list_info_alter(&$element_info, RulesAbstractPlugin $element) {
  // Update the required type for the list item if it is known.
  $element->settings += array('list:select' => NULL);
  if ($wrapper = $element->applyDataSelector($element->settings['list:select'])) {
    if ($type = entity_property_list_extract_type($wrapper->type())) {
      $info = $wrapper->info();
      $element_info['parameter']['item']['type'] = $type;
      $element_info['parameter']['item']['options list']  = !empty($info['options list']) ? 'rules_data_selector_options_list' : FALSE;
    }
  }
}

So here's where my thinking's at currently:

- from what I can tell, 7 allows you to alter the type of a list item. It uses the same alter hook for many similar functions such as add item to list, list contains, etc. With my limited knowledge of form api I can't tell whether it allows for the type of the item being added to the list (or whatever the function is) to be altered or not, but my hunch is it doesn't. Should it? That's another question - if alter hooks are there for enabling the complete control over types at runtime then yes, it should.

- in 8 we should allow for refining configs on all available places, i.e. type of item and type of each item in list. I don't currently know how to achieve this in code, hence why posting update here before I spend days figuring out something which is wrong ;) It does beg the question though that surely if all this alter 'hook' is allowing for is changing of type then it could be extracted to a level higher up as 'type' should be discoverable?

Still reading up on various discussions around this so no doubt more will become clear as has done in the past week!

Have updated the PR with (working) code and comments (with questions) https://github.com/fago/rules/pull/184#issuecomment-104692862

Hi - so I've been following progress today and saw the resultant issue https://www.drupal.org/node/2501939

Could you please have a look at this again and let me know what, if anything I can do on this? Am not really understanding the whole refining context story.

Have been stuck on this for ages, happy to look into other issues, feel free to point me to something!

Cool thanks - also met @matslats who created and maintains mutual credit suite of modules last week at a workshop on money here in Brightoncisco - certainly an interesting area! I mentioned I'd been helping on rules and he's got a D8 version in development needing help with rules so will also apply some knowledge and time there - it's good to see things in action, helps my understanding of things. Drupal dev is a little abstract ;)

See my comments on https://github.com/fago/rules/pull/184#discussion_r33425007 - I added the code but it doesn't pass my tests checking whether it refines the context or not.

Still working out what I need to mock for the tests to work - not sure what I'd ask if I was on IRC so reading up more on mocking

Having discussed this with dasjo, fago, and klausi I am postponing this until the issue about runtime context setting is figured out.