Part of #2006152: [meta] Don't call theme() directly anywhere outside drupal_render().

To test this code

  1. Visit the taxonomy administration page at (admin/structure/taxonomy)
  2. add a few terms to the tags vocabulary
  3. re-order the tags so one is indented (by dragging one under and to the right of another)
  4. confirm that the indentation shows as expected
Files: 
CommentFileSizeAuthor
#78 replace-theme-with-render-2009680-78.patch760 bytesEric_A
FAILED: [[SimpleTest]]: [MySQL] 57,031 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#76 replace-theme-with-render-2009680-76.patch657 bytesEric_A
FAILED: [[SimpleTest]]: [MySQL] 57,094 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#72 replace-theme-with-render-test-only-2009680-72.patch2.02 KBDarthDrupal
FAILED: [[SimpleTest]]: [MySQL] 56,739 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#72 replace-theme-with-render-2009680-72.patch2.98 KBDarthDrupal
PASSED: [[SimpleTest]]: [MySQL] 56,954 pass(es).
[ View ]
#72 interdiff-2009680-69-72.txt950 bytesDarthDrupal
#71 taxonomy-drupal-render-tests-only-71.patch2.02 KBhussainweb
FAILED: [[SimpleTest]]: [MySQL] 56,785 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#71 taxonomy-drupal-render-71.patch2.98 KBhussainweb
PASSED: [[SimpleTest]]: [MySQL] 56,584 pass(es).
[ View ]
#69 replace-theme-with-render-2009680-69.patch2.98 KBDarthDrupal
PASSED: [[SimpleTest]]: [MySQL] 56,428 pass(es).
[ View ]
#67 replace-theme-with-render-2009680-67.patch2.99 KBDarthDrupal
PASSED: [[SimpleTest]]: [MySQL] 56,628 pass(es).
[ View ]
#65 replace-theme-with-render-2009680-65.patch2.99 KBDarthDrupal
PASSED: [[SimpleTest]]: [MySQL] 56,426 pass(es).
[ View ]
#64 replace-theme-with-render-2009680-64.patch2.99 KBDarthDrupal
FAILED: [[SimpleTest]]: [MySQL] 56,435 pass(es), 2 fail(s), and 0 exception(s).
[ View ]
#62 replace-theme-with-render-with-empty-func-2009680-62.patch3.02 KBDarthDrupal
PASSED: [[SimpleTest]]: [MySQL] 56,434 pass(es).
[ View ]
#62 replace-theme-with-render-with-isset-func-2009680-62.patch2.99 KBDarthDrupal
PASSED: [[SimpleTest]]: [MySQL] 56,299 pass(es).
[ View ]
#57 replace-theme-with-render-2009680-57.patch2.75 KBDarthDrupal
PASSED: [[SimpleTest]]: [MySQL] 56,760 pass(es).
[ View ]
#55 replace-theme-with-render-2009680-55.patch2.75 KBDarthDrupal
PASSED: [[SimpleTest]]: [MySQL] 58,145 pass(es).
[ View ]
#50 twig-2009680-45.patch2.87 KBadamcowboy
PASSED: [[SimpleTest]]: [MySQL] 58,072 pass(es).
[ View ]
#46 replace-theme-with-render-2009680-46.patch2.73 KBDarthDrupal
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/taxonomy/lib/Drupal/taxonomy/Tests/TaxonomyTermsIndentation.php.
[ View ]
#43 replace-theme-with-render-2009680-43.patch2.63 KBDarthDrupal
PASSED: [[SimpleTest]]: [MySQL] 56,825 pass(es).
[ View ]
#42 replace-theme-with-render-2009680-42.patch2.58 KBDarthDrupal
FAILED: [[SimpleTest]]: [MySQL] 56,758 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#39 replace-theme-with-render-2009680-39.patch2.92 KBDarthDrupal
PASSED: [[SimpleTest]]: [MySQL] 58,039 pass(es).
[ View ]
#35 TaxonomyTermsIndentation.test1.6 KBDarthDrupal
#32 taxonomy_depth_broken.png12.32 KBalexpott
#32 taxonomy_depth_fixed-3.png12.32 KBalexpott
#28 replace-theme-with-render-2009680-28.patch987 bytesDarthDrupal
PASSED: [[SimpleTest]]: [MySQL] 58,499 pass(es).
[ View ]
#27 replace-theme-with-render-2009680-27.patch957 bytesDarthDrupal
PASSED: [[SimpleTest]]: [MySQL] 58,327 pass(es).
[ View ]
#24 replace-theme-with-render-2009680-24.patch978 bytescwells73
FAILED: [[SimpleTest]]: [MySQL] 56,432 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#22 replace-theme-with-render-2009680-21.patch977 bytescwells73
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/taxonomy/taxonomy.admin.inc.
[ View ]
#21 replace-theme-with-render-2009680-21.patch977 bytescwells73
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/taxonomy/taxonomy.admin.inc.
[ View ]
#19 replace-theme-with-render-2009680-19.patch974 bytescwells73
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/taxonomy/taxonomy.admin.inc.
[ View ]
#17 replace-theme-with-render-2009680-17.patch974 bytescwells73
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/taxonomy/taxonomy.admin.inc.
[ View ]
#15 replace-theme-with-render-2009680-15.patch973 bytescwells73
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/taxonomy/taxonomy.admin.inc.
[ View ]
#7 replace-theme-with-render-2009680-7.patch976 bytesSamvel
PASSED: [[SimpleTest]]: [MySQL] 56,353 pass(es).
[ View ]
#2 replace-theme-with-render-2009680-2.patch908 bytesSamvel
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
#3 replace-theme-with-render-2009680-3.patch909 bytesSamvel
FAILED: [[SimpleTest]]: [MySQL] 56,532 pass(es), 0 fail(s), and 63 exception(s).
[ View ]

