Action "Add an item to a list" needs to be ported to Rules 8.x-3.x.

Comments

Anonymous’s picture

dasjo’s picture

Title: Port "Add an item to a list" to 8.0.x » Port "Add an item to a list" action to D8
Anonymous’s picture

Status: Active » Needs review
dasjo’s picture

Assigned: » Unassigned

thanks steve, unassigning as per the needs review state

fago’s picture

Status: Needs review » Needs work

Thanks! See PR, some minor stuff needs work.

Anonymous’s picture

Bit stuck on this one, here's the deets I posted to github:

Hi - am stuck on two issue on this one, just spoke to fago who didn't know what the issue is either so posting here whilst trying to debug too, although may move to something else whilst I'm stuck!

I'm setting a provided value but it says the context is not valid. Also I'm having to force values for 'unique' and 'pos' even though I say they're not required. Here's the output of my test:

There was 1 error:

1) Drupal\Tests\rules\Unit\Action\ListAddItemTest::testActionExecution
Drupal\Component\Plugin\Exception\ContextException: The outputlist provided context is not valid.

/Users/steve/Sites/d8dev/modules/rules/src/Context/RulesContextTrait.php:61
/Users/steve/Sites/d8dev/modules/rules/src/Context/RulesContextTrait.php:50
/Users/steve/Sites/d8dev/modules/rules/src/Context/RulesContextTrait.php:40
/Users/steve/Sites/d8dev/modules/rules/src/Plugin/Action/ListAddItem.php:90
/Users/steve/Sites/d8dev/modules/rules/tests/src/Unit/Action/ListAddItemTest.php:66
phar:///usr/local/bin/phpunit/phpunit/TextUI/Command.php:179
phar:///usr/local/bin/phpunit/phpunit/TextUI/Command.php:132

There was 1 failure:

1) Drupal\Tests\rules\Unit\Condition\UserHasEntityFieldAccessTest::testConditionEvaluation
Failed asserting that false is true.

/Users/steve/Sites/d8dev/modules/rules/tests/src/Unit/Condition/UserHasEntityFieldAccessTest.php:123
phar:///usr/local/bin/phpunit/phpunit/TextUI/Command.php:179
phar:///usr/local/bin/phpunit/phpunit/TextUI/Command.php:132

FAILURES!

Anonymous’s picture

ok, think it's fixed now & ready for testing

Anonymous’s picture

Status: Needs work » Needs review
fago’s picture

Status: Needs review » Needs work

Thanks - I reviewed your PR and posted come comments.

  • klausi committed 5217f8e on 8.x-3.x
    Merge pull request #102 from stevepurkiss/port-list-add-item-to-D8-...
klausi’s picture

Status: Needs work » Fixed

Merged, thanks!

Status: Fixed » Closed (fixed)

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

fago’s picture

Re-opening for addressing the todo for the info alter. Check the d7 info alter, implement like it's done for FetchEntityById (see refineContextDefinitions()) and add test coverage for it.

fago’s picture

Status: Closed (fixed) » Needs work
dasjo’s picture

Assigned: Unassigned » dasjo
dasjo’s picture

Status: Needs work » Needs review

I made some progress here.

While EntityFetchByField refines based on the type being configured, in this case we need to get the type from the configured context itself.

We noticed that ContextHandlerTrait::mapContext() should also work without a state.

The code can be found in this branch:

https://github.com/fago/rules/compare/8.x-3.x...dasjo:2317193-data-add-w...

This was maybe a good exercise and can be useful for other plugins, but I realised that we shouldn't provide anything for this action: the drupal 7 action just manipulates the list and also the drupal 8 implementation is coded like this. So instead of refining context here, we just remove the provided definition.

Made a pull request:

https://github.com/fago/rules/pull/196

fago’s picture

Status: Needs review » Fixed

Thanks, merged.

  • 586f85b committed on 8.x-3.x
    Merge pull request #196 from dasjo/2317193-data-list-add
    
    Issue #2317193...

Status: Fixed » Closed (fixed)

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