The DBTNG example needs to be converted to more object orientated code, to be a proper Drupal 8 example.

Things that need to be done:

  • Use a controller to store all the different functions
  • Use form controllers
  • add a .routing.yml file
CommentFileSizeAuthor
#7 examples-convert-dbtng-to-oop-2074243-7.patch38.99 KBrteijeiro
FAILED: [[SimpleTest]]: [MySQL] 129 pass(es), 1 fail(s), and 0 exception(s). View
#5 examples-convert-dbtng-to-oop-2074243-5.patch39.65 KBrteijeiro
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch examples-convert-dbtng-to-oop-2074243-5.patch. Unable to apply patch. See the log in the details link for more information. View
#1 examples-Convert_DBTNG_to_OOP-2074243-1.patch41.57 KBwesleydv
FAILED: [[SimpleTest]]: [MySQL] 147 pass(es), 3 fail(s), and 2 exception(s). View
Members fund testing for the Drupal project. Drupal Association Learn more

Comments

wesleydv’s picture

Status: Needs work » Needs review
FileSize
41.57 KB
FAILED: [[SimpleTest]]: [MySQL] 147 pass(es), 3 fail(s), and 2 exception(s). View

Adding a patch

Status: Needs review » Needs work

The last submitted patch, examples-Convert_DBTNG_to_OOP-2074243-1.patch, failed testing.

Mile23’s picture

Thanks, wesleydv. Sorry about the block_example test fail. :-)

I'm not sure about the pattern of putting database-related methods like load() and delete() on the controller. It's not a bad idea, but it breaks the separation of concerns. It'd be handy if D8 gave us something like Doctrine's repository, but oh well.

That said, I think it's a reasonable move, so we can leave those methods where they are, for the sake of readability.

Just a few other things:

  1. +++ b/dbtng_example/dbtng_example.module
    @@ -23,7 +23,9 @@
    - * The several examples here demonstrate basic database usage.
    + * The several examples in DBTNGExampleController (see
    + * /lib/Drupal/dbtng_example/Controller/DBTNGExampleController.php) demonstrate
    + * basic database usage.
    

    Use @see and/or @link

  2. +++ b/dbtng_example/lib/Drupal/dbtng_example/Tests/DBTNGExampleTest.php
    @@ -1,31 +1,45 @@
      * @file
    - * SimpleTests for dbtng_example module.
    + * Contains DBTNGExampleTest.
    

    Qualify the namespace in the @file docblock.

rteijeiro’s picture

Assigned: wesleydv » rteijeiro

@wesleydv, I will continue with this issue as I also am working in https://drupal.org/node/1868178 if you agree ;)

rteijeiro’s picture

Status: Needs work » Needs review
FileSize
39.65 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch examples-convert-dbtng-to-oop-2074243-5.patch. Unable to apply patch. See the log in the details link for more information. View

Refactored to OOP. Tried to follow the best practices taking ideas from contrib modules :)

marvil07’s picture

Thanks for the new patch!

Uhm, it seems like tests are not passing on upstream code, I'll take a look tonigh in order to tests to be run here.

In the meantime, a little quick review with really trivial stuff.

  1. +++ b/dbtng_example/dbtng_example.module
    @@ -23,7 +23,9 @@
    + * The several examples in DBTNGExampleController (see
    + * /lib/Drupal/dbtng_example/DBTNGExampleController.php) demonstrate
    

    This reference can be fullname(full namespace and class), and docs should convert it in a link IIRC.

  2. +++ b/dbtng_example/dbtng_example.routing.yml
    @@ -0,0 +1,35 @@
    +    _access: 'TRUE'
    +    ¶
    

    Minor space error.

  3. +++ b/dbtng_example/lib/Drupal/dbtng_example/DBTNGExampleController.php
    @@ -0,0 +1,62 @@
    +class DBTNGExampleController {
    +  ¶
    

    Again, minor space error.

  4. +++ b/dbtng_example/lib/Drupal/dbtng_example/Tests/DBTNGExampleTest.php
    @@ -1,31 +1,42 @@
    +  public function setUp() {
    +    parent::setUp();
    +  }
    +
    

    No need to include setUp() if not used.

rteijeiro’s picture

FileSize
38.99 KB
FAILED: [[SimpleTest]]: [MySQL] 129 pass(es), 1 fail(s), and 0 exception(s). View

Fixed minor fixes and rebased with the latest changes in DBTNGExampleTest.php file (maybe done by you :P)

marvil07’s picture

@rteijeiro: Thanks for the new patch, I will be reviewing it in detail as soon as I end up fixing tests in 8.x-1.x (core changes breaking us :-p)

Status: Needs review » Needs work

The last submitted patch, examples-convert-dbtng-to-oop-2074243-7.patch, failed testing.

Mile23’s picture

Linking together with this one: #1868178: (better docs for) Port dbtng_example to D8

Also note that #7 removes a whole lot of lovingly crafted documentation in docblocks. It would be great if we could retain and update as much of that info as possible.

rteijeiro’s picture

Hi Mile23, I guess I keep all the docblock comments. Let me know if I have deleted something but I tried to keep all ;)

Mile23’s picture

Issue summary: View changes
Status: Needs work » Fixed

Since we're having a branch blocker at the testbot level, I've hammered this patch into test-passing.

Found a few references to the old module functions and sorted those out, and also some minor formatting changes.

Not submitting a patch because the testbot will hang anyway, and because you can read it all here:

http://drupalcode.org/project/examples.git/commit/4d79cdc69e179709845f99...

Marking this one closed... Go ahead and open a new issue if you want to continue work on it.

Thanks, folks. :-)

Status: Fixed » Closed (fixed)

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