Hello, I've recently installed this module and it's great, but I've changed the behaviour of the 'user/%/messages' menu link. I think that it's natural that users could view his own messages in this link, so I've patched the code to get it working. The patch works fine and I'm using it in my little web site. I've very little experience in Drupal development, so I don't know if my approach is correct.

It's my first patch too, sorry if there are many mistakes.

Kind regards.

Files: 
CommentFileSizeAuthor
#27 privatemsg-1443172-27.patch5.49 KBptmkenny
PASSED: [[SimpleTest]]: [MySQL] 4,987 pass(es).
[ View ]
#22 privatemsg-fix-user-tab-access-1443172-7.x-1.x-22.patch5.45 KBmstef
FAILED: [[SimpleTest]]: [MySQL] 2,819 pass(es), 227 fail(s), and 182 exception(s).
[ View ]
#16 privatemsg-fix-user-tab-access-1443172-7.x-1.x.patch5.45 KBmstef
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch privatemsg-fix-user-tab-access-1443172-7.x-1.x.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#13 privatemsg-1443172-9.patch1.93 KBigalarza
PASSED: [[SimpleTest]]: [MySQL] 3,715 pass(es).
[ View ]
#11 privatemsg-1443172-9.patch1.93 KBigalarza
FAILED: [[SimpleTest]]: [MySQL] 3,705 pass(es), 9 fail(s), and 5 exception(s).
[ View ]
#9 privatemsg-1443172-9.patch1.94 KBigalarza
FAILED: [[SimpleTest]]: [MySQL] 3,715 pass(es), 0 fail(s), and 15 exception(s).
[ View ]
#5 privatemsg-view_own_messages.patch2.05 KBigalarza
PASSED: [[SimpleTest]]: [MySQL] 3,715 pass(es).
[ View ]
privatemsg-view_own_messages.patch2.11 KBigalarza
FAILED: [[SimpleTest]]: [MySQL] Invalid patch format in privatemsg-view_own_messages.patch.
[ View ]

Comments

this makes sense

Status:Active» Needs review

The problem is that the page is not a full replacement for /messages, for example, there is no tag or blocked user management. The sole purpose for these pages is to view messages of other users.

6.x-2.x has a feature that allows to move it below the user profile but that's very complicated and I'm not sure about porting it.

For example, there are many links and redirections which point to /messages. Giving users two separate places is IMHO confusing.

Status:Needs review» Needs work

The last submitted patch, privatemsg-view_own_messages.patch, failed testing.

Your patch probably contains windows line endings, the testbot doesn't like that.

StatusFileSize
new2.05 KB
PASSED: [[SimpleTest]]: [MySQL] 3,715 pass(es).
[ View ]

Hello Berdir, the patch does a redirect from /user/%/messages to /messages when you are viewing your own user, so it don't use two different places for messages.

I think that the line endings are right now.

Regards

Status:Needs work» Needs review

Remember to set the Status to needs review when posting patches.

ok, sorry. And thank you for your patience.

Status:Needs review» Needs work

Ah, didn't see the redirect, sorry. Then I'm fine with this.

Lot's of nitpicking about coding style and how to simplify the function below. It looks like a lot, but it's not, don't worry. I'm just picky about the coding standard and I've been very detailed. In fact, with my suggestions, you should be able to reduce the lines of code in that function to 4-5 ;)

