Typo in the machine name for Container field in ForumController::addForum

This would actually impact functionality

Patch added

Patch applied cleanly and fixed the typo. Moving to RTBC

We still need a test here

Hello @larowlan, please suggest the test that need to perform.

Visit the add page, and make sure the field value is set correctly

Forum creation and containers seems to be working fine after patch.

RTBC +1

I tried to write something like:

public function testAddForum() {
    $this->drupalLogin($this->adminUser);
    $this->drupalGet('/admin/structure/forum/add/forum');
    $this->submitForm(['name[0][value]' => 'foo'],'Save');
    $term = \Drupal::entityTypeManager()->getStorage('taxonomy_term')->load(1);
    $forum_container_value = $term->forum_container->value;
    $this->assertEquals('0', $forum_container_value, 'The key forum_container exists.');
  }

But even if the key is forum_controller, the test passes. So not sure how to do it.
And not sure how to verify it directly on the add page as the field is not rendered.

I think the issue is that the default value is zero, so the change here makes no difference

I think if the tests add additional coverage, we should add them.

But I think we should remove the line altogether, it doesn't matter that it says 'forum_controller', because the default of zero is still applied for 'forum_container'.

It could say 'forum_potato' and still work :)

So yeah, if the tests add new coverage, let's add them.

And let's remove the line altogether.

And that also means this is a task, for cleanup, rather than a user-facing bug.

Thanks everyone for their work so far.

Added interdiff and patch for the changes suggested in #15

Thanks, so we just need to confirm that we have existing coverage that addForum creates a forum (not a container).

If we do, then this is RTBC

I applied the patch and can normally create the forum and the container

Still need an answer for #18 I think.

So if we add the test that I wrote in comment #14 in the patch, we can confirm that the addForum is creating a Forum.

Or do we need to add anything more?

Tagging for Florida DrupalCamp which is happening today, Feb 10, 12-4pm ET

Thanks

I confirm addForum creates a forum after patching with #17 .

Thanks folks, we still need to confirm that we have existing test coverage that addForum creates a forum (not a container).

I'll update the issue summary with that step so that this doesn't get marked RTBC again without it.

Updating tags

Patch #17 implemented as per #15 guidelines. Patch seems like solution for the existing issue. Leaving as needs review for further verifications

We have existing test coverage for this form and that the forum_container field is set correctly in \Drupal\Tests\forum\Functional\ForumTest::createForum

So, the patch in #17 is indeed RTBC

Updated issue summary.

Not created any screenshots as these didn't answer the question whether we have existing automated test coverage of forum creation. That was answered in #31.

Committed and pushed 3cb6f37cc1 to 10.1.x and 2301594474 to 10.0.x and 1163080ceb to 9.5.x. Thanks!

Not backporting to 9.4.x as this is a task and changes runtime code.