Adding a vocabulary without any associated content type gives php notices [#163099]

Environment Details

PHP - 5.2.3
Apache - 2.2.3
Drupal - drupal-6.x-dev
OS - Linux (Fedora Core 6)

Steps:

1 - Create a vocubary http://www.testsite8.com/admin/content/taxonomy/add/vocabulary
Name: Vocab 1
leave other fields as it is.

2 - Click on "Save" button.
This takes the user back to the Administer Category page (http://www.testsite8.com/admin/content/taxonomy)

it shows listing of difference vocabularys and gives the php notices

* notice: Undefined variable: type in /work/projects/testsites/drupal/6.x/testsite8/www/modules/node/node.module on line 342.
* notice: Undefined index: in /work/projects/testsites/drupal/6.x/testsite8/www/modules/node/node.module on line 342.

yashesh bhatia

Comment	File	Size	Author
#6	163099_2.patch	725 bytes	yasheshb
#6
#1	filename_1.patch	635 bytes	yasheshb
#1
	Screenshot-5.png	137.19 KB	yasheshb

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

yasheshb CreditAttribution: yasheshb commented 30 July 2007 at 15:28

Status:

Active

» Needs review

File	Size
filename_1.patch	635 bytes

the problem was in the taxonomy module where it was creating an array with keys and no values in the function
taxonomy_get_vocabularies.

here's the patch for review. appreaciate feedback.

thanks.

yashesh bhatia.

Comment #2

Dries CreditAttribution: Dries commented 30 July 2007 at 21:31

Why would you want to create a vocabulary without types?

Comment #3

Crell CreditAttribution: Crell commented 31 July 2007 at 00:52

@Dries: In case you wanted to use it with cck_taxonomy or some other "unconventional" module.

Comment #4

yasheshb CreditAttribution: yasheshb commented 31 July 2007 at 05:23

true, there may be very few instances where one would want to create a taxonomy tree without associating content types to it,
however, should the code not be solid enough to trap these one off edge cases ? again, it's debatable, one school of thought would be that if a user does something unexpected, let them see the warnings.

another would be to assume that a user may not provide conventional (generally accepted) data in the form and trap those edge cases - this way a newbie user of drupal would not be welcomed with those deadly red colored php notices when they're using / evaluating drupal.

rgd.

yashesh bhatia.

Comment #5

Dries CreditAttribution: Dries commented 31 July 2007 at 13:27

yasheshb: the code should be solid enough, but I was trying to understand the problem. If the answer is: "there should always a be a node type associated with the vocabulary", we might was well make it a required field, and the notice should not trigger. See what I mean?

PS: the coding style in the patch needs some work. We put the "else {" on a new line.

Comment #6

yasheshb CreditAttribution: yasheshb commented 1 August 2007 at 07:10

File	Size
163099_2.patch	725 bytes

dries: yep, i see your point of view and don't have an answer whether it should be a required field or no. thx for point it out. also, i've fixed the code (actually coded it a bit differently). patch is attached for review and can be used in case node type is not made a required field.

yashesh bhatia.

p.s. sorry about the 'else' part of the code.. i was very used to the pear coding standard, which is a little bit different from the drupal one and hence the error.