Comments

Assigned:Unassigned» Samvel

StatusFileSize
new908 bytes
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

attached

p.s. Don't use this one, see #3

Status:Active» Needs review
StatusFileSize
new909 bytes
FAILED: [[SimpleTest]]: [MySQL] 56,532 pass(es), 0 fail(s), and 63 exception(s).
[ View ]

ups, small mistake in #2

new attached

Issue tags:+CodeSprintUA

+ CodeSprintUA

Status:Needs review» Reviewed & tested by the community

simple replacement
looks good for me
RTBC if bot green

Status:Reviewed & tested by the community» Needs work

The last submitted patch, replace-theme-with-render-2009680-3.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new976 bytes
PASSED: [[SimpleTest]]: [MySQL] 56,353 pass(es).
[ View ]

attached new one

Status:Needs review» Needs work
Issue tags:-Novice, -CodeSprintUA

The last submitted patch, replace-theme-with-render-2009680-7.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+Novice, +CodeSprintUA

EDIT tests are passes locally, seems bot troubles

#7: replace-theme-with-render-2009680-7.patch queued for re-testing.

Status:Needs review» Reviewed & tested by the community

RTBC

Status:Reviewed & tested by the community» Needs work

+++ b/core/modules/taxonomy/taxonomy.admin.incundefined
@@ -241,8 +241,12 @@ function taxonomy_overview_terms($form, &$form_state, Vocabulary $vocabulary) {
+    $indentation =  array('#theme' => 'indentation');
+    if (isset($term->depth->value)) {
+      $indentation['#size'] = $term->depth->value;
+    }

should be

<?php
if (isset($term->depth->value) && $term->depth->value > 0) {
 
$indentation = array(
   
'#theme' => 'indentation',
   
'#size' = $term->depth->value,
  );
}
?>

+++ b/core/modules/taxonomy/taxonomy.admin.incundefined
@@ -241,8 +241,12 @@ function taxonomy_overview_terms($form, &$form_state, Vocabulary $vocabulary) {
-      '#prefix' => isset($term->depth->value) && $term->depth->value > 0 ? theme('indentation', array('size' => $term->depth->value)) : '',
+      '#prefix' => isset($term->depth->value) && $term->depth->value > 0 ? drupal_render($indentation) : '',
       '#type' => 'link',
       '#title' => $term->label(),
       '#href' => "taxonomy/term/" . $term->id(),

then simply

<?php
'#prefix' => isset($indentation) ? drupal_render($indentation) : '',
?>

<?php
$indention
= array();
if () {
 
// as #11
}
then + (empty($indetion))
?>

because of loop

Assigned:Samvel» Unassigned

this has been sitting idle for a while, feel free to re-assign.

Assigned:Unassigned» cwells73

I'll take this.. getting my feet wet here. This seems straight forward enough. New patch forthcoming.

Status:Needs work» Needs review
StatusFileSize
new973 bytes
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/taxonomy/taxonomy.admin.inc.
[ View ]

Patch attached.. I hope this is correct.

Status:Needs review» Needs work

The last submitted patch, replace-theme-with-render-2009680-15.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new974 bytes
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/taxonomy/taxonomy.admin.inc.
[ View ]

Lets try this again.. added missing '('

Status:Needs review» Needs work

The last submitted patch, replace-theme-with-render-2009680-17.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new974 bytes
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/taxonomy/taxonomy.admin.inc.
[ View ]

I'll get this to pass if it kills me!

Status:Needs review» Needs work

The last submitted patch, replace-theme-with-render-2009680-19.patch, failed testing.

StatusFileSize
new977 bytes
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/taxonomy/taxonomy.admin.inc.
[ View ]

This is getting a little.. silly.. sorry..

forgot to set to 'needs review', grrrrr

Status:Needs work» Needs review
StatusFileSize
new977 bytes
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/taxonomy/taxonomy.admin.inc.
[ View ]

trying again

Status:Needs review» Needs work

The last submitted patch, replace-theme-with-render-2009680-21.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new978 bytes
FAILED: [[SimpleTest]]: [MySQL] 56,432 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

this version looks good on my end... again, sorry for all the posts.

hope this patch is what's expected.

Status:Needs review» Needs work

The last submitted patch, replace-theme-with-render-2009680-24.patch, failed testing.

Assigned:cwells73» Unassigned

someone else can look at this.. not sure what the fail message:

One-time link is no longer valid. Other UserPasswordResetTest.php 81 Drupal\user\Tests\UserPasswordResetTest->testUserPasswordReset()

has anything to do with my changes.

Status:Needs work» Needs review
StatusFileSize
new957 bytes
PASSED: [[SimpleTest]]: [MySQL] 58,327 pass(es).
[ View ]

I'm trying to making some debug and I noticed that $term->depth already has the size of the indentation and $term->depth->value obviously gives this error:

Trying to get property of non-object in taxonomy_overview_terms()

Do you think that $term->depth will become an object in the next releases?
I fixed it for the moment. Patch attached.

StatusFileSize
new987 bytes
PASSED: [[SimpleTest]]: [MySQL] 58,499 pass(es).
[ View ]

Ops sorry I didn't notice indications at #12.
Re-attached patch.

Status:Needs review» Reviewed & tested by the community

Tested the patch and it seemed to work! I added a few terms to the tags vocabulary and every thing seemed normal. After that I re-order the tags so one is indented and confirmed that the indentation showed up as expected.

Tested the patch and it seemed to work! I added a few terms to the tags vocabulary and every thing seemed normal. After that I re-order the tags so one is indented and confirmed that the indentation showed up as expected.

Category:task» bug
Status:Reviewed & tested by the community» Needs work
Issue tags:+Needs tests
StatusFileSize
new12.32 KB
new12.32 KB

This is actually fixing a bug... therefore I think we need to add a test for this...

To test...

  1. Create a vocabulary
  2. Create some terms
  3. Drag one of the terms to be "parented" another term
  4. Press save

Current result

taxonomy_depth_broken.png

With this patch

taxonomy_depth_fixed-3.png

Assigned:Unassigned» DarthDrupal

Ok, I'll try!

Status:Needs work» Needs review
StatusFileSize
new1.6 KB

I wrote the test! It's the first time I write a functional test so sorry if I made mistakes :)
I read the other tests in the taxonomy module folder (core/modules/taxonomy/lib/Drupal/taxonomy/Tests) to have some directions about the right sintax.
I hope I did it in the right way.

In the test I:

- create a vocabulary
- create three terms
- create the $edit array to simulate that the second term has been dragged under the first
- call the drupalPost method passing in the $edit array and clicking the submit button with label 'Save'
- call an assertion to check if in the raw html the indentation div element is present ( )

In my local drupal installation the test passed correctly.
Let me know if I did something wrong so I can improve my "test approach" :D

Status:Needs review» Needs work

@DarthDrupal, you need to roll the tests into the patch rather than provide them in a separate file, otherwise the testbots can't run them.

@thedavidmeister I looked for some documentation about that but I didn't find anything.
Can you give me some hint on how to roll the patch and the test together?

Sorry but it's the first time for me :)

Apply the patch in #28, add your tests, stage all of your changes in git together with the changes from the patch.

Now if you do $ git diff --cached you should see the patch and your changes in the diff.

$ git diff --cached > 2009680-39.patch will write it into a patch file you can upload here.

Status:Needs work» Needs review
StatusFileSize
new2.92 KB
PASSED: [[SimpleTest]]: [MySQL] 58,039 pass(es).
[ View ]

Thanks for the hint!
Patch + Test attached!

Issue tags:-Needs tests, -CodeSprintUA

Awesome work, removing tags :)

Status:Needs review» Needs work

+      //'terms[tid:1:0][term][tid]' => 1,
+      //'terms[tid:1:0][term][parent]' => '',
+      //'terms[tid:1:0][term][depth]' => 0,
+      //'terms[tid:1:0][weight]' => 0,
+      'terms[tid:' . $term2->id() . ':0][term][tid]' => 2,
+      'terms[tid:' . $term2->id() . ':0][term][parent]' => 1,
+      'terms[tid:' . $term2->id() . ':0][term][depth]' => 1,
+      'terms[tid:' . $term2->id() . ':0][weight]' => 1,
+      //'terms[tid:3:0][term][tid]' => 3,
+      //'terms[tid:3:0][term][parent]' => 2,
+      //'terms[tid:3:0][term][depth]' => 2,
+      //'terms[tid:3:0][weight]' => 2,

Make sure to remove your debug code when you put up a patch for review.

Status:Needs work» Needs review
StatusFileSize
new2.58 KB
FAILED: [[SimpleTest]]: [MySQL] 56,758 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Ops sorry, comments removed!

Btw I'm not sure the test ran in #39...
If I click on View Details I don't see my test listed in the Drupal\taxonomy\Tests\ section..
Is that normal?

StatusFileSize
new2.63 KB
PASSED: [[SimpleTest]]: [MySQL] 56,825 pass(es).
[ View ]

Test class name typing error in #42.
Fixed and attached again

Status:Needs review» Needs work

Minor code style tweaks needed:

+++ b/core/modules/taxonomy/lib/Drupal/taxonomy/Tests/TaxonomyTermsIndentation.phpundefined
@@ -0,0 +1,38 @@
+  }
+  public function setUp() {

blank line needed between methods

+++ b/core/modules/taxonomy/lib/Drupal/taxonomy/Tests/TaxonomyTermsIndentation.phpundefined
@@ -0,0 +1,38 @@
+    $this->vocabulary = $this->createVocabulary();
+  }
+  function testTermIndentation() {

blank line needed between methods, and method description also needed

Dibs.

Status:Needs work» Needs review
StatusFileSize
new2.73 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/taxonomy/lib/Drupal/taxonomy/Tests/TaxonomyTermsIndentation.php.
[ View ]

Fixed code style issues.

Status:Needs review» Needs work
Issue tags:-Novice

The last submitted patch, replace-theme-with-render-2009680-46.patch, failed testing.

Status:Needs work» Needs review

Status:Needs review» Needs work
Issue tags:+Novice

The last submitted patch, replace-theme-with-render-2009680-46.patch, failed testing.

StatusFileSize
new2.87 KB
PASSED: [[SimpleTest]]: [MySQL] 58,072 pass(es).
[ View ]

I'll give it a go.

Status:Needs work» Needs review

Assigned:DarthDrupal» azinoman

Good job Adam, this code is as smooth as a baby's bottom

Status:Needs review» Reviewed & tested by the community

Assigned:azinoman» DarthDrupal
Status:Reviewed & tested by the community» Needs review
StatusFileSize
new2.75 KB
PASSED: [[SimpleTest]]: [MySQL] 58,145 pass(es).
[ View ]

testTermIndentation() comment fixed and removed changes to default.settings.php file.

Status:Needs review» Needs work

+ * Creating and posting three terms.

trailing whitespace.

Status:Needs work» Needs review
StatusFileSize
new2.75 KB
PASSED: [[SimpleTest]]: [MySQL] 56,760 pass(es).
[ View ]

Fixed #56.

Status:Needs review» Reviewed & tested by the community

This is looking good to me now.

Status:Reviewed & tested by the community» Needs work

This is looking very good overall, here are some nitpicks :)

  1. +++ b/core/modules/taxonomy/lib/Drupal/taxonomy/Tests/TaxonomyTermsIndentation.phpundefined
    @@ -0,0 +1,44 @@
    +<?php
    +/**
    + * Testing functionality of terms indentation in
    + * admin/structure/taxonomy/manage/tags
    + */
    +namespace Drupal\taxonomy\Tests;

    Needs an @file docblock per http://drupal.org/node/1354#file and there should be a blank line before and after the docblock.

  2. +++ b/core/modules/taxonomy/lib/Drupal/taxonomy/Tests/TaxonomyTermsIndentation.phpundefined
    @@ -0,0 +1,44 @@
    +class TaxonomyTermsIndentation extends TaxonomyTestBase {
    +  public static $modules = array('taxonomy');

    Needs an empty line between the class definition and the $modules property definition.

    The $modules property needs a docblock now as well, see other tests for an example.

  3. +++ b/core/modules/taxonomy/lib/Drupal/taxonomy/Tests/TaxonomyTermsIndentation.phpundefined
    @@ -0,0 +1,44 @@
    +  /**
    +   * Creating and posting three terms.
    +   * Then checking to see if a div with class indentation exists.
    +   */
    +  function testTermIndentation() {

    Please see http://drupal.org/node/1354#functions, this should probably have a summary line like "Tests term indentation." Then it could have another paragraph below with the rest of the text, or you could use inline comments to describe the different steps of the test.

  4. +++ b/core/modules/taxonomy/lib/Drupal/taxonomy/Tests/TaxonomyTermsIndentation.phpundefined
    @@ -0,0 +1,44 @@
    +  }
    +}

    There should be a blank line between the last function and the end of the class.

  5. +++ b/core/modules/taxonomy/taxonomy.admin.incundefined
    @@ -159,8 +159,15 @@ function taxonomy_overview_terms($form, &$form_state, Vocabulary $vocabulary) {
    +    $indentation = array();
    ...
    +      '#prefix' => !empty($indentation) ? drupal_render($indentation) : '',

    If we check !empty($indentation) before rendering, why do we set $indentation to an empty array?

…and the test class name should end with Test, i.e. TaxonomyTermsIndentationTest.

Edit: and the class definition needs a docblock :)

