-
Notifications
You must be signed in to change notification settings - Fork 4
by yanniboi: Created new field type for logging status changes. #41
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
base: 8.x-1.x
Are you sure you want to change the base?
Conversation
| $source_field = $definition->getFieldStorageDefinition()->getSetting('source_field'); | ||
|
|
||
| $value = $entity->{$source_field}->value; | ||
| $original_value = $entity->original->{$source_field}->value; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use \Drupal\Core\Field\FieldItemBase::mainPropertyName rather than assuming value?
| /** | ||
| * {@inheritdoc} | ||
| */ | ||
| public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks these are being overridden with no changes?
modules/crm_tools/src/Plugin/Field/FieldFormatter/StatusLogFormatter.php
Show resolved
Hide resolved
| foreach ($items as $delta => $item) { | ||
| /* @var \Drupal\decoupled_auth_crm_tools\Plugin\Field\FieldType\StatusLog $item */ | ||
| $values = $item->getValue(); | ||
| if (is_array($values) && !empty($values['value'])) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we just use $item->isEmpty()? And is that a scenario that's possible once you get to the formatter?
| if (is_array($values) && !empty($values['value'])) { | ||
|
|
||
| $elements[$delta]['value'] = [ | ||
| '#type' => 'html_tag', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would this be better if $elements['#type'] = 'item_list'; and the values were the #items?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That might mess with other things as I think $elements[$delta] is the expected pattern...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The formatter can say it handles multiple values and then deal with the whole lot as one.
| * | ||
| * Update status log fields. | ||
| */ | ||
| function decoupled_auth_crm_tools_entity_presave(EntityInterface $entity) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this not live in the field \Drupal\Core\Field\FieldItemListInterface::preSave?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I tried all of the fielditemlist hooks... Unless the field actually is set pre save, nothing on the fielditemlist gets triggered.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ContentEntityStorageBase::invokeFieldMethod is invoked for each field on presave, I'm surprised this doesn't work and is worth checking again.
| '#disabled' => $has_data, | ||
| ]; | ||
|
|
||
| // @todo Make this field a select field of existing fields on entity. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we do this, we need to filter the fields by appropriate ones. My suggestion would be ones where there is a main value and the main value is a string... We may want to put some similar validation somewhere else for programmatic fields, but not exactly sure where that should live...
| foreach ($entity->getFieldDefinitions() as $field_name => $definition) { | ||
| /* @var \Drupal\Core\Field\FieldDefinitionInterface $definition */ | ||
| if ($definition->getType() == 'status_log') { | ||
| $source_field = $definition->getFieldStorageDefinition()->getSetting('source_field'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's put all of this logic inside the status log field list class.
| /** | ||
| * {@inheritdoc} | ||
| */ | ||
| public function applyDefaultValue($notify = TRUE) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems redundant.
No description provided.