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

Refactor membership module to better support plugins. Update namespac… #15

Open
wants to merge 3 commits into
base: 8.x-1.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

This file was deleted.

This file was deleted.

21 changes: 17 additions & 4 deletions src/Entity/Membership.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
use Drupal\entity\Revision\RevisionableContentEntityBase;
use Drupal\membership\EventDispatcherTrait;
use Drupal\membership\Exception\MembershipFeatureNotImplementedException;
use Drupal\membership\MembershipEvent;
use Drupal\membership\MembershipEvents;
use Drupal\membership\Event\MembershipEvent;
use Drupal\membership\Event\MembershipEvents;
use Drupal\user\UserInterface;

/**
Expand Down Expand Up @@ -148,9 +148,10 @@ public static function baseFieldDefinitions(EntityTypeInterface $entity_type) {
->setDescription(t('The Membership type/bundle.'))
->setSetting('target_type', 'membership_type')
->setRequired(TRUE);

$fields['user_id'] = BaseFieldDefinition::create('entity_reference')
->setLabel(t('Authored by'))
->setDescription(t('The owner of the Membership entity.'))
->setLabel(t('Primary user'))
Copy link
Member

Choose a reason for hiding this comment

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

"Primary" implies some precedence of multiple users... how about just "User"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, will change.

We do plan to support multiple users per membership, that is a requirement for several of our clients. So this will essentially be the author who can "change own" and add/remove users to a membership if more than one is allowed. We will want the API to support this, without requiring it...

->setDescription(t('The owner of the Membership.'))
->setRevisionable(TRUE)
->setSetting('target_type', 'user')
->setSetting('handler', 'default')
Expand All @@ -173,6 +174,7 @@ public static function baseFieldDefinitions(EntityTypeInterface $entity_type) {
))
->setDisplayConfigurable('form', TRUE)
->setDisplayConfigurable('view', TRUE);

$fields['data'] = BaseFieldDefinition::create('map')
->setLabel(t('Data'))
->setReadOnly(TRUE)
Expand Down Expand Up @@ -200,6 +202,7 @@ public static function baseFieldDefinitions(EntityTypeInterface $entity_type) {
->setDisplayConfigurable('view', TRUE)
->setRevisionable(TRUE)
->setSetting('workflow_callback', ['\Drupal\membership\Entity\Membership', 'getWorkflowId']);

$fields['provider'] = BaseFieldDefinition::create('membership_provider_id')
->setLabel('Provider plugin/remote ID')
->setDisplayConfigurable('form', false)
Expand All @@ -225,6 +228,7 @@ public function preSave(EntityStorageInterface $storage) {
if ($this->isExpired()) {
$this->getEventDispatcher()->dispatch(MembershipEvents::EXPIRE, $event);
}
$this->setRevisionLogMessage($this->t('State changed: '.$this->state->getValue()));
}
parent::preSave($storage);
}
Expand All @@ -248,6 +252,15 @@ public function postCreate(EntityStorageInterface $storage) {
parent::postCreate($storage);
}

/**
* @inheritDoc
*/
function postSave(EntityStorageInterface $storage, $update = TRUE) {
$event = new MembershipEvent($this);
$this->getEventDispatcher()->dispatch(MembershipEvents::UPDATED, $event);
parent::postSave($storage, $update);

}
Copy link
Member

Choose a reason for hiding this comment

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

Do we really need to throw our own event here? State Machine module emits an event on state changes, and core already has hook_entity_insert() and friends, that while not a Symfony event provides similar functionality. Not opposed to an event per se but if it basically duplicates the hook that might not be worth the extra overhead here.

In the same vein I think we can back out the STATE_CHANGE and EXPIRE invocations in preSave() given that state machine does the workflow transition event. That was a late addition in state machine so I'm thinking that code predates those events being thrown for us.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok. Still haven't gotten to the state change events yet -- probably will be in there today or tomorrow -- so I haven't seen how state_machine handles this.

I was trying to avoid hook_entity_insert() and hook_entity_update() and go with a more OO approach, and this looked like the best way...