I'm working on it

Status:Needs work» Needs review
StatusFileSize
new2.99 KB
PASSED: [[SimpleTest]]: [MySQL] 56,299 pass(es).
[ View ]
new3.02 KB
PASSED: [[SimpleTest]]: [MySQL] 56,434 pass(es).
[ View ]

Ok, added all the missing docblocks and blank lines.

Regarding the fifth point in #59 I followed andypost indications at #12.

However I think that could be enough using the isset() function instead of empty() here

+      '#prefix' => !empty($indentation) ? drupal_render($indentation) : '',

I attach two different version of the same patch, one using empty() and the $indentation array initialization and the other with just the isset() function instead of empty() so you can decide what is better.

Status:Needs review» Needs work

Thanks @DarthDrupal!

I missed the loop :) that makes sense then, I would go with the first patch.

+++ b/core/modules/taxonomy/lib/Drupal/taxonomy/Tests/TaxonomyTermsIndentationTest.phpundefined
@@ -0,0 +1,61 @@
+ * Definition of Drupal\taxonomy\Tests\TaxonomyTermsIndentationTest.

This should be Contains \Drupal… per https://drupal.org/node/1354#file.

+++ b/core/modules/taxonomy/lib/Drupal/taxonomy/Tests/TaxonomyTermsIndentationTest.phpundefined
@@ -0,0 +1,61 @@
+/**
+ * Testing functionality of terms indentation in
+ * admin/structure/taxonomy/manage/tags.
+ */
+class TaxonomyTermsIndentationTest extends TaxonomyTestBase {

This is a summary line so should fit on one line of 80 characters or less per http://drupal.org/node/1354#drupal.

+++ b/core/modules/taxonomy/lib/Drupal/taxonomy/Tests/TaxonomyTermsIndentationTest.phpundefined
@@ -0,0 +1,61 @@
+    // Submit the edited form and check for html indentation element presence.

s/html/HTML/

Status:Needs work» Needs review
StatusFileSize
new2.99 KB
FAILED: [[SimpleTest]]: [MySQL] 56,435 pass(es), 2 fail(s), and 0 exception(s).
[ View ]

Ok let's go further with the original patch (I missed the loop too :) ).
Fixed as indicated in #63.

StatusFileSize
new2.99 KB
PASSED: [[SimpleTest]]: [MySQL] 56,426 pass(es).
[ View ]

Ops fixed extra full stop in class comment block :)

