Attached file not added

the file should probably be put into the session during preview.

The same thing also happens in the 5.x-1.0 release - even when you do not preview it.

releases are regular nodes, using the upload module for file attachments, so this is not a project problem. at any rate, i have not been able to reproduce the problem via local testing.

closing the issue for now. if the problem persists, it should be reclassified as a core bug against the upload module.

I can reproduce this problem on my test site following hass's instructions in the original post.

Furthermore, I don't think it's a problem with core. Project_release may use the upload module, but it's clearly doing things a bit differently, as the UI for adding and removing files in project_release nodes is different than, for example, story nodes. Also, if I follow the same steps as in the original post except create a story node instead of a project_release node (but still add the file as an attachment, preview, submit) there is no problem and the file is uploaded and attached to the node correctly.

I'm using Drupal 5.2 and project 5.x-1.x-dev

is there something we gain here from having our own special 'file' section in a release node? with upload attachments enabled, you already have a way to attach files to a release node. having this extra file field seems both unnecessary and confusing.

if we need to store some additional db info about the file, i would think we could do that in an extra #submit handler.

I'm with hass on this one. I think it would be more work to restrict files uploaded as node attachments properly than to fix project*'s file upload code to handle attachments properly (but maybe not).

If standard uploads via node attachments were used, those would need to be restricted such that only 1 file can be uploaded to the node and the file itself cannot be deleted from the node except by people with administer projects permission (I think). We'd also still need to store the file's information in {project_release_nodes}.

This is purely a guess, but I suspect that outside of d.o, most sites using project* don't do in combination with CVS or other version control and the associated packaging scripts. On my site we allow project owners to upload files or to use subversion and have automatic packaging. About 1/2 of our project nodes use subversion, but most of those are mine.

My point here is that this bug probably affects a lot of sites (just not d.o), and that we have to be careful fixing it so that the functionality itself isn't changed, since if that were the case it could mess up many sites.

As a temporary fix for this problem, on my site I form_alter the project_release preview button off of the project_release form so that files don't get lost/stuck. We could do that for project_release.module if fixing this bug turned out to be really difficult--but maybe people think the preview is really important and that would be a bad idea.

a) I agree this is a bug (given the current code).

b) I agree that most sites using project* don't use cvs.module.

c) Many of the sites using project* would probably use versioncontrol.module and a SVN backend for it if it existed.

d) I rather strongly disagree with the assumption that all release nodes should only ever have a single file associated with them. For example, the project* sites at my day job are representing releases of compiled binaries. We support about 20 different platforms, and have separate tarballs for each one, not to mention the source tarball itself. Of course, as currently written, project_release.module makes assumptions about a 1:1 relation between releases and files, but that is itself a bug, not a feature we should hold on to.

e) It is silly for project_release to duplicate file attachment code -- it should just use core's code.

So, I'd prefer to see this fixed by the following:

1) Rip out the existing file upload code.

2) Fix places in project_release that assume 1:1 relation between releases and files. Perhaps make this a site-wide and/or per-project setting so that people can choose between 1:1 and 1:N. In all cases, make sure we do whatever we need to do to play nicely with upload.module, instead of forking that code.

That said, I have no time to work on this, so if someone provides a patch for a short-term work-around that fixes the existing bug in project_release's upload code, I won't object, and would happily review and commit it. But, medium-term, the above plan is how we should solve this.

Cheers,
-Derek

attached has been applied to 5.x-1.x

let's move the the discussion for using core's file handling over to http://drupal.org/node/179471

patch doesn't apply to 4.7.x-2.x, so leaving to be ported in case anybody wants to try fixing that as well.

...and the patch...

FYI: this patch broken file uploads. See http://drupal.org/node/185354

committed the following patch to 4.7.x-2.x, which is the fix supplied here, plus the correction made at http://drupal.org/node/185354

tested, and works fine with both cvs module enabled or not.

this will not be backported to 4.7.x.-1.x by the project team.

Right, there's nothing to backport to 4.7.x-1.* anyway, since project_release.module was new in 4.7.x-2.*. ;)