Hi,

I was able to add CCK field to project_issue content type in version 5.x-1-3. I cant do it in any of the later versions. Clicking "manage fields" or "add field" redirect me to
node/add/project-issue

I would appreciate any help with this !
simul

CommentFileSizeAuthor
#4 project_issue_cck_manage.patch954 bytesaclight

Comments

aclight’s picture

Version: 5.x-2.1 » 5.x-2.x-dev

I'm able to confirm this. I believe that this is all due to the changes to the project_issue add form made in #199138: Remove multipage form for issue nodes. Though I haven't checked, I suspect that this same problem would be present with project release nodes since a similar fix went into the project module in #105532: project release link at node/add is a 404. Both of these seem to be due to the fact that CCK renders the form internally first and somehow project issue doesn't like that.

By commenting out lines 664-668 of issue.inc (see code below), I was able to get CCK fields to work OK with the issues content type, but of course this isn't by any means an actual fix to this problem, just proof that project_issue_form() is part of the problem here.

  //if (empty($pid)) {
  //  drupal_set_message(t('Invalid project selected.'), 'error');
  //  drupal_goto('node/add/project-issue');
  //  return;
  //}

We'll have to investigate this and see if we can come up with a fix.

dww’s picture

Eeek. :( That sucks. Yeah, we'll have to dig deeper, this is definitely functionality we want to support, but I'd hate to have to undo the multipage form simplification patch.

aclight’s picture

Yeah, I wouldn't want to undo that simplification patch either. The good news is that I briefly messed around with adding fields to project_release node types, and couldn't get the same problematic behavior. I didn't have a chance to dig into how pi and pr do the multipage simplification differently, if they do, but we might be able to get a workaround by investigating the differences between the two implementations.

aclight’s picture

StatusFileSize
new954 bytes

On further inspection, I see where the problem is. As I said above, CCK renders the node forms to see what fields are there and what the weights are. This allows someone to set the order of the CCK fields at admin/content/types/project-issue/fields, for example. The problem is that in both project_issue_form() and project_release_form(), at some point the code needs to know the pid of the project for the release/issue. When CCK builds the forms, this information isn't provided so the form builder functions crap out at some point.

CCK builds a dummy form in content_admin_field_overview_form() of content_admin.inc. After it builds the dummy form, it iterates over the form and pulls out top level fields into the manage fields table. For project issue nodes, title is the only top level field. For project release nodes, there's title, rel_id, taxonomy, and body. So previous to pi 2.x, you could add cck fields and manage them without getting an error but you didn't have complete control over placement of the fields.

Fixing this problem is pretty easy, though slightly hackish. When CCK creates this dummy form, it sets $node->cck_dummy_node_form = TRUE. So, from project_issue_form() and project_release_form(), we can look for the presence of this property in $node and if it's there just return an empty array. Again, since CCK isn't going to use the fields that aren't top level fields, theres no need to expose them in the first place.

I'm attaching the patch for project issue here. If we're ok with this approach, I'll create an issue for the project module and attach a similar patch there as well.

aclight’s picture

Status: Active » Needs review

setting status

dww’s picture

Nice detective work, aclight. ;) Does seem slightly hacky, but sounds it it should do the trick (I haven't tested, yet). I wouldn't mind getting a CCK developer to comment on this, if possible, but they're probably all pissed at me for taking so long with #218973: Reorganize CCK in CVS. ;) That said, if this solves the problem, it seems much better than undoing the landing-page-is-really-a-different-form simplification. I'll probably just commit this, but let's see if we can get another opinion first (and ideally, some testing from the original reporter of this bug).

Thanks,
-Derek

aclight’s picture

Yep, getting some CCK feedback would be nice. I've set up my IRC client to stalk yched and KarenS, but I don't see them around all that much on IRC.

@simul: As dww said, it would be great if you could test this patch to see if it solves your problem. Also, it would be nice if you can confirm that in previous versions of project issue, when you clicked the manage fields link for the project issue node type, the only fields you saw listed were the title and then whatever CCK fields you had already added. I want to make sure that we're not regressing in the number of fields on the issue node type that CCK is aware of.

aclight’s picture

I've contacted both KarenS and yched via their contact forms and have asked them to post a note here with any feedback.

yched’s picture

This is the right fix - the $node->cck_dummy_node_form flag was added just to solve this kind of problems with another module, can't remember which one now.
Building a dummy node form to build that 'manage fields' page is definitely hackish, but it's the only way we had to find out what elements appear in the node form to position cck fields around. This has been refactored in CCK D6, and the pseudo form is gone. (me wonders if there could / should be a D5 backport...)

@dww : we're not pissed at you, we know what it means to be swamped ;-)

dww’s picture

Status: Needs review » Fixed

Cool, thanks. Tested and committed to DRUPAL-5--2 and HEAD.

Anonymous’s picture

Status: Fixed » Closed (fixed)

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