Looking much much better, thanks for sticking with this @DarthDrupal!

+++ b/core/modules/taxonomy/lib/Drupal/taxonomy/Tests/TaxonomyTermsIndentationTest.phpundefined
@@ -0,0 +1,60 @@
+ * Testing terms indentation functionality in terms list page.
...
+      'description' => 'Ensure that the terms indentation works properly',
...
+   * Test terms indentation.

"term indentation functionality", "term list page", and "terms indentation" - all should be singular I think.

And I think that's the end of my nitpicks on this issue :)

StatusFileSize
new2.99 KB
PASSED: [[SimpleTest]]: [MySQL] 56,628 pass(es).
[ View ]

Ok, singularized the three sentences :D

I keep on forgetting to mention but interdiffs are great, really helpful especially in cases like this :)

+++ b/core/modules/taxonomy/lib/Drupal/taxonomy/Tests/TaxonomyTermsIndentationTest.phpundefined
@@ -0,0 +1,60 @@
+ * Testing term indentation functionality in terms list page.
...
+      'name' => 'Taxonomy Terms Indentation',

Missed a couple here. Also... test names are not usually Title Cased like that. Maybe just "Taxonomy term indentation"?

StatusFileSize
new2.98 KB
PASSED: [[SimpleTest]]: [MySQL] 56,428 pass(es).
[ View ]

