Node type disappears if its "type" has already been used (same orig_type)

Rerolled

This needs to be tested before we commit this....

The problem is certainly there and it's there for the reason stated: the four lines near the end of the function that look like this:

if ($type_object->type != $type_object->orig_type) {
        unset($_node_types->types[$type_object->orig_type]);
        unset($_node_types->names[$type_object->orig_type]);
      }

Imagine that node type with machine type "page" has had its type changed to "zpage", and then a new node type is create with machine type "page". This function parses through all the available machine types alphabetically, so it hits the new "page" first and adds it to $_node_types. Eventually it hits "zpage", and adds that to $_node_types. But then the "if" statement above is triggered and it goes back and unsets what it had added to $_node_types under page.

I think you're right that these lines should be removed, as I can't work out when they would actually be useful. This isn't helped by the complete absence of code comments so I've attached a new patch which both removes the two lines of code and adds some comments.

Here's the argument, though:

$_node_types is reset on line 668. Then nodes which are created using hook_node_info in some module are added to $_node_types on lines 670-675:

$info_array = module_invoke_all('node_info');
  foreach ($info_array as $type => $info) {
    $info['type'] = $type;
    $_node_types->types[$type] = node_type_set_defaults($info);
    $_node_types->names[$type] = $info['name'];
  }

Next the database is queried for all the node types in the node_type table. This is so that node types which haven't shown up in some hook_node_info but are still in the node_type table can be marked as disabled - I think the disabling/deletion is done by node_types_rebuild. At the same time, any node types which aren't created by a module are added to $_node_types.

Finally, the function goes through each node type in the database and, if its machine type has been changed, it removes any reference to the original machine type from $_node_types. But because of the way the function is constructed, it's impossible that the original machine type would have been used incorrectly.

I don't want to spend any time going through the CVS records to work out why this was originally done, but it would be interesting to hear from anyone who can work it out. For the moment, though, I think the argument is clear so those lines should be removed.

I like the added comments, but I think the bug is elsewhere. The condition clearly expects that type == orig_type. Hence, orig_type was not updated after type was renamed. Hence, we need to make it update orig_type.

As far as I could see from stepping through this code, those lines didn't serve a purpose. Any idea what they are supposed to do, or why they were originally added? I think that might be the quickest way to get to the bottom of this.

EDIT:

we need to make it update orig_type

Unless I'm missing something, orig_type is never updated: it's the original type of a node created by core or a module, and is stored in the node_type table in the database.

Those lines were definitely added on purpose, but since the commenting in the code is non-existent I can't tell what that purpose is supposed to be!

I've posted on the issue that originally gave us this code, to try and ping the people that were working on it - but since that was three years ago it's doubtful any of them will remember what the code is supposed to do.

I'll have a bit more of a look at this later and try and follow the logic through. And I'll sort the extra white space out in the next patch - tks.

Reading the patch in #62340-162: Pave the way for CCK, I think those lines are trying to do what I briefly explained in #6 already.

1) We want to add those comments.

2) But I think the added comments are partially wrong.

3) And we additionally want to fix the underlying issue I mentioned in #6.

4) And we need tests.

New patch with proper comments to use as base for further patches.

thirdly it doesn't work consistently anyway, as described above at #5 and in my original post; and as you say, orig_type would then also need to be cleared once the rename is finished

Right. And that's the part that needs to merged into the patch in #10. Plus tests.

Changing status.

#10: drupal.node-type-orig.patch queued for re-testing.

I still have this patch in my file system, so let's get this fixed. Any takers? At the very least, we want to commit the comments added in #10.

Subscribe. I'm now left with an unusable namespace, that I want to use.

Updating version so this gets on the radar

Comments from #10 look fine to include.
To do: we need to add some validation to the node types form that tests the machine name 'type' field against the existing 'old types' for when creating a new node type and throws an error if it already exists otherwise the new node type is never usable (without changing the old_type in the db).

Here is the patch for the validation node type for drupal 6.22.

My first patch, ... creating a patch is not so easy.

Here is the patch for the validation node type, checking the orig_type value, for drupal 6.22 (retry).
it edits the node_type_form_validate() function.
EDIT : both patches are indentical, I have uploaded it twice by mistake.

Here is the correct patch (on drupal 6.24).
Comparing to my previous patch, it allows a content type to switch back to its origin type value.

Here is the patch for drupal 6.25.

This bug exists on D5, D6, D7 and D8 !
Patch for D7 and D8 below.

Here is the patch for drupal 7.12.

This bug exists on D5, D6, D7 and D8 !
Patch for D6 in my previous post.
Patch for D8 below.

Here is the patch for drupal 8x-dev.

This bug exists on D5, D6, D7 and D8 !
Patch for D6 and D7 in my previous posts.

Updating version to the current Drupal major release.

This needs to be resolved upstream first to limit regression.

*bump*

For your information, I have contributed the LimeSurvey Sync module : It contains the fix for that issue for Drupal 6 and Drupal 7 : All you have to do is to download it and enable the module LimeSurvey Sync Survey (no need to install the LimeSurvey software).

Note : On Drupal 8 this issue causes a fatal error !

Tested on drupal 8.0.0-beta4 : This issue does not exists anymore, since the 'orig_type' is not used on Drupal 8.
Unfortunately, this issue still exists on Drupal 7 and 6,
So I have changed the issue version to 7.x-dev.
I will prepare a patch for the 7.34 version.

Here is a full description of this issue, that affects Drupal 5, 6 and 7 :
Drupal 8 is not affected anymore.

When creating a new content type with a type that has already used, the new (or the old) content type disappear everywhere.
The only way to make it back is to delete the dooblon directly into the database.

Steps to reproduce this issue :

1. go to admin/structure/types/add and create a new content type with machine name (type) = 'test1'.
2. save it
3. go to admin/structure/types/manage/test1 and edit the machine name (type) from 'test1' to 'test2' (and edit the human name to).
4. save it
5. go to admin/structure/types/add and create a new content type with machine name (type) = 'test1'.
6. --> the new content type with machine name 'test1' does not appear but it has been created and is present and enabled into the database : see screenshots.

This issue appears because the node_type_form_validate() function does not check for dooblons on the node type orig_type value.

Patches below for Drupal 7.34 and 6.34.

Patch for Drupal 6.34

Patch for Drupal 7.34

I have create a distinct issue for Drupal 6 :
https://www.drupal.org/node/2397793

I am glad I came across this ticket. Experiencing the same problem in D7.39. Applied #40, and then did the following:

1. Created a new content type "Test A" (machine name: test_a)
2. Updated the name and machine name to: "Test B" and "test_b"
3. Attempted to create a new content type "Test A"

And it gave me the following:

It seems to behave as thedut intended in the patch. (Although it would have been great if I can actually create a new test_a after the first one!)

@thedut i have applied the patch

---- Create a content type with test1 and save and edit it and rename test2 with machine name also

---- now go to create another content type with test1 name so i am getting following error and not able to create content type with test1 name

Hello vikashsoni,

I guess that thanks to the #40 patch you get your nodes back.
The work arround to come back to test1 is :

• either to delete test2 content type from the user interface, But you will loose all your nodes definitively ! So you probably don't want that !
• or to edit the 'origin_type' value for test2 into the node_type database table from: 'test1 to test2, using PhpMyAdmin for instance.

The patch is still interessting because it prevents from loosing an entire content type !
I guess you could mark this issue as "reviewed".

I can confirm this is still an issue on the latest 7.x branch.

I tested my MR on simplytest.me. It fixes the issue reported here.