What's your sense of preference going forward -- symfony events vs hooks? I would prefer events, just because that's a pattern I use on a bunch of projects (and never particularly liked hooks). But I can live with a hook here if they aren't going to be deprecated... I was surprised to not find any decorator or observers related to the Entity Manager to supersede these hooks...


/**
* @inheritDoc
Expand Down
57 changes: 22 additions & 35 deletions src/Entity/MembershipTermInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,32 +15,39 @@ interface MembershipTermInterface extends ContentEntityInterface, EntityChanged

Copy link
Member

Choose a reason for hiding this comment

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

This interface belongs in the membership_term submodule namespace.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah can move that...

// Add get/set methods for your configuration properties here.


/**
* Gets the Membership term type.
* Gets the membership for a term
*
* @return string
* The Membership term type.
* @return Membership
* The membership entity
*/
public function getType();
public function getMembership();

/**
* Gets the Membership term name.
* Set a membership on a term.
*
* @return string
* Name of the Membership term.
* @param \Drupal\membership\Entity\MembershipInterface $membership
*
* @return \Drupal\membership\Entity\MembershipTermInterface
* The called Membership term.
*/
public function getName();

public function setMembership(MembershipInterface $membership);
/**
* Sets the Membership term name.
* Gets the membership id for a term
*
* @param string $name
* The Membership term name.
* @return integer
* The membership entity_id
*/
public function getMembershipId();

/**
* Gets the Membership term type.
*
* @return \Drupal\membership\Entity\MembershipTermInterface
* The called Membership term entity.
* @return string
* The Membership term type.
*/
public function setName($name);
public function getType();

/**
* Gets the Membership term creation timestamp.
Expand All @@ -61,26 +68,6 @@ public function getCreatedTime();
*/
public function setCreatedTime($timestamp);

/**
* Returns the Membership term published status indicator.
*
* Unpublished Membership term are only visible to restricted users.
*
* @return bool
* TRUE if the Membership term is published.
*/
public function isPublished();

/**
* Sets the published status of a Membership term.
*
* @param bool $published
* TRUE to set this Membership term to published, FALSE to set it to unpublished.
*
* @return \Drupal\membership\Entity\MembershipTermInterface
* The called Membership term entity.
*/
public function setPublished($published);

/**
* Returns the Membership Type (bundle) that this membership term is associated with.
Expand Down
9 changes: 9 additions & 0 deletions src/Entity/MembershipTermTypeInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,15 @@ public function setWorkflowId($workflow_id);
*/
public function getMembershipType();

/**
* @return string
Copy link
Member

Choose a reason for hiding this comment

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

Same as above; belongs in membership_term namespace.

Copy link
Member Author

Choose a reason for hiding this comment

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

ok

*/
public function getTermLength();

/**
* @return string
*/
public function getGracePeriod();
/**
* @param string $membership_type
* @return string
Expand Down
21 changes: 21 additions & 0 deletions src/Entity/MembershipType.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,13 @@ class MembershipType extends ConfigEntityBase implements MembershipTypeInterface
*/
protected $workflow;

/**
* The processor plugin string ID.
*
* @var string
*/
protected $processor_plugin_id;
Copy link
Member

Choose a reason for hiding this comment

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

What's with the "processor plugin"? I'm not sure what this is?

Copy link
Member Author

Choose a reason for hiding this comment

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

This was/is an instance of \Drupal\membership\Plugin\MembershipProviderInterface. I was thinking it would be useful to have some membership types control the plugin for their membership instances.

However, after getting something actually working, I reached the conclusion you were correct, it needs to be on the Membership instance, not on MembershipType. Will remove.


/**
* {@inheritdoc}
*/
Expand All @@ -81,6 +88,20 @@ public function setWorkflowId($workflow_id) {
return $this;
}

/**
* @inheritdoc
*/
public function getProcessorPlugin() {
// TODO: Implement getProcessorPlugin() method.
}

/**
* @inheritdoc
*/
public function setProcessorPluginId($processor_plugin_id) {
// TODO: Implement setProcessorPluginId() method.
}

/**
* @inheritDoc
*
Expand Down
18 changes: 17 additions & 1 deletion src/Entity/MembershipTypeInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace Drupal\membership\Entity;

use Drupal\entity\Entity\RevisionableEntityBundleInterface;
use Drupal\membership\Plugin\MembershipProviderInterface;

/**
* Provides an interface for defining Membership type entities.
Expand All @@ -24,8 +25,23 @@ public function getWorkflowId();

/**
* @param string $workflow_id
* @return mixed
* @return MembershipTypeInterface This object
*/
public function setWorkflowId($workflow_id);

/**
* Get the provider plugin for this membership type.
*
* @return MembershipProviderInterface
*/
public function getProcessorPlugin();

/**
* Set the processor plugin type and instance id.
*
* @param $processor_plugin_id string
* @return MembershipTypeInterface This object
*/
public function setProcessorPluginId($processor_plugin_id);

}
2 changes: 1 addition & 1 deletion src/MembershipEvent.php → src/Event/MembershipEvent.php
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?php

namespace Drupal\membership;
namespace Drupal\membership\Event;

use Drupal\membership\Entity\MembershipInterface;
use Symfony\Component\EventDispatcher\Event;
Expand Down
6 changes: 5 additions & 1 deletion src/MembershipEvents.php → src/Event/MembershipEvents.php
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?php

namespace Drupal\membership;
namespace Drupal\membership\Event;

/**
* Class MembershipEvents
Expand All @@ -21,6 +21,10 @@ final class MembershipEvents {
*/
const CREATED = 'membership.create';

/**
* Membership is updated.
*/
const UPDATED = 'membership.updated';
/**
* Membership state changes.
*/
Expand Down
3 changes: 3 additions & 0 deletions src/Form/MembershipForm.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ public function buildForm(array $form, FormStateInterface $form_state) {
/* @var $entity \Drupal\membership\Entity\Membership */
$form = parent::buildForm($form, $form_state);

// Hide revision message and revision fields...
$form['revision_information']['#access'] = FALSE;
$form['revision_log_message']['#access'] = FALSE;
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason to hide these? I see your logic for adding a default message on save, however I think there's a use case for revision notes by the admin?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi,

There's certainly a use case for admins leaving comments, should probably change this to a permission. It's quite confusing with these fields visible, though, and for some reason it's showing up twice.

For now was just hiding to make this usable.

return $form;
}

Expand Down
5 changes: 2 additions & 3 deletions src/Form/MembershipTypeForm.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ public function form(array $form, FormStateInterface $form_state) {
$workflow_manager = \Drupal::service('plugin.manager.workflow');
$workflows = $workflow_manager->getGroupedLabels('membership');

/** @var MembershipTypeInterface $entity */
$entity = $this->entity;
/** @var MembershipTypeInterface $membership_type */
$membership_type = $this->entity;
$form['label'] = array(
'#type' => 'textfield',
Expand All @@ -46,7 +45,7 @@ public function form(array $form, FormStateInterface $form_state) {
'#type' => 'select',
'#title' => t('Workflow'),
'#options' => $workflows,
'#default_value' => $entity->getWorkflowId(),
'#default_value' => $membership_type->getWorkflowId(),
'#description' => $this->t('Used by all memberships of this type.'),
'#required' => true,
];
Expand Down
30 changes: 29 additions & 1 deletion src/Plugin/Field/FieldType/ProviderIdFieldItemList.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,36 @@
use Drupal\Core\Field\FieldItemList;

/**
* Defines the 'membership_provider_id' field item list class.
* Defines the 'membership_remote_id' field item list class.
*/
class ProviderIdFieldItemList extends FieldItemList {

/**
* {@inheritdoc}
*/
public function getByProvider($provider) {
foreach ($this->list as $delta => $item) {
if ($item->provider == $provider) {
return $item->remote_id;
}
}
return NULL;
}

/**
* {@inheritdoc}
*/
public function setByProvider($provider, $remote_id) {
$target_item = NULL;
foreach ($this->list as $delta => $item) {
if ($item->provider == $provider) {
$target_item = $item;
break;
}
}
$target_item = $target_item ?: $this->appendItem();
$target_item->provider = $provider;
$target_item->remote_id = $remote_id;
}

}