I agree, fixed test name.
I singularized the filename and the class name too.

PS: thanks for all your hints it's my first patch and my first test so every counsel is good for me :D

Status:Needs review» Needs work

Keep up the great work @DarthDrupal!

I brought this down locally and ran the test without the fix and it failed as expected. Generally we post test-only patches on the issue though, see the bottom of https://drupal.org/contributor-tasks/write-tests for an example. So I would recommend posting a test-only and combined patch on this issue, generally that is done when the test is added to the issue or when changes are made to the test. We want to make sure that the test works properly and would expose any regressions in the future.

…and I said I was done but I found two more tiny things to update. Then I will happily RTBC the patch :)

+++ b/core/modules/taxonomy/lib/Drupal/taxonomy/Tests/TaxonomyTermIndentationTest.phpundefined
@@ -0,0 +1,60 @@
+      'description' => 'Ensure that the term indentation works properly',

'description' should end in a period per https://drupal.org/node/325974.

+++ b/core/modules/taxonomy/lib/Drupal/taxonomy/Tests/TaxonomyTermIndentationTest.phpundefined
@@ -0,0 +1,60 @@
+   * Test term indentation.

Should be 'Tests' per http://drupal.org/node/1354#functions.

Status:Needs work» Needs review
StatusFileSize
new2.98 KB
PASSED: [[SimpleTest]]: [MySQL] 56,584 pass(es).
[ View ]
new2.02 KB
FAILED: [[SimpleTest]]: [MySQL] 56,785 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

