Support from Acquia helps fund testing for Drupal Acquia logo

Comments

bhosmer’s picture

Well, here goes my first try at contributing a patch to core.

bhosmer’s picture

Assigned: Unassigned » bhosmer
bhosmer’s picture

Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, drop-remnants-of-node-edit-for-1297410-1.patch, failed testing.

bhosmer’s picture

Status: Needs work » Needs review
FileSize
547 bytes
sun’s picture

Status: Needs review » Needs work
+++ b/modules/node/node.pages.inc
@@ -107,8 +107,7 @@ function node_form($form, &$form_state, $node) {
   // Identify this as a node edit form.
-  // @todo D8: Remove. Modules can implement hook_form_BASE_FORM_ID_alter() now.
-  $form['#node_edit_form'] = TRUE;
+  $form_state['build_info']['base_form_id'] == 'node_form';

The removals are OK, but should actually include the comment.

And the new condition actually needs to be used wherever #node_edit_form is currently used throughout core. (most likely, in tests only)

-13 days to next Drupal core point release.

bhosmer’s picture

From looking at the simpletest, I see it was also used in the forum. module too.

What comment are you referring to that still needs to be included?

The removals are OK, but should actually include the comment.

sun’s picture

The chunk of lines you touched in node_form() can be removed entirely; i.e., including the comment, and without replacement.

bhosmer’s picture

Cool. Thanks.

Now, what about forum.module:

if (!empty($form['node_form']) && isset($form['taxonomy_forums'])) {
    $langcode = $form['taxonomy_forums']['#language'];
    // Make the vocabulary required for 'real' forum-nodes.
    $form['taxonomy_forums'][$langcode]['#required'] = TRUE;
    $form['taxonomy_forums'][$langcode]['#multiple'] = FALSE;
    if (empty($form['taxonomy_forums'][$langcode]['#default_value'])) {
      // If there is no default forum already selected, try to get the forum
      // ID from the URL (e.g., if we are on a page like node/add/forum/2, we
      // expect "2" to be the ID of the forum that was requested).
      $requested_forum_id = arg(3);
      $form['taxonomy_forums'][$langcode]['#default_value'] = is_numeric($requested_forum_id) ? $requested_forum_id : '';
    }
  }
}

I assume this entire function can be removed or not?

bhosmer’s picture

Status: Needs work » Needs review
FileSize
1.2 KB

Status: Needs review » Needs work

The last submitted patch, drop-remnants-of-node-edit-form-1297410-8.patch, failed testing.

bhosmer’s picture

bhosmer’s picture

Status: Needs work » Needs review
FileSize
1.7 KB

Status: Needs review » Needs work

The last submitted patch, drop-remnants-of-node-edit-form-1297410-8.patch, failed testing.

bhosmer’s picture

Status: Needs work » Needs review
FileSize
1.66 KB

Status: Needs review » Needs work

The last submitted patch, drop-remnants-of-node-edit-form-1297410-8.patch, failed testing.

bhosmer’s picture

Status: Needs work » Needs review
FileSize
1.2 KB

Status: Needs review » Needs work

The last submitted patch, drop-remnants-of-node-edit-form-1297410-16.patch, failed testing.

bhosmer’s picture

What should I do with the forum problems? Any advice would be appreciated.

bfroehle’s picture

Status: Needs work » Needs review
FileSize
1.4 KB

I split the relevant section of code out to forum_form_node_form_alter() (i.e., an implementation of hook_form_BASE_FORM_ID_alter).

sun’s picture

Status: Needs review » Reviewed & tested by the community

Thanks all!

chx’s picture

So agreed. A followup patch should rename the node form to node_form_$node_type. But for now, this is great.

catch’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Novice, +API change, +API clean-up

The last submitted patch, remove_pound_node_edit_form-1297410.patch, failed testing.

bfroehle’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
1.78 KB

Rerolled after /core.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Nice little clean-up. Committed to 8.x. Thanks!

sun’s picture

Title: Drop remnants of #node_edit_form » [change notice] Drop remnants of #node_edit_form
Status: Fixed » Needs work

This needs an API change notice.

bfroehle’s picture

Status: Needs work » Needs review

Change notice created: $form['#node_edit_form'] removed

xjm’s picture

Title: [change notice] Drop remnants of #node_edit_form » Drop remnants of #node_edit_form
Status: Needs review » Fixed

I updated the change notice with corrections from the comments.

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