Compatibility with comment_upload

sure thing

the "DEV Note" is nice to have it at hand when setting up this module,
right now it can be disabled with assertions altogether at the top of comment_driven.module
(but they are both vital when reporting issues)

where it reads
define('COMMENT_DRIVEN__ASSERTIONS', COMMENT_DRIVEN__ASSERTIONS__BOTH);
make it read
define('COMMENT_DRIVEN__ASSERTIONS', COMMENT_DRIVEN__ASSERTIONS__LOG);
then no more displaying errors or anything else
if you don't wan't log entries either there you have COMMENT_DRIVEN__ASSERTIONS__NONE

this, of course, should be turned into UI settings

you'll have it (just not today)

BTW, I'm handling compatibility issues as being "support" requests (those which requires debugging an unknown situation)
"feature" requests are for those issues requesting other kinds of driven properties (a known situation which needs per-case review before being enabled)

so, you're asking for "compatibility support" rather than "a new kind of featured driven property"

I'm about to commit this

committed to HEAD

I'm deeply sorry, but this is being backed out for the same reasons explained at #741544-8: comment_upload breaks on preview

EDIT: link was pointing to the wrong comment

re-opened as requested by obrienmd #741544-11: comment_upload breaks on preview

If I disable comment preview, will comment_upload still work? Until we move to D7, comment_upload will be an important part of my company's projects, and we'd like to use comment_driven widely. In cases where we'll be using comment_driven, we can disable preview if this would make it work (preview is rarely/never used)...

Yes, arhak - if you could provide a setting to remove preview button in exchange for comment_upload support, that would be great.

I would need feedback on this and #741544 as well
even when previewing comment_upload will probably end in a "won't fix"

NOTE: testing this patch requires to re-edit and re-save the content type in question
note the displayed warning regarding "comment preview" not being available when "driven properties" and "attachments on comments" are both enabled

the trial for preview is described at #741544-14: comment_upload breaks on preview
and the patch in the next comment bellow it

#10 missed a return statement & got a wrong varname

this is the same patch with those fixed

committed to HEAD

at the last minute I made a wrong merge,
there is a flag telling comment_driven to step out of the way if it detects "attachments on comments"

but even removing that flag, there are several node upload/comment_upload conflicts yet
I've to rework this out

the more I review it the more reluctant I'm to (almost) rewrite a third version of core's upload module

this is definitely POSTPONED
(with great chances to be unsupported)

as a feasible alternative you have filefield (which in addition will live for D7)

it would be very easy to set up a filefield (e.g. named Attachments) to be driven through comments
- there will be support to allow/deny its modification on node edition
- it would be much easier to make filefield properties to optionally support "only new/incoming values" to preserve existing files untouchable (i.e. don't allow outgoing)
- it just won't allow to be modified in comment edition

resulting effect:
- instead of having a table with files associated to comments they will be directly associated to the node (but visitor don't inspect the tables)
- the attachments visualization will be into the diff summary (with icons provided by filefield) with the left hand side always empty, since there will be no outgoing files, e.g:
Attachments: » +file1, +file2
- if disabled in node edition, the only other difference would be not being modifiable on comment edition

then I wonder:
is that bad not having comment_upload support?

this was backed out from HEAD for the second time

the reason to refuse the trade-off suggested by obrienmd (#8) is simple: it worked by chance

having enabled attachments on comments AND on the node gave: comment_form+node_form=works
having enabled attachments on comments BUT disabled for node gave: madness (i.e. won't work)

I would like to say that this is the module more compatible (or easy to make compatible) with other modules, but it is not actually
it aims to be extremely compatible with node_form-related mods, and will try to bring support for every comment_form-related mods I can

but the case of comment_upload is a special exception, it mimics core's upload, and therefore uses plenty strategies that can't be seamlessly merged when mixing node_form+comment_form, and after a merge, they must keep working as not being merged
in addition, it "rapes" the comment_form, and doesn't expect it to be cached
plus a lot of reported bugs

I think this took me more time than huge refactors to this module

anyway I'll keep it postponed, to let others bring their 2 cents
eventually, if not solved, it will get a "won't fix"

#746870: FAQ: what might be the problem with comment-related modules?

Argh, I'm trying to figure out how I can use the filefield method on our projects, but we have so many things using comment_upload :)

I like standardization and cleanliness, and thus agree with your filefield method, and think it is probably a best practice moving forward. Thus, I will give up my petition and simply use comment_cck (so painful, your module is far superior) with comment_upload on the sites I need to, until I can set aside the resources for a full migration to filefield + comment_driven.

Thanks for all your hard work! Apologies for not watching the issue queue, I've had my head in a few major projects for the last week.

the alternative at #16 is not available yet
right now I'm working on several things at the same time to try getting a first alpha out the door

Subscribe.

Also, why i see this as important is its installed with Open Atrium and when being used as an issue tracker/helpdesk ticket system (a likely use case) being able to attach files to a comment is very handy (e.g. a screen shot).

we are close to provide alternative suggested at #16
which will make it not to be so indispensable
or do you have any objections with proposal at #16

#16 looks good to get the functionality. But my main reason for wanting full compatibility is just for OA integration.

OAtrium will have to wait, I've wasted too much time trying to figure this out and haven't so far

BTW, the module doesn't "stop working", rather it "steps out" when detects "attachments on comments", to avoid reflecting non-existent bugs on c_upload,
since cdriven can continue working properly (as far as I recall),
moreover, it can make comment_upload disabled for those driven content types, but it seems to me that it would be a transgression
thus, I choose to make the module inactive in such cases, while displaying a warning on its settings

arhak, hats off to you for the progress you've made so far with the module. Its VERY good, my OA obsession is due to https://community.openatrium.com/issues/node/52 and this module (previously had been using comment_cck) is going to be central to our implementation. But it works fine, just have to turn off comment_upload ;)

proposed alternative @#16 is now available (latest HEAD of Driven API)
it will work with comment_driven-6.x-1.0-alpha1 (since the required code was already there)
but I would recommend keep tuned with dev in case of bug fixes (log at http://drupal.org/project/cvs/705778)

steps:
- create a multiple/unlimited filefield named "Attachments"
- enable new module "Driven CCK Producer/Consumer Policies"
- make it "Incoming" from its property settings ("Comment driven" tab) i.e. Producer policy
- optionally make it "hidden" with "Driven Node Edition Policy" ("Edition hidden" tab)

anyone willing to write a handbook page or FAQ issue about it would be very appreciated

#29 thanks for the illustrative screenshot

BTW, there is an old known bug in alpha1 related to the use of add_more button on preview that should be fixed in alpha2 (both alpha2: comment_driven & Driven API)

well according to filefield

The "list" option lets a user choose if a file should shown in a list when viewing the content after creation.

if you let the user to choose how it will be rendered.. well, comment_driven will tell CCK to render it, whatever that might be (in this case, an empty string)

I would recommend to configure the field with "List field" Disabled on its "Global settings"
that way users won't have the chance to uncheck the "list" option

what else could be done?

@#34 debatable
lets take the discussion to a separate issue #762126: Discussion: rendering existing differences that doesn't display/look like differences at all

Hi,
I have tried method from #16 and it is partially working. I am also using FileField path module and file paths in comments are not correct. The are correct in node. See the attached picture.
Is this possible to fix?

Best regards,
gapa

@36 please, file a new issue, with a link to that screenshot
and details regarding your filefield_path configuration
(also whether you're using private/public download method)
and anything else you might find relevant

#36, #37: #851118: compatibility with filefield_paths combined with imagefield

To clarify, comment_upload support is postponed and lurching toward a Won't Fix.

However, filefield support is committed to replicate the core functionality of comment_upload?

To clarify, comment_upload support is postponed and lurching toward a Won't Fix.

that's correct, it was said at #19 and it keeps being the case so far

filefield support is committed to replicate the core functionality of comment_upload?

not sure what you meant,
but #29 might be the kind of answer you're looking for
(I don't have any other references than what was said on this thread)

I am referring to #16/#28.
Yes, as illustrated by #29.

Thank you for confirming.

Way forward for Atrium then is to use a Filefield, and identify whether I need the Latest Files view provided by Atrium (and built on a custom Upload module views handler). Thank you for taking the extra time on this.