I apologize for jumping in, but I really wanted to try this out too. BTW, I have made changes as per #70.

StatusFileSize
new950 bytes
new2.98 KB
PASSED: [[SimpleTest]]: [MySQL] 56,954 pass(es).
[ View ]
new2.02 KB
FAILED: [[SimpleTest]]: [MySQL] 56,739 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Patch in #71 seems good.
Just added the issue number in the patch file name and the interdiff file (thanks to Cottser for teaching me ;) ).

Status:Needs review» Reviewed & tested by the community

Looks good to me :)

Status:Reviewed & tested by the community» Fixed

Committed/pushed to 8.x, thanks!

Status:Fixed» Needs review
StatusFileSize
new657 bytes
FAILED: [[SimpleTest]]: [MySQL] 57,094 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Sorry, but this reverted typed data conversions for no real reason. It works now but won't soon, so it needs to be fixed anyways and the fix is trivial.

The revert was done in #27 as a fix, but the real fix was done later when the new local variable $indentation was initialized every loop. From then on the term depth objects again were only accessed when set.

Status:Needs review» Needs work

The last submitted patch, replace-theme-with-render-2009680-76.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new760 bytes
FAILED: [[SimpleTest]]: [MySQL] 57,031 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Status:Needs review» Needs work

The last submitted patch, replace-theme-with-render-2009680-78.patch, failed testing.

Status:Needs work» Fixed

Sorry for the noise, you guys did a very nice coding job. The only thing is that the issue summary (and the issue itself) is all about the conversion and very little about the bug fix.

Yes Eric_A maybe we didn't focus on the bug enough, sorry :)

however I wanna thank all of you guys who taught me how to approach with patch submission and writing functional tests.

Thank you all!

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

Issue summary:View changes

to test