+++ b/privatemsg.moduleundefined
@@ -349,6 +349,36 @@ function privatemsg_user_access($permission = 'read privatemsg', $account = NULL
+ * Privatemsg wrapper for user_access and the user/%/messages menu options,
+ * allow access to the own profile, otherwise calls to privatemsg_user_access

The perfect first sentence in a docblock should be as short as possible, start with a verb in 3rd person and if possible, below 80 characters.

Suggestion:

Checks access for the messages tab on the user profile.

You can then add a more detailed description below that, after an empty line.

+++ b/privatemsg.moduleundefined
@@ -349,6 +349,36 @@ function privatemsg_user_access($permission = 'read privatemsg', $account = NULL
+ * @param $permission
+ *   Permission string, defaults to read privatemsg

Nope, it doesn't :) Since we are using this function only in this case, we can save the arguments completely (both), and just hardcode a privatemsg_user_access('read all private messages') at the end.

+++ b/privatemsg.moduleundefined
@@ -349,6 +349,36 @@ function privatemsg_user_access($permission = 'read privatemsg', $account = NULL
+ * @return
+ *   TRUE if user has access, FALSE if not

This line should end with a .

+++ b/privatemsg.moduleundefined
@@ -349,6 +349,36 @@ function privatemsg_user_access($permission = 'read privatemsg', $account = NULL
+ *
+ * @ingroup api

This also isn't really an API function, so remove this.

+++ b/privatemsg.moduleundefined
@@ -349,6 +349,36 @@ function privatemsg_user_access($permission = 'read privatemsg', $account = NULL
+function privatemsg_ownuser_access ($permission = NULL, $account = NULL) {

No space before the (. Also, I'd suggest to actually pass in the account object of the user we're visiting. Then the access check below becomes really simply. Because all you need is a if ($user->uid && $user->uid == $account->uid). To pass in the account, change the access arguments to array(1).

+++ b/privatemsg.moduleundefined
@@ -349,6 +349,36 @@ function privatemsg_user_access($permission = 'read privatemsg', $account = NULL
+  if (!$account->uid) { // Disallow anonymous access, regardless of permissions
+    return FALSE ;

Comments should always be on their own line, either within or above the if, whatever you prefer. And also end with a .

+++ b/privatemsg.moduleundefined
@@ -349,6 +349,36 @@ function privatemsg_user_access($permission = 'read privatemsg', $account = NULL
+  else

Either add {} (they should always be used, even for single lines, to prevent confusion or remove the else completely. Because if the mentioned if statement above is true, it will return and not continue in the function anyway.

+++ b/privatemsg.pages.incundefined
@@ -150,6 +150,11 @@ function privatemsg_list_page($argument = 'list', $uid = NULL) {
+  // redirect to 'messages' if it's the own profile
+  else if ((int)$uid > 0 && $uid == $user->uid) {
+      drupal_goto('messages') ;
+      return ;

Same stuff here, start comment with an uppercase letter and end with a .

Also, no space before the semicolon.

Status:Needs work» Needs review
StatusFileSize
new1.94 KB
FAILED: [[SimpleTest]]: [MySQL] 3,715 pass(es), 0 fail(s), and 15 exception(s).
[ View ]

Hello again and thanks for your corrections. I'm not used to the drupal code standard and I'm very happy to have this detailed feedback of my little piece of code.

I've tried to follow all your advices and there is the result; maybe my patch still have mistakes, english is not my mother language and sometimes is hard to express what I want to explain and to understand all the details.

I hope that everything is correct now, but I would be happy to change anything you want if remains any problem. Of course you can change yourself anything in the patch if you prefer it.

Kind regards.

Status:Needs review» Needs work

The last submitted patch, privatemsg-1443172-9.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new1.93 KB
FAILED: [[SimpleTest]]: [MySQL] 3,705 pass(es), 9 fail(s), and 5 exception(s).
[ View ]

Sorry, a stupid mistake.

I think is right now.

Status:Needs review» Needs work

The last submitted patch, privatemsg-1443172-9.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new1.93 KB
PASSED: [[SimpleTest]]: [MySQL] 3,715 pass(es).
[ View ]

Wrong patch, there is the correct one.

sub

This needs to be fixed in 7.x-1.x as well.

I'll write a patch

Title:View own messages in 'user/%/messages'git
StatusFileSize
new5.45 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch privatemsg-fix-user-tab-access-1443172-7.x-1.x.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Patch here to fix the access callback for user tabs on the profile. It will allow users to access it if they're viewing themselves, or for anyone, if they have "read all messages' perm.

I also removed privatemsg_list_page() because it's completely unnecessary. %user should be used in hook_menu() to load and pass the user object. All other menu callbacks can call drupal_get_form('privatemsg_list') directly, and bypass the need for that function.

PS: This is for 7.x-1.x.

Status:Needs review» Needs work

The last submitted patch, privatemsg-fix-user-tab-access-1443172-7.x-1.x.patch, failed testing.

Title:git View own messages in 'user/%/messages
Status:Needs work» Needs review

Didn't mean to change the title

Any progress with this?

I have resolved this issue in my custom module

/**
* Implements hook_menu().
*/
function YOURMODULE_menu() {
$items['user/%user/messages'] = array(
    'title' => 'Messages',
    'page callback'    => 'YOURMODULE_list_page',
    'page arguments'   => array('list', 1),
    'access arguments' => array('read privatemsg'),
    'type' => MENU_LOCAL_TASK,
  );
return $items;
}

function YOURMODULE_list_page ($argument = 'list', $account) {
  global $user;
  module_load_include('inc', 'privatemsg', 'privatemsg.pages');
  if($account->uid > 0 && $account->uid != $user->uid) {
    if (!privatemsg_user_access('read all private messages')) {
      return MENU_ACCESS_DENIED;
    }
  }
  return drupal_get_form('privatemsg_list', $argument, $account);
}

Version:7.x-2.x-dev» 7.x-1.x-dev

Let's try again with the correct version for my patch

StatusFileSize
new5.45 KB
FAILED: [[SimpleTest]]: [MySQL] 2,819 pass(es), 227 fail(s), and 182 exception(s).
[ View ]

Status:Needs review» Needs work

The last submitted patch, privatemsg-fix-user-tab-access-1443172-7.x-1.x-22.patch, failed testing.

Thanks to tymofii.pashkevych. #20 Good solution.

Potential solution (patch) here: https://drupal.org/node/1772236

Version:7.x-1.x-dev» 7.x-2.x-dev
Priority:Minor» Normal
Status:Closed (duplicate)» Needs review
StatusFileSize
new5.49 KB
PASSED: [[SimpleTest]]: [MySQL] 4,987 pass(es).
[ View ]

This patch works for user/UID/messages but is there a way to make it go to the sent box like user/UID/messages/sent as well?

+1