Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added code for refining item context #184

Open
wants to merge 6 commits into
base: 8.x-3.x
Choose a base branch
from
Open

Added code for refining item context #184

wants to merge 6 commits into from

Conversation

stephenpurkiss
Copy link
Contributor

Initial code for refining item context. See https://www.drupal.org/node/2281083#comment-9834577

Posting for visibility and testing.

public function refineContextdefinitions() {

// Refine the item context
if($item = $this->getContextValue('item')) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there should be one space after the "if"

@stephenpurkiss
Copy link
Contributor Author

Will try to get on the regular IRC meeting, meanwhile here's where I'm at on this:

https://www.drupal.org/node/2281083#comment-9866613

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!

@stephenpurkiss
Copy link
Contributor Author

OK I think I've got this working thanks to @tstoeckler who was here in spain today.

My next question is this covers refining the context for the item, not the list. In 7 you couldn't alter the item context, only the list. So I'm presuming I need to add code to alter context of the list? Do I need to step through each item in the list to achieve this? Some pointer would be good - I'm done for the day here but will continue tomorrow/next week.


// Refine the item context.
if ($item = $this->getContextValue('item')) {
if ($item instanceof EntityInterface && $type = $item->getEntityTypeId()) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why EntityInterface here? I think this should just be "$typed_data_item = $this->getContextData('item')" and then "$this->pluginDefinition['context']['item']->setDataType($typed_data_item->getDataDefinition()->getDataType());"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"$typed_data_item = $this->getContextData('item')" created this error:

PHP Fatal error: Call to undefined method Drupal\rules\Plugin\Condition\DataListContains::getContextData() in /Users/steve/Sites/d8dev/modules/rules/src/Plugin/Condition/DataListContains.php on line 45

...so I copied the code from DataComparison.php:

$item = $this->condition->getContextDefinition('item');

Your suggested code failed my test though:

Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'any'
+'entity:entity_zero_type_id'


// Get the context definition for the item
// and make sure that the type is now the entity type.
$item = $this->condition->getContextDefinition('item');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

item is a bad variable name here, since this is not the item but the context definition of item.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, well made ;)

@stephenpurkiss
Copy link
Contributor Author

OK I've done the changes recommended but now stuck at what needs mocking in the test. I realise now we're not using getDataTypeId but getDataType so changed that but still no joy. Tried mocking all the things - getContext, getContextData and getDataDefinition, but that made no difference, still coming back as 'any' type.

@stephenpurkiss
Copy link
Contributor Author

Am looking at ContextTest.php and see a whole setup routine for mocking context definitions and can see it's a bit more involved so will investigate that further

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants