We need an example module for making Config Entities.

I'm working on something :)

CommentFileSizeAuthor
#76 interdiff.txt51.54 KBMile23
#76 2084301_76.patch33.75 KBMile23
#72 interdiff.txt1.98 KBMile23
#72 2084301_72.patch33.34 KBMile23
#67 interdiff.txt3.83 KBMile23
#67 2084301_67.patch32.97 KBMile23
#63 interdiff.txt359 bytesMile23
#63 2084301_63.patch32.86 KBMile23
#60 interdiff-2084301-58-60.txt1.23 KBsocketwench
#60 2084301_60.patch32.51 KBsocketwench
#58 interdiff-2084301-56-58.txt862 bytessocketwench
#58 2084301_58.patch32.48 KBsocketwench
#56 interdiff-2084301-50-56.txt4.83 KBsocketwench
#56 2084301_56.patch32.56 KBsocketwench
#50 interdiff_46.txt16.26 KBMile23
#50 2084301_50.patch29.99 KBMile23
#46 interdiff_43.txt2.48 KBMile23
#46 2084301-46.examples.config-entity-wip.patch26.29 KBMile23
#43 interdiff-2084301-41-43.txt6.86 KBsocketwench
#43 2084301-43.examples.config-entity-wip.patch25.95 KBsocketwench
#41 interdiff-2084301-36-41.txt6.91 KBsocketwench
#41 2084301-41.examples.config-entity-wip.patch25.8 KBsocketwench
#36 interdiff_34.txt1.44 KBMile23
#36 2084301-36.examples.config-entity-wip.patch22.64 KBMile23
#34 interdiff_26.txt15.23 KBMile23
#34 2084301-34.examples.config-entity-wip.patch21.74 KBMile23
#26 interdiff-2084301-22-26.txt18.47 KBsocketwench
#26 2084301-26.examples.config-entity-wip.patch20.63 KBsocketwench
#23 interdiff-2084301-16-22.txt7.57 KBsocketwench
#22 2084301-22.examples.config-entity-wip.patch19.46 KBsocketwench
#16 2084301-16.examples.config-entity-wip.patch19.24 KBsocketwench
#15 2084301-15.examples.config-entity-wip.patch19.04 KBsocketwench
#12 2084301-12.examples.config-entity-wip.patch17.11 KBsocketwench
#10 2084301-10.examples.config-entity-wip.patch15.05 KBjoachim
#8 2084301-8.examples.config-entity-wip.patch12.88 KBsocketwench
#1 2084301-1.examples.config-entity-wip.patch12.63 KBjoachim
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joachim’s picture

Status: Active » Needs review
FileSize
12.63 KB

Here's my work so far in case anyone wants to take a look :)

It's reasonably complete -- though I realize I've been working with the patch at #2084255: add label to rows in EntityListController on my system, so if that doesn't get in soon we'll have to implement an Entity List Controller here.

Mile23’s picture

Looks like a lot of good stuff, thanks.

I think it might be useful to have an example of EntityListController, whether the base system provides a default one or not.

Needs tests, and a lot of todos in there. And a few things I noticed:

+++ b/config_entity_example/config_entity_example.moduleundefined
@@ -0,0 +1,69 @@
+/**
+ * Loads a robot.
+ *
+ * This is our menu loader, which it used for the '%robot' placeholders in
+ * config_entity_example_menu().
+ *
+ * @param $id
+ *   The ID of the robot to load.
+ *
+ * @return \Drupal\config_entity_example\Entity\Robot|null
+ *   A Robot object or NULL if the requested $id does not exist.
+ */
+function robot_load($id) {
+  return entity_load('robot', $id);
+}

I'm not entirely sure how a config entity is supposed to work, but I'm inclined to assume this should be config_entity_example_robot_load() instead of just robot_load().

+++ b/config_entity_example/lib/Drupal/config_entity_example/Entity/Robot.phpundefined
@@ -0,0 +1,91 @@
+ * credits: originally based on code from blog post at
+ * http://previousnext.com.au/blog/understanding-drupal-8s-config-entities

Should be @see

joachim’s picture

> + * http://previousnext.com.au/blog/understanding-drupal-8s-config-entities

> Should be @see

Not so much... that blog post is pretty out of date now -- I started off with the code from that and made a lot of changes before my module didn't crash! :)

Should it be a @see if it's just to give another author credit?

> I'm not entirely sure how a config entity is supposed to work, but I'm inclined to assume this should be config_entity_example_robot_load() instead of just robot_load().

You mean use '%config_entity_example_robot' as the menu placeholder? I'll look into it.

> I think it might be useful to have an example of EntityListController, whether the base system provides a default one or not.

Yup, that's a good idea.

> Needs tests,

I'll write some.

> and a lot of todos in there.

Some of those are things I just don't know...

Albert Volkman’s picture

I'm not entirely familiar with the subject myself, but aren't upcasts now handled by ParamConverterInterface? Further, I believe if an entity is defined, it automatically gets upcasted via EntityConverter. I may be entirely wrong, of course.

marvil07’s picture

Status: Needs review » Needs work

Changing status based on comments.

Mile23’s picture

Should be @see

My mistake. Not @see but @link. https://drupal.org/coding-standards/docs#link

You mean use '%config_entity_example_robot' as the menu placeholder? I'll look into it.

No, I mean functions in a module should start with the module name, like hooks. That way we avoid namespace collision. Again, I'm not sure how a config entity works, so it might need that function name.

socketwench’s picture

Issue summary: View changes
  1. +++ b/config_entity_example/config_entity_example.module
    @@ -0,0 +1,69 @@
    +/**
    + * Implements hook_menu().
    + */
    +function config_entity_example_menu() {
    +  $items = array();
    

    Wasn't this hook removed? https://drupal.org/node/2184797

  2. +++ b/config_entity_example/config_entity_example.routing.yml
    @@ -0,0 +1,28 @@
    +# The routing.yml file defines the paths for our module.
    

    *.routing.yml can have title keys now.

  3. +++ b/config_entity_example/config_entity_example.routing.yml
    --- /dev/null
    +++ b/config_entity_example/config_entity_example.schema.yml
    
    +++ b/config_entity_example/config_entity_example.schema.yml
    +++ b/config_entity_example/config_entity_example.schema.yml
    @@ -0,0 +1,19 @@
    
    @@ -0,0 +1,19 @@
    +# Schema for the configuration files of the Config Entity Example module.
    +
    +# This schema tells the config system how to read our config YML files.
    +# See for example the file config/config_entity_example.robot.marvin.yml, which
    +# contains our default config entity.
    +
    

    Usually schemas are found in /config/schema

  4. +++ b/config_entity_example/lib/Drupal/config_entity_example/Entity/Robot.php
    @@ -0,0 +1,91 @@
    +  }
    

    Why are we overriding this? I don't really see the point. The function name also changed to url().

socketwench’s picture

Status: Needs work » Needs review
FileSize
12.88 KB

Fixed a number of things. Still more left to fix.

Status: Needs review » Needs work

The last submitted patch, 8: 2084301-8.examples.config-entity-wip.patch, failed testing.

joachim’s picture

Thanks! Here's some more work.

Some things that need attention:

- the default entity provided in the config file isn't showing up any more
- when I enable the module I get translation errors

socketwench’s picture

Assigned: Unassigned » socketwench

Self assigning. Have lots of new changes in the queue. Just not ready for a patch. Hopefully tonight.

socketwench’s picture

Status: Needs work » Needs review
FileSize
17.11 KB

Changes:

  • Split up entity form into base, add, edit, and delete forms.
  • Added a list controller.
  • Modified the access controller based on Tim Plunkett's suggestion.

Still needs more comments to explain why. I can work on those.

tim.plunkett’s picture

Looking good!

  1. +++ b/config_entity_example/config_entity_example.local_actions.yml
    @@ -0,0 +1,5 @@
    +    - robot_list
    \ No newline at end of file
    
    +++ b/config_entity_example/lib/Drupal/config_entity_example/Controller/RobotListController.php
    @@ -0,0 +1,30 @@
    +} ¶
    \ No newline at end of file
    
    +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotAddForm.php
    @@ -0,0 +1,21 @@
    +} ¶
    \ No newline at end of file
    
    +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotDeleteForm.php
    @@ -0,0 +1,40 @@
    +} ¶
    \ No newline at end of file
    
    +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotEditForm.php
    @@ -0,0 +1,21 @@
    +} ¶
    \ No newline at end of file
    

    Fix this up

  2. +++ b/config_entity_example/config_entity_example.module
    @@ -0,0 +1,68 @@
    +/**
    + * Implements hook_menu().
    + */
    +function config_entity_example_menu() {
    +  $items = array();
    +
    +  // Our admin page to list robots.
    +  $items['examples/config_entity_example'] = array(
    +    'title' => 'Config Entity Example',
    +    'route_name' => 'robot_list',
    +  );
    +
    +  // Local action to add a robot.
    +  $items['examples/config_entity_example/add'] = array(
    +    'title' => 'Add robot',
    +    'route_name' => 'robot_add',
    +    'type' => MENU_SIBLING_LOCAL_TASK,
    +    'weight' => 1,
    +  );
    +
    +  $items['examples/config_entity_example/manage/%robot'] = array(
    +    'title' => 'Edit robot',
    +    'route_name' => 'robot_edit',
    +  );
    +  $items['examples/config_entity_example/manage/%robot/edit'] = array(
    +    'title' => 'Edit',
    +    'type' => MENU_DEFAULT_LOCAL_TASK,
    +  );
    +
    +  return $items;
    +}
    ...
    +/**
    + * Loads a robot.
    + *
    + * This is our menu loader, which it used for the '%robot' placeholders in
    + * config_entity_example_menu().
    + *
    + * @param $id
    + *   The ID of the robot to load.
    + *
    + * @return \Drupal\config_entity_example\Entity\Robot|null
    + *   A Robot object or NULL if the requested $id does not exist.
    + */
    +function robot_load($id) {
    +  return entity_load('robot', $id);
    +}
    

    Can delete all of this now!

  3. +++ b/config_entity_example/config_entity_example.routing.yml
    @@ -0,0 +1,40 @@
    +    _access: 'TRUE'
    

    _entity_create_access: robot

  4. +++ b/config_entity_example/config_entity_example.routing.yml
    @@ -0,0 +1,40 @@
    +    # This (TODO! does it?) uses our entity access controller.
    

    Yes it does!

  5. +++ b/config_entity_example/config_entity_example.routing.yml
    @@ -0,0 +1,40 @@
    +    _access: 'TRUE'
    

    Should be _entity_access: robot.delete

  6. +++ b/config_entity_example/lib/Drupal/config_entity_example/Controller/RobotListController.php
    @@ -0,0 +1,30 @@
    +/**
    + * Created by PhpStorm.
    + * User: tess
    + * Date: 3/15/14
    + * Time: 6:06 PM
    + */
    
    +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotAddForm.php
    @@ -0,0 +1,21 @@
    +/**
    + * Created by PhpStorm.
    + * User: tess
    + * Date: 3/15/14
    + * Time: 12:38 PM
    + */
    
    +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotDeleteForm.php
    @@ -0,0 +1,40 @@
    +/**
    + * Created by PhpStorm.
    + * User: tess
    + * Date: 3/15/14
    + * Time: 12:41 PM
    + */
    
    +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotEditForm.php
    @@ -0,0 +1,21 @@
    +/**
    + * Created by PhpStorm.
    + * User: tess
    + * Date: 3/15/14
    + * Time: 12:40 PM
    + */
    

    Remove this :)

  7. +++ b/config_entity_example/lib/Drupal/config_entity_example/Controller/RobotListController.php
    @@ -0,0 +1,30 @@
    +class RobotListController extends ConfigEntityListController {
    ...
    +  public function buildHeader() {
    ...
    +  public function buildRow(EntityInterface $entity) {
    

    Docblocks!

  8. +++ b/config_entity_example/lib/Drupal/config_entity_example/Entity/Robot.php
    @@ -0,0 +1,102 @@
    +  /**
    +   * {@inheritdoc}
    +   *
    +   * This defines the URI for our entities.
    +   */
    +  public function uri() {
    +    return array(
    +      // TODO docs.
    +      'path' => 'examples/config_entity_example/manage/' . $this->id(),
    +      'options' => array(),
    +    );
    +  }
    

    No need for this

  9. +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotAddForm.php
    @@ -0,0 +1,21 @@
    +class RobotAddForm extends RobotFormBase {
    ...
    +  protected function actions(array $form, array &$form_state) {
    
    +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotDeleteForm.php
    @@ -0,0 +1,40 @@
    +class RobotDeleteForm extends EntityConfirmFormBase {
    ...
    +  public function getQuestion() {
    ...
    +  public function getConfirmText() {
    ...
    +  public function getCancelRoute() {
    ...
    +  public function submit(array $form, array &$form_state) {
    
    +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotEditForm.php
    @@ -0,0 +1,21 @@
    +class RobotEditForm extends RobotFormBase {
    ...
    +  protected function actions(array $form, array &$form_state) {
    
    +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotFormBase.php
    @@ -0,0 +1,85 @@
    +   * Overrides Drupal\Core\Entity\EntityFormController::form().
    +   */
    +  public function buildForm(array $form, array &$form_state) {
    ...
    +  protected function actions(array $form, array &$form_state) {
    ...
    +   * Overrides Drupal\Core\Entity\EntityFormController::validate().
    

    docblocks! Use {@inheritdoc} where possible

  10. +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotFormBase.php
    @@ -0,0 +1,85 @@
    + * Definition of Drupal\config_entity_example\RobotFormController.
    

    Contains \Drupal\...

Mile23’s picture

As far as tim.plunkett's stuff: Awesome and thanks for the review.

Just a quibble on item #2: We don't need hook_menu(), but we do need hook_menu_link_defaults() to provide some in-site explanation of what's going on, and links to the new/view/edit/delete pages so newbies can find their way around.

Thanks to socketwrench and joachim for so much effort! :-)

  1. +++ b/config_entity_example/config_entity_example.local_actions.yml
    @@ -0,0 +1,5 @@
    \ No newline at end of file
    

    Needs newline... Also in a few other places.

  2. +++ b/config_entity_example/config_entity_example.module
    @@ -0,0 +1,68 @@
    + * @defgroup config_entity_example Example: Config Entity
    + * @ingroup examples
    + * @{
    + * Example of how to define a configuration entity type.
    + *
    + * This example is part of the Examples for Developers Project
    + * which you can download and experiment with at
    + * http://drupal.org/project/examples
    + *
    + * Start at the Robot class, which defines our entity type.
    

    @see namespace\Robot

    Also, at some point we have to explain what a config entity is, and how it's different from a non-config entity or whatever else. I think this explanation should be in the @defgroup block.

  3. +++ b/config_entity_example/lib/Drupal/config_entity_example/Controller/RobotListController.php
    @@ -0,0 +1,30 @@
    + * Created by PhpStorm.
    

    Thanks, PhpStorm! Now switch to @file. :-)

  4. +++ b/config_entity_example/lib/Drupal/config_entity_example/Entity/Robot.php
    @@ -0,0 +1,102 @@
    + * TODO: add a @see to the annotation docs, assuming these exist!!!
    + *  - id: The machine name of the entity type.
    + *  - label: The human-readable label of the entity type.
    + *    TODO: explain @Translation.
    + *  - module: The name of the module that provides this.
    + *    TODO: is this necessary?
    

    Even for properties where we have good defaults, we want to set them in annotation with documentation to explain what they are, and which ones can be safely omitted.

  5. +++ b/config_entity_example/lib/Drupal/config_entity_example/Entity/Robot.php
    @@ -0,0 +1,102 @@
    + *   entity_keys = {
    + *     "id" = "id",
    + *     "label" = "label"
    + *   },
    ...
    +  /**
    +   * The robot ID.
    +   *
    +   * @var string
    +   */
    +  public $id;
    

    In the property docblock, explain the mapping in the annotation. Even as simple as "Maps to id for this entity through annotation." It might be illustrative to use a different name for the class property so the mapping is visible.

socketwench’s picture

Fixed issues in #14, a few others I found, added lots and lots of comments.

socketwench’s picture

Added @file docblocks.

Mile23’s picture

Component: Other » Entity Example
Status: Needs review » Needs work

It's shaping up.. Thanks.

  1. +++ b/config_entity_example/config/config_entity_example.robot.marvin.yml
    @@ -0,0 +1,12 @@
    +# The id of the config entity.
    +id: marvin
    

    I don't see Marvin in my entity list right off the bat. Is that the expected behavior?

  2. --- /dev/null
    +++ b/config_entity_example/config_entity_example.api.php
    

    What's supposed to be here?

  3. +++ b/config_entity_example/config_entity_example.info.yml
    @@ -0,0 +1,7 @@
    +files:
    +  - tests/config_entity_example.test
    

    I bet Drupal can't find this test. :-)

    Also, this module needs tests...

  4. +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotFormBase.php
    @@ -0,0 +1,133 @@
    +  public function validate(array $form, array &$form_state) {
    +    parent::validate($form, $form_state);
    +
    +    // Add code here to validate your config entity's form elements.
    +    // Nothing to do here.
    +  }
    

    Would super-duper love to see some more complex validation behavior, so people can learn how it works. Maybe some arbitrary string length or a numeric value that has to be validated.

joachim’s picture

> I don't see Marvin in my entity list right off the bat. Is that the expected behavior?

No, it's not. It's a bug which I don't know how to fix.

> +++ b/config_entity_example/config_entity_example.api.php

Nope, that should go.

> Would super-duper love to see some more complex validation behavior, so people can learn how it works. Maybe some arbitrary string length or a numeric value that has to be validated.

Is there anything we could do that is specific to config entities? If not, then isn't that best left to the Form API example? We could write a comment there pointing the reader to that.

Mile23’s picture

OK, we do a @see form api or whatever example that ends up being.

Berdir’s picture

  1. +++ b/config_entity_example/config_entity_example.api.php
    @@ -0,0 +1,6 @@
    +
    +/**
    + * @addtogroup hooks
    + * @{
    

    No need except you want to define the default hoosk (load, insert, update, delete, presave, ...) which are automatically called for each entity type, which would probably make sense...

  2. +++ b/config_entity_example/config_entity_example.local_actions.yml
    @@ -0,0 +1,5 @@
    +config_entity_example_add_action:
    
    +++ b/config_entity_example/config_entity_example.routing.yml
    @@ -0,0 +1,39 @@
    +robot_list:
    

    Machine names for routes, local tasks and actions should be prefixed with "modulename.", instead of "modulename_".

  3. +++ b/config_entity_example/config_entity_example.routing.yml
    @@ -0,0 +1,39 @@
    +  requirements:
    +    # We're not setting any access control on our admin page for simplicity.
    +    _access: 'TRUE'
    

    No need for simplicity, you have an admin permission, use it...

  4. +++ b/config_entity_example/lib/Drupal/config_entity_example/Controller/RobotListController.php
    @@ -0,0 +1,27 @@
    + * @file
    + * Contains RobotListController.
    

    Should have the full namespace.

  5. +++ b/config_entity_example/lib/Drupal/config_entity_example/Entity/Robot.php
    @@ -0,0 +1,102 @@
    + *   module = "config_entity_example",
    

    Not necessary.

  6. +++ b/config_entity_example/lib/Drupal/config_entity_example/Entity/Robot.php
    @@ -0,0 +1,102 @@
    + *   config_prefix = "config_entity_example.robot",
    

    The config prefix is now optionally and forces the module name, so it would just be "robot", which is now the default, so you can leave it out.

    That's very likely why the default content entity isn't found.

  7. +++ b/config_entity_example/lib/Drupal/config_entity_example/Entity/Robot.php
    @@ -0,0 +1,102 @@
    +
    +  /**
    +   * {@inheritdoc}
    +   *
    +   * This defines the URI for our entities.
    +   */
    +  public function uri() {
    +    return array(
    +      // TODO docs.
    +      'path' => 'examples/config_entity_example/manage/' . $this->id(),
    +      'options' => array(),
    +    );
    

    Remove this, works all based on the links/routes now.

  8. +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotFormBase.php
    @@ -0,0 +1,133 @@
    +
    +    // Redirect the user to the following path after the save optionation.
    +    $form_state['redirect'] = 'examples/config_entity_example';
    

    Should use route_redirect with a Url object now.

  9. +++ b/config_entity_example/lib/Drupal/config_entity_example/RobotAccessController.php
    @@ -0,0 +1,40 @@
    +/**
    + * Defines an access controller for the robot entity.
    + *
    + * The access controller determines access to Robot entities.
    + *
    + * @see \Drupal\config_entity_example\Entity\Robot.
    + */
    +class RobotAccessController extends EntityAccessController {
    

    There is no need for an access controller if you have an admin_permission. If you want to specify one as an example if access is not that trivial, you should explicitly document that this is only necessary when not defining an admin_permissions.

  10. +++ b/config_entity_example/lib/Drupal/config_entity_example/RobotAccessController.php
    @@ -0,0 +1,40 @@
    +    if ($operation == 'view') {
    +      return TRUE;
    +    }
    +    else {
    +      // Other than that, we're going to be insanely lax about access.
    +      // Don't try this at home!
    +      return parent::checkAccess($entity, $operation, $langcode, $account);;
    

    Nothing lax about it, this will use the specified admin_permission.

    checking for view is a bit pointless as this entity doesn't define any view route/ability to display it, which I would leave out, block is the only special case that does use this.

    also double ;;.

    Suggestion: If you want to have an access controller as example, add a include some special thing like not allowing to default your default config entity (you can do so based on the id or by introducing a locked property similar to other config entities. And then describe that for everything else, you use the default implementation which will check admin_permission.

Mile23’s picture

Basically every entity API related thing Berdir said just above would be great to include in docblocks and inline comments. :-)

socketwench’s picture

Status: Needs work » Needs review
FileSize
19.46 KB

Fixed the issues in #20.

I left the access controller in for now since I'm unsure if we should keep it or not. If we do keep it, what should it check?

socketwench’s picture

FileSize
7.57 KB

Interdiff. Hopefully I did that right.

jibran’s picture

  1. +++ b/config_entity_example/lib/Drupal/config_entity_example/Controller/RobotListController.php
    @@ -0,0 +1,44 @@
    +   * @param EntityInterface $entity
    
    +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotAddForm.php
    @@ -0,0 +1,36 @@
    + *
    

    @param desc missing.

  2. +++ b/config_entity_example/lib/Drupal/config_entity_example/Controller/RobotListController.php
    @@ -0,0 +1,44 @@
    +   * @return array
    
    +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotAddForm.php
    @@ -0,0 +1,36 @@
    +   * @return array
    
    +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotDeleteForm.php
    @@ -0,0 +1,77 @@
    +   * @return string
    ...
    +   * @return string
    ...
    +   * @return array
    ...
    +   * @return \Drupal\Core\Entity\EntityInterface|void
    
    +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotEditForm.php
    @@ -0,0 +1,36 @@
    +   * @return array
    
    +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotFormBase.php
    @@ -0,0 +1,134 @@
    +   * @return array
    ...
    +   * @return array
    

    Missing blank line before @return and desc.

  3. +++ b/config_entity_example/lib/Drupal/config_entity_example/Controller/RobotListController.php
    @@ -0,0 +1,44 @@
    +    $row['label'] = $this->getLabel($entity);
    

    Why not $entity->label();?

  4. +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotAddForm.php
    @@ -0,0 +1,36 @@
    +use Drupal\config_entity_example\Form\RobotFormBase;
    
    +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotEditForm.php
    @@ -0,0 +1,36 @@
    +use Drupal\config_entity_example\Form\RobotFormBase;
    

    Not needed.

  5. +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotAddForm.php
    @@ -0,0 +1,36 @@
    +   * @param array $form
    +   * @param array $form_state
    
    +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotDeleteForm.php
    @@ -0,0 +1,77 @@
    +   * @param array $form
    +   * @param array $form_state
    
    +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotEditForm.php
    @@ -0,0 +1,36 @@
    +   * @param array $form
    +   * @param array $form_state
    
    +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotFormBase.php
    @@ -0,0 +1,134 @@
    +   * @param array $form
    +   * @param array $form_state
    ...
    +   * @param array $form
    +   * @param array $form_state
    ...
    +   * @param array $form
    +   * @param array $form_state
    

    desc missing.

  6. +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotDeleteForm.php
    @@ -0,0 +1,77 @@
    +   * The question is used a a title in our confirm form. For delete confirm forms,
    ...
    +   * Provides the route name to go to if the user cancels the action. For entity
    +   * delete forms, this is typically the route that points at the list controller.
    
    +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotFormBase.php
    @@ -0,0 +1,134 @@
    +    // Drupal provides the entity to us as a class variable. If this is an existing
    ...
    +    // this is a new entity, it will be a new object with the class of our entity.
    ...
    +   * the form values. Do not override submit() as save() is the preferred method
    ...
    +    // form field was saved as a public variable in the entity class. PHP allows
    

    more then 80 chars.

  7. +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotEditForm.php
    @@ -0,0 +1,36 @@
    + * Contains RobotEditForm.
    

    Use full namespace.

  8. +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotFormBase.php
    @@ -0,0 +1,134 @@
    +   * Overrides Drupal\Core\Entity\EntityFormController::save().
    

    missing @param block

  9. +++ b/config_entity_example/lib/Drupal/config_entity_example/RobotAccessController.php
    @@ -0,0 +1,40 @@
    +      return TRUE;
    

    Shouldn't it be static::ALLOWED?

tim.plunkett’s picture

Addressing #3, because that's what all list controllers do, shortcut for checkPlain

socketwench’s picture

As for #9: The base class seems to only specify TRUE or FALSE. https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Entity%21...

Fixed all other issues except #3. Still needs tests...

jibran’s picture

Issue tags: +PHPUnit, +Needs tests

Thanks for fixing the issues. I think only @todo and tests are remaining and missing so tagging accordingly. Here is some more minor issues and suggestions.

  1. +++ b/config_entity_example/config_entity_example.module
    @@ -0,0 +1,24 @@
    \ No newline at end of file
    

    :(

  2. +++ b/config_entity_example/config_entity_example.routing.yml
    @@ -0,0 +1,38 @@
    +    # TODO: explain by what magic '_entity_list' turns into using the entity list controller.
    

    it is a todo but still 80 char limit applies ;)

  3. +++ b/config_entity_example/lib/Drupal/config_entity_example/Entity/Robot.php
    @@ -0,0 +1,87 @@
    + * TODO: add a @see to the annotation docs, assuming these exist!!!
    

    block module has some docs.

  4. +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotDeleteForm.php
    @@ -0,0 +1,85 @@
    +   * The question is used a a title in our confirm form. For delete confirm
    +   * forms, this typically takes the form of "Are you sure you want to
    +   * delete...", including the entity label.
    ...
    +   * The confirm text is used as a the text in the button that confirms the
    +   * question posed by getQuestion().
    ...
    +   * Provides the route name to go to if the user cancels the action. For entity
    +   * delete forms, this is typically the route that points at the list
    +   * controller.
    ...
    +   * The submit handler for the confirm form. For entity delete forms, you use
    +   * this to delete the entity in $this->entity.
    

    Single line desc of function is missing. Then we can add explanation within 80 chars limit.

  5. +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotFormBase.php
    @@ -0,0 +1,149 @@
    +   * Build the entity add/edit form.
    

    Builds

  6. +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotFormBase.php
    @@ -0,0 +1,149 @@
    +   * the form values. Do not override submit() as save() is the preferred method
    ...
    +    // form field was saved as a public variable in the entity class. PHP allows
    

    More then 80 chars.

  7. +++ b/config_entity_example/lib/Drupal/config_entity_example/RobotAccessController.php
    @@ -0,0 +1,40 @@
    +      // Don't try this at home!
    

    lol

joachim’s picture

> + # TODO: explain by what magic '_entity_list' turns into using the entity list controller.

My most recent patch had that part rewritten:

+++ b/config_entity_example/config_entity_example.routing.yml
@@ -0,0 +1,38 @@
+    _title: 'Config Entity Example'
+    # This tells the routing system to use the entity list controller that our
+    # entity type defined for this route.
+    # @see \Drupal\Core\Entity\Enhancer\EntityRouteEnhancer
+    _entity_list: 'robot'

@socketwench: could you check you got changes I made in that please?

Berdir’s picture

You removed hook_menu() now, which is correct, but you do want a hook_menu_link_defaults() for your menu link to the list page.

You need to look at the checkAccess() method, you're already relying on it or it wouldn't work: https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Entity%21...

As mentioned above, I think a nice example would be to introduce a locked column and set that to true for your default config, then deny deletion in the UI for that. That's for example what contact.module does for the special personal category.

tim.plunkett’s picture

Issue tags: -PHPUnit, -Needs tests

Not sure why tests are needed here.
This is to show how to write a ConfigEntity, not how to write PHPUnit tests.
And it wouldn't be possible until #2134857: PHPUnit test the entity base classes is in anyway.

Mile23’s picture

Issue tags: +Needs tests

Needs tests because it's code and it has to be maintainable.

The rules for Examples with testing is that we're demonstrating APIs, so the demonstration has to work. If you have classes that aren't presentational, those have to be unit tested. For instance, an entity class. :-) However, the entity class in this case is just some properties so no biggie.

Here's the module checklist: #2209627: [meta] Module Checklist for Examples

  1. +++ b/config_entity_example/config_entity_example.info.yml
    @@ -0,0 +1,7 @@
    +files:
    +  - tests/config_entity_example.test
    

    Don't need this in .info.yml file.

  2. +++ b/config_entity_example/config_entity_example.local_actions.yml
    @@ -0,0 +1,5 @@
    +config_entity_example_add_action:
    

    Give at least a one-liner comment describing this local action and why it exists.

  3. +++ b/config_entity_example/config_entity_example.module
    @@ -0,0 +1,24 @@
    +
    

    Needs @defgroup, which is where there should be a large chunk of documentation which describes the API we're showing off, why you'd want to use it, why you wouldn't want to use it, and where to look for more information.

    Examples is documentation, written in docblocks, parsed by API module in api.drupal.org.

    Here's an example of the kind of things we need: https://api.drupal.org/api/examples/field_permission_example%21field_per...

  4. +++ b/config_entity_example/lib/Drupal/config_entity_example/Controller/RobotListController.php
    @@ -0,0 +1,46 @@
    + * Class RobotListController
    

    @ingroup config_entity_example

  5. +++ b/config_entity_example/lib/Drupal/config_entity_example/Entity/Robot.php
    @@ -0,0 +1,87 @@
    + * http://previousnext.com.au/blog/understanding-drupal-8s-config-entities
    

    @link

  6. +++ b/config_entity_example/lib/Drupal/config_entity_example/Entity/Robot.php
    @@ -0,0 +1,87 @@
    + * Defines the robot entity.
    

    Describe what the entity models, why it has the properties it has, what the expectations for it are, why it's a config entity and not something else.

    Explain what the various properties do, what they map to, why they exist.

  7. +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotAddForm.php
    @@ -0,0 +1,37 @@
    + * Class RobotAddForm
    

    @ingroup config_entity_example

  8. +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotDeleteForm.php
    @@ -0,0 +1,85 @@
    + * Class RobotDeleteForm
    

    @ingroup config_entity_example

  9. +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotEditForm.php
    @@ -0,0 +1,37 @@
    + * Class RobotEditForm
    

    @ingroup config_entity_example

  10. +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotFormBase.php
    @@ -0,0 +1,149 @@
    + * Class RobotFormBase
    

    @ingroup config_entity_example

  11. +++ b/config_entity_example/lib/Drupal/config_entity_example/RobotAccessController.php
    @@ -0,0 +1,40 @@
    + * Defines an access controller for the robot entity.
    

    @ingroup config_entity_example

Berdir’s picture

I agree with needing tests but I think unit tests are useless here. We're not interested in testing the implementation of those classes, what we want to test is the integration with core, and making sure that it still works when core changes. So what we need is a functional test that verifies they can be created and listed in the UI. And maybe a DUBT test with basic CRUD stuff like create, save, update.

Mile23’s picture

Berdir: That's why I didn't add back the phpunit tag.

See the Examples module checklist: #2209627: [meta] Module Checklist for Examples

Mile23’s picture

All the changes above, with some coding standards stuff.

Still needs functional tests, expanded newbie documentation, hook_menu_default_links().

Also: Is there a way to specify a blob of text at top of an entity list controller? I want to put "This is a list of the entity we created.. You can do the following with it..." Does ConfigEntityListController have a method I can't find, or a special trick? Or does it need to be hook_help()?

tim.plunkett’s picture

public function render() {
  $build['some_text'] = array(
    '#markup' => $this->t('Here is some text'),
  );
  $build += parent::render();
  return $build;
}
Mile23’s picture

Thanks, tim.plunkett.

Added some descriptive text to the entity form.

Berdir’s picture

  1. +++ b/config_entity_example/config_entity_example.local_actions.yml
    @@ -1,5 +1,15 @@
    +# Add some local task links to facilitate navigation.
    +
     config_entity_example_add_action:
       route_name: robot.add
    

    See below, only "create new thing" should be a local action, so this should just say that it adds a local action to create a new robot to the list overview or so.

  2. +++ b/config_entity_example/config_entity_example.local_actions.yml
    @@ -1,5 +1,15 @@
    +
    +config_entity_example_list_action:
    +  route_name: robot.list
    +  title: 'List Robots'
    +  appears_on:
    +    - robot.add
    +    - robot.edit
    +    - robot.delete
    

    This is not action. If anything, then list/edit/delete should be local tasks.

socketwench’s picture

Assigned: socketwench » Unassigned

I started working on the group documentation when I noticed there were other updates. Here's what I got so far:

<?php
/**
 * @defgroup config_entity_example: Configuration Entity Example
 * @ingroup config_entity_example
 * @{
 * Example defining a Configuration Entity
 *
 * This is an example of a simple configuration entity, the kind you might
 * create to store administrator-defined objects like blocks or views.
 *
 * In this module we define a configuration entity named 'Robot'. The entity
 * has a unique ID (also called a machine name), a human-readable label used
 * for display, and a universally unique identifier. You can create new robots
 * by navigating to &lt;your_site_root&gt;/examples/config_entity_example. A
 * default robot, "marvin", is included with the module.
 *
 * What's special about a Configuration Entity?
 *
 * Configuration entities are entities just like content entities. The key
 * difference is where the data is stored. Content entities are stored in the
 * database. Configuration entities are stored in *.yml files, typically under
 * &lt;your_site_root&gt;/sites/default/files/config_&lt;unique_id&gt;.
 *
 * Another key difference with configuration entities is the expectation they
 * are created by administrators, and not end users. As files, configuration
 * entities can be added to a version control system.
 */
?>
Mile23’s picture

Sorry, socketwench. Please reassign yourself if you'd like.

Mile23’s picture

socketwench’s picture

Remerged @joachim's changes. Added several more comments.

jibran’s picture

Awesome we have know a lot of documentation. Thank you @Mile23 and @socketwench.

  1. +++ b/config_entity_example/lib/Drupal/config_entity_example/Controller/RobotListController.php
    @@ -0,0 +1,93 @@
    +   * Add some descriptive text to our entity list.
    

    Adds

  2. +++ b/config_entity_example/lib/Drupal/config_entity_example/Controller/RobotListController.php
    @@ -0,0 +1,93 @@
    +   * however, if you want to add markup before or after the table. ¶
    

    white space.

  3. +++ b/config_entity_example/lib/Drupal/config_entity_example/Controller/RobotListController.php
    @@ -0,0 +1,93 @@
    +        . " in your Drupal site.</p><p>By default, when you enable this module,"
    

    I think intention here is to keep it to 80 char so this more then 80 chars.

  4. +++ b/config_entity_example/lib/Drupal/config_entity_example/Entity/Robot.php
    @@ -0,0 +1,88 @@
    + *    TODO: explain @Translation.
    ...
    + *    TODO: is this necessary?
    ...
    + *  - entity_keys: TODO.
    

    Highlighting all the remaining @todo just for a self reminder for next review.

  5. +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotAddForm.php
    @@ -0,0 +1,40 @@
    + * Class RobotAddForm
    
    +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotDeleteForm.php
    @@ -0,0 +1,97 @@
    + * Class RobotDeleteForm
    

    Full stop missing.

  6. +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotDeleteForm.php
    @@ -0,0 +1,97 @@
    +   * Gather a confirmation question.
    ...
    +   * Gather the confirmation text.
    

    Gathers

  7. +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotDeleteForm.php
    @@ -0,0 +1,97 @@
    +   * The question is used a a title in our confirm form. For delete confirm
    

    as a

  8. +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotDeleteForm.php
    @@ -0,0 +1,97 @@
    +   * Get the cancel route.
    

    Gets

  9. +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotEditForm.php
    @@ -0,0 +1,40 @@
    + * Class RobotEditForm
    
    +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotFormBase.php
    @@ -0,0 +1,152 @@
    + * Class RobotFormBase
    

    Full stop missing.

  10. +++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotFormBase.php
    @@ -0,0 +1,152 @@
    +   * the form values. Do not override submit() as save() is the preferred method
    ...
    +    // form field was saved as a public variable in the entity class. PHP allows
    

    More then 80 chars.

socketwench’s picture

Fixes as per #42, went through and completed TODOs.

Status: Needs review » Needs work

The last submitted patch, 43: 2084301-43.examples.config-entity-wip.patch, failed testing.

jibran’s picture

Thanks for the fixes. Great work fixing all the @todos.

+++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotDeleteForm.php
@@ -25,9 +25,9 @@
+   * The question is used aa a title in our confirm form. For delete confirm

I fine with aa :D

Mile23’s picture

Status: Needs work » Needs review
FileSize
26.29 KB
2.48 KB

Added hook_menu_link_defaults() and fixed #45.

Needs tests at this point, for the following stuff:

  • Verify that Marvin was added.
  • Verify permissions for managing entities (perms = 200, no perms = 403)
  • Drive around each of the management pages through the UI.

Anyone want to assign themselves? :-)

Status: Needs review » Needs work

The last submitted patch, 46: 2084301-46.examples.config-entity-wip.patch, failed testing.

Mile23’s picture

Mile23’s picture

Assigned: Unassigned » Mile23

OK. Here I go with the tests.

Mile23’s picture

Status: Needs work » Needs review
FileSize
29.99 KB
16.26 KB

Added functional tests until I got an error that seemed very very strange, and probably has nothing to do with this project....

call_user_func() expects parameter 1 to be a valid callback, function 'contact_category_load' not found or invalid function name

Which is very strange because we're not doing anything with the contact module.

Also some adjustments for https://drupal.org/node/2200867

This test will fail, but might pass later.

Status: Needs review » Needs work

The last submitted patch, 50: 2084301_50.patch, failed testing.

tim.plunkett’s picture

+++ b/config_entity_example/lib/Drupal/config_entity_example/Form/RobotFormBase.php
@@ -0,0 +1,153 @@
+        'exists' => 'contact_category_load',

See DateFormatFormBase for a better example of how this should be done.

Mile23’s picture

Thanks, tim.plunkett, but you'll have to be more specific... DateFormatFormBase doesn't implement buildForm().

tim.plunkett’s picture

In this patch, you copy/pasted contact_category_load. You should use a method on the class.

From DateFormatFormBase::form()

    $form['id'] = array(
      '#type' => 'machine_name',
      '#description' => t('A unique machine-readable name. Can only contain lowercase letters, numbers, and underscores.'),
      '#disabled' => !$this->entity->isNew(),
      '#default_value' => $this->entity->id(),
      '#machine_name' => array(
        'exists' => array($this, 'exists'),
        'replace_pattern' =>'([^a-z0-9_]+)|(^custom$)',
        'error' => 'The machine-readable name must be unique, and can only contain lowercase letters, numbers, and underscores. Additionally, it can not be the reserved word "custom".',
      ),
    );

The important part is the 'exists' => array($this, 'exists'),

Also in DateFormatFormBase

  /**
   * Checks for an existing date format.
   *
   * @param string|int $entity_id
   *   The entity ID.
   * @param array $element
   *   The form element.
   * @param array $form_state
   *   The form state.
   *
   * @return bool
   *   TRUE if this format already exists, FALSE otherwise.
   */
  public function exists($entity_id, array $element,  array $form_state) {
    return (bool) $this->dateFormatStorage
      ->getQuery()
      ->condition('id', $element['#field_prefix'] . $entity_id)
      ->execute();
  }
Mile23’s picture

Assigned: Mile23 » Unassigned

Thanks, Tim.

Unassigning myself because I was just writing tests...

Anyone who wants to take it up can go ahead. I might get back to this tomorrow if I can.

socketwench’s picture

Status: Needs work » Needs review
FileSize
32.56 KB
4.83 KB

Added exists code based on #54.

Status: Needs review » Needs work

The last submitted patch, 56: 2084301_56.patch, failed testing.

socketwench’s picture

Status: Needs work » Needs review
FileSize
32.48 KB
862 bytes

Fixed the error in the test. Posting here since I still haven't figured out why I can't run D8 tests locally...

Status: Needs review » Needs work

The last submitted patch, 58: 2084301_58.patch, failed testing.

socketwench’s picture

Status: Needs work » Needs review
FileSize
32.51 KB
1.23 KB

Fixed the default entity yml due to https://drupal.org/node/2234799

Status: Needs review » Needs work

The last submitted patch, 60: 2084301_60.patch, failed testing.

socketwench’s picture

60: 2084301_60.patch queued for re-testing.

Re-testing due to https://drupal.org/node/2262989

Mile23’s picture

Status: Needs work » Needs review
FileSize
32.86 KB
359 bytes

Needs the menu_links.yml file makeover...

Edit: Ug. Forgot to delete the old hook_menu().

joachim’s picture

> * - entity_keys: Specifies the class variable(s) in which unique keys are
* stored for this entity type.

Could this be phrased a bit more clearly? At the moment I don't understand it at all.

- I think for something that's visibly a list, using '(s)' is unnecessary. It's usually always the case that you can have a list of one thing, so we don't need to say that.
- 'class variable(s)' - is this what used to be called 'entity properties'?
- 'unique keys' - is the label unique? It's a bit confusing.

What entity_keys seems to do is tell the entity system which object property to look in for standard properties it wants to find on an entity. So "label" = "wibble" would mean the entity system would look in $robot->wibble for the label. Maybe we should deliberately change these so it's easier to see how they work?

joachim’s picture

config_entity_example.robot.*:

 * @ConfigEntityType(
 *   id = "robot",

Is the string 'robot' what ties these together?

When the system reads the file config/config_entity_example.robot.marvin.yml, how does it know to use Robot.php for the resulting object?

tim.plunkett’s picture

#65, yes that's what that is.

https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Config%21...
that's the function that finds out what entity type to use, and the entity type definition says which class has this ID in it. That part is just standard plugin behavior (mapping an ID to a class).

Mile23’s picture

FileSize
32.97 KB
3.83 KB

How does that look? Check out the interdiff to see what I did.

joachim’s picture

+++ b/config_entity_example/config/schema/config_entity_example.schema.yml
@@ -0,0 +1,19 @@
+  mapping:
+    id:
+      type: string
+      label: 'Robot id'
+    uuid:
+      type: string
+      label: 'UUID'
+    label:
+      type: label
+      label: 'Label'

Could we give a link to documentation about the format of this file, and what the different types are? Though all I've found is https://drupal.org/node/1905070, and I find it utterly impenetrable :/

joachim’s picture

Also, some questions I have that could do to be answered in the comments:

- how do you make Drupal notice a new entity type?
- how do you make Drupal notice a new default config entity file, or changes to it?

Berdir’s picture

Neither of those really belong here I think? Entity types are discovered like everything else, by clearing the cache. And default config entities are like all default config, when the module is installed.

joachim’s picture

Okay so links to docs to find that out? Because I've no idea where those are.

Mile23’s picture

FileSize
33.34 KB
1.98 KB

Added some info to the yml files to try and explain some of the things joachim mentioned.

joachim’s picture

Could we add a custom property to the Robot entity, say, 'floopiness'?

I'm trying to do that in my own entity and the values aren't getting picked up from default config.

Mile23’s picture

72: 2084301_72.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 72: 2084301_72.patch, failed testing.

Mile23’s picture

Status: Needs work » Needs review
FileSize
33.75 KB
51.54 KB

Re-roll for PSR-4, updated getCancelRoute() for https://drupal.org/node/2189619, added Floopy flag with test. :-)

  • Commit 03d7d89 on 8.x-1.x authored by joachim, committed by Mile23:
    Issue #2084301 by socketwench, Mile23, joachim: Added a Config Entity...
Mile23’s picture

Status: Needs review » Fixed

Committed! http://cgit.drupalcode.org/examples/commit/?h=8.x-1.x&id=03d7d89a6f46ea5...

Make a new issue for new stuff.

Thanks, joachim, socketwench, and everyone else!

Status: Fixed » Closed (fixed)

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