I am having a problem, if I enable the Private Message File Attachments part of this module and grant correct permissions, if I go to add a file to a private message, I get the following text:

File attachments are disabled. The file directories have not been properly configured. Please visit the file system configuration page.

However, the file system is configured correctly, as the file attachments work everywhere else just fine.

I am using the latest 6.x version of Drupal and the latest dev version of this module.

Thanks,

JB

Comments

berdir’s picture

Status: Active » Postponed (maintainer needs more info)

That message is displayed if the module can't create the specified folder where it should upload the files. Maybe you have configured a folder that isn't accessible and can't be created. The setting is admin/settings/messages in a Privatemsg attachments fieldset. What does it display there? Try setting it to empty or a folder that does exist.

imatechie’s picture

That fixed it, I did have a path there and a corresponding folder with correct permissions, but it still didn't work, can you give a sample path that could go there??

I removed the path and it works now. The error message was a bit misleading, as it directed me to the file system configuration page of the main site and I think it should of mentioned about the settings in the module admin section instead.

berdir’s picture

Did you use an absolate path or something like that? Right now, only a relative path (based on the global upload directory) is supported.

And yeah, you're right about updating the error message, I've just copied that from upload.module.

imatechie’s picture

Aha, so your talking about a a directory off of Drupal's file attachments directory which by defualt would be:

drupal_root/sites/default/files

(Not sure about the trailing slash)

So if I was to put pm_attachments in the field set, the actual directory path would be one of the following:

drupal_root/sites/default/files/pm_attachments

drupal_root/sites/default/files/pm_attachments/

Thanks,

Jeff

berdir’s picture

Exactly, we probably need to explain that better and if possible, even provide a live preview of the resulting path and if the permissions are correct. Any suggestions are welcome!

imatechie’s picture

StatusFileSize
new43.28 KB

I'm not much of a programmer at all, and don't know anything about patch or diff files.

Anyway, in the interim, I modified the privatemsg_attachments.module file to provide a little better descriptions for the file-attachment path and the error message when the file-attachment fieldset is not configured correctly.

The changed .module file (I commented the changes) and two screen shots of the new messages are attached.

Hope you like it and that you don't mind the descriptions change.

I don't know the proper way to escape characters in Drupal, so I had to fall back on the backslash to escape an apostrophe or two.

You may want to verify that my string changes haven't broken anything else accidentally in that file. As from the screen shots, it looks OK.

Replace the privatemsg_attachments.moudle file with this one.

(For others, this goes into the privatemsg_attachments directory off of the privatemsg module directory)

Jeff

berdir’s picture

Status: Postponed (maintainer needs more info) » Needs review

Thanks for working on it, I'll check it out soon.

FYI, creating patches isn't *that* hard, see http://drupal.org/patch/create. And patches make it a lot easier to see the actual changes.

imatechie’s picture

Here is the patch for my changes in my previous comment, as stated, it simply changes the file attachment error message and links to the correct admin page if the private message file attachments are disabled due to a misconfiguration of the attachment directory fieldset in the private message administration settings.

This error message shows up when a user tries to attach a file to a private message and it isn't configured correctly.

The second change is in the administration settings for the private message module, the way the current wording is, one could think one was talking about a regular system path when they go to configure the path to the private message attachment directory.

This just changes the description here to indicate that the path is off of Drupal's main configured file attachment directory.

I am not a developer and hardly know anything about coding and this is my first patch, I did test it on my site and it worked.

Hopefully, this will make the descriptions much clearer.

berdir’s picture

Great, the patch per-se looks good, a few comments on the content...

+++ privatemsg_attachments/privatemsg_attachments.module.new	2010-04-20 05:06:27.000000000 -0400
@@ -48,9 +48,20 @@ function privatemsg_attachments_form_pri
+ ¶
+// Reword the error message to explain that the Private Message file attachment settings may be
+// configured incorrectly instead of referring to Drupal's file system.

Comments should only explain what's there and not the process that lead to the code. If someone else reads that comment in a year, it would make no sense at all.

I suggest to simply remove it, it's not something complex that we need to explain. Same for the comment below.

Also, the first line has a trailing whitespace, you should remove that too.

+++ privatemsg_attachments/privatemsg_attachments.module.new	2010-04-20 05:06:27.000000000 -0400
@@ -48,9 +48,20 @@ function privatemsg_attachments_form_pri
+ if (user_access('administer site configuration')) {

You seem to have changed the indentation of that if statement, there is no need for this patch to change this line.

+++ privatemsg_attachments/privatemsg_attachments.module.new	2010-04-20 05:06:27.000000000 -0400
@@ -85,7 +96,12 @@ function privatemsg_attachments_form_pri
+    ¶
....
+	'#description' => t('Relative path, based on Drupal\'s configured file attachments path, where private message attachments will be stored.'),

Again, a few trailing whitespaces and you used tabs instead of spaces. You should always use two spaces to intend.

Generelly, you should always try to keep the patch as small as possible (and as big as necessary). In your case, only the lines with the updated #descriptions should show up as changed.

Thanks for working on this! I'm sure you'll be able to re-use what you're learning here :)

Powered by Dreditor.

berdir’s picture

Status: Needs review » Needs work
berdir’s picture

Status: Needs work » Needs review
StatusFileSize
new2.23 KB

Updated the patch.

berdir’s picture

Status: Needs review » Fixed

Commited to 6.x-2.x-dev, thanks for reporting!

Status: Fixed » Closed (fixed)

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