Support tokens in private messages

the data would eventually have to be pulled from the db anyway and pmgrowl does a good job of that. The "token support" mentioned in #495662-4: Theme functions for notifications afaik has nothing to do with privatemsg.

privatemsg provides a query builder and query functions to ease the use of the data. I think what you need to use to get the data for a particular message is:

  $query = _privatemsg_assemble_query('load', $pmid, $account);

and that will provide the relevant data.

Granted, it is not the same as token, but having proper token support will be something that may take a lot of time and still not be used at all since the above methods work.

Allowing privatemsg expose tokens is a very good idea especially because it is a future-proof feature as George2 pointed out. yes some data may easily be reached by using our query builder. however not every data should be pulled that way, specially when generating 1 token and using it in 10 places is more performant than running the same query build 10 times (hypothetical example).

I heard that token is going through some development so unless this feature request is very urgent we should take the time to understand how token module is changing and how it will be in the future before anyone begins work on this.

I am not sure how this would work.

I don't know much about tokens, but afaik, they are always in a context. For example, in a node, and then, you can access various information of that node with tokens.

However, that quote looks more as if you were looking for api functions:

message title, message sender, total unread messages etc instead of having to pull it from the db manually.

And for most of these things, we alread have them..

unread messages: http://blog.worldempire.ch/de/api/function/privatemsg_unread_count/1
loading a message: http://blog.worldempire.ch/de/api/function/_privatemsg_load/1
loading a thread, with recipients and everything: http://blog.worldempire.ch/de/api/function/privatemsg_thread_load/1

And I agree with litwol in #4, that we should wait for tokens 2.0, that will hopefully be part of core too.

Subscribing to this issue and hoping to get it going again (as Berdir suggested in another thread). I'll put some thought into how this should function, UI, etc...

Hey Berdir and everyone,

I did a bit more research and because Token is now included in D7 core (with a lot of documentation in core, too), it's the perfect time to implement Privatemsg support for tokens. There's really two separate issues here:

A. Allowing tokens from core (and contrib modules that supply tokens) to be used within a Private Message.
B. Having Private Message add its own tokens (like message title, unread messages, etc.) to be used by other modules.

Neither looks too difficult to implement, but perhaps we should start with A because it looks especially straightforward. Also, there's more of a need now for A because Berdir is close to finishing support for sending a private message to a role (a token would let you include the user's name in the private message).

So I took a look at the token.inc file found in D7 core (includes/token.inc). It looks like the token_replace() function should do the trick. Here's the example it provides at the top of the file:

 * Tokens follow the form: [$type:$name], where $type is a general class of
 * tokens like 'node', 'user', or 'comment' and $name is the name of a given
 * placeholder. For example, [node:title].
 *
 * In addition to raw text containing placeholders, modules may pass in an array
 * of objects to be used when performing the replacement. The objects should be
 * keyed by the token type they correspond to. For example:
 *
 * @code
 * // Load a node and a user, then replace tokens in the text.
 * $text = 'On [date:short], [user:name] read [node:title].';
 * $node = node_load(1);
 * $user = user_load(1);
 *
 * // [date:...] tokens use the current date automatically.
 * $data = array('node' => $node, 'user' => $user);
 * return token_replace($text, $data);
 * @endcode

Note that D7 core doesn't provide a method to list available tokens for users. However, I found this documentation page (http://drupal.org/handbook/modules/token/update/6/7) that shows how to list available tokens using the D7 version of the Token contrib module (which fills in a few minor gaps in the core implementation):

<?php
// You can call the theme function directly:
theme('token_tree', array('token_types' => array('node')));
// Or use it in a form:
$form['tokens'] = array(
  '#theme' => 'token_tree'
  '#token_types' => array('node'),
);
?>

The cool thing about the above code is that it displays a brand new "token browser" that is much nicer to look at than in D6.

So what do you think?

Cheers,
Ben

Support for A) in Drupal 6 might also be possible through http://drupal.org/project/token_filter.

Thanks for your input, I still need to think about how to implement it with permissions/security in mind. Obviously, not everyone should be allowed to use tokens in private messages.

Hey Berdir,

Yeah, I suppose it's important to think about how the permissions should work for token support. Do you envision creating a "Use tokens in message" permission?

So basically, token_replace would only be executed if a user had this permission?

--Ben

If the *author* (not the current user) has permission, yes.

Attaching a first patch.

- Displays the token browser if the token module is installed
- Replaces tokens in body of messages if the author has the permission

- No support yet for subjects, while it is easy to do when displaying a thread, it is a problem when looking at /messages because we would have to load the author of the message to check the permission so we'd have 20 more user_loads() on that page in the worst case.

#10: tokens.patch queued for re-testing.

Re-roll.

Hey Berdir,

I just did some testing of the patch in #13 and the token functionality is very cool! The permissions worked as expected and I was able to successfully use the following tokens in private messages:

[current-user:name]
[current-user:last-login]
[date:short]
[site:name]
[site:mail]
[site:url]

I also installed the Token module for D7 and the token browser worked great. As an added bonus, the tokens are replaced on message preview, so it's a handy way to make sure that the tokens are working properly.

So in terms of the basic functionality of token replacement, it's working quite well. Rather than bugs per se, I just have a few issues to consider:

A. Because of how token in core works, it seems like all of the available user tokens are related to the currently logged in user. This has strange effects when an administrator with "Read all private messages" permission reads the messages of another user (at user/[uid]/messages). This is because even though the actual message recipient will see their own information merged into the available tokens, the administrator will see the administrator's own information when reading the message.

Even without granting someone "Read all private messages" permission, this could get a little confusing... For instance, if you sent a message with tokens to a recipient and they reply back with questions about the token information they read, the original sender would only see their own account information in the place of recipient's tokens that they are asking about.

So I'm not sure how easy it would be to solve this issue. We're not dealing with a node here, so we don't have tokens for node author or something similar. I suppose one possibility would be if Private Message created its own tokens for the message recipient and the message sender. But I'm not sure how difficult this would be to implement. One the plus side, because of how chained tokens work in core, once we have the message recipient as a token, it should be straightforward to get their username or something similar ([messagerecipient-user:name]). But maybe you have a simpler solution...

B. It isn't apparent to an end-user with the "Use tokens in private messages" permission that he is able to include tokens. For users with that permission, should we print a message that says they are permitted to use tokens in messages? Should the message also say that if they install the Token contributed module for D7, then they can browse available tokens?

C. I've got an idea about how to address tokens in subject lines (given user_load problems at /messages). What if we were to create a checkbox that says "Use token replacement in message subject line." This checkbox would only be visible to those with "Use tokens in private messages" permission and would be unchecked by default (and unchecked for all users without that permission). Then, when displaying subject lines at /messages, if the checkbox were checked, the token replacement would occur without having to do a user_load(). If the checkbox was unchecked, no token replacement of the subject line would be necessary. Anyways, just an idea. What do you think?

By the way, really great work on this functionality. Once we combine tokens with the awesome work you've done on sending messages to roles, it's going to be a super powerful system.

Cheers,
Ben

Hey Sascha,

One other thought: I'd really like to test this token patch with the work you've done on sending messages to a role. Do you think that the role sending for D6 is ready to bring over to the D7 version? Or would you rather wait a bit longer?

Cheers,
Ben

I will port the patch once it has been commited to 6.x-2.x since that will be quite a bit of work. There are many database related chances introduced in that patch and that part of the code is totally different in 6.x and 7.x.

#13: tokens2.patch queued for re-testing.

Hey Berdir,

Did you want me to do more testing on this? I wasn't sure if you read my comments in #14.

Is there a new patch to test or did you want more feedback on the patch in #13?

Thanks,
Ben

Bumping this so I don't lose track of it... next on my agenda.

Sascha,

I tried applying the patch in #13 (so that I could do more testing), but it doesn't successfully apply anymore. Here's the message I got when I tried to patch:

patching file privatemsg.module
Hunk #2 FAILED at 597.
1 out of 2 hunks FAILED -- saving rejects to file privatemsg.module.rej
patching file privatemsg.pages.inc
Hunk #1 FAILED at 335.
1 out of 1 hunk FAILED -- saving rejects to file privatemsg.pages.inc.rej

Can you re-roll?

Thanks,
Ben

I'm not sure how to address A. Adding tokens for the author is pretty easy, but it gets more complicated once we get to the recipients. It is not that hard for a simple message written from user X (author) to Y (recipient), but there can be multiple recipients, an admin can view the message through view all permission and what not...

Thinking about it, I might have an idea....: What about simply only replacing [recipient:*] tokens if the user that is currently viewing the message is a recipient? And if not, use a dummy placeholder or leave the original token visible? We could also add a special case if there is only a single recipient and then always replace the tokens with him, but I'm not sure if that would be confusing if it "suddenly stops working" when you write a message to two recipients.

B. Not sure. It is imho pretty obvious if token.module is installed and without, you'd have to remember the tokens by name, that is hard anyway. Too bad Drupal doesn't support module suggestions. I guess we simply need to document somewhere that tokens are supported (I'd love to have a README.txt, but I don't find the time to write one...) and if one wants to use them, they should install token.module.

C. I guess that would be possible, but it would require to add an additional column to the database that won't be used in most cases. Maybe we can postpone until we have a pm_thread table (there is an issue somewhere, I want to start working on that soon). Right now, the subject is stored for every message and we would first need to figure out which author we actually need here. (Once we add author/recipient tokens, we need to load them anyway).

Ok, after discussing this in IRC, we came to the following conclusion:

- Add recipient/author tokens as suggested above, aka, not replacing recipient unless the current user is one
- Add a has_tokens flag to pm_message. This allows to use tokens in subject (both thread view and message listing)
- Also added a few more tokens, including subject, mid and sent time. We will probably add more when converting the mail notification template to use token instead of the current solution with !.

This is working pretty nicely from what I can see, please test.

Ok, I need to cast the value to an integer. Let's try again.

Just bumping this so it shows at the top of my to-do list... will test soon.

Hey Berdir,

I did a bunch of testing of the patch in #24. First of all, the patch is working pretty well. I send a bunch of messages both to users and to roles with tokens in the main body as well as subject line.

Here are the potential bugs I noticed:

A. If you use the [privatemsg_message:subject] token in the body of your message and your subject also contains a token (such as [privatemsg_message:recipient]), then the token [privatemsg_message:recipient] will appear in the body of your message without being replaced. Put another way, if your subject is "Hey there, [privatemsg_message:recipient]" and you use the [privatemsg_message:subject] in the body of your message, it will appear in the body as "Hey there, [privatemsg_message:recipient]" instead of as "Hey there, User1". Basically, there's an issue with nested tokens.

B. The following token is not working properly for the sender: [privatemsg_message:recipient]. Basically, everything works fine from the recipient's point of view (both in the subject line and in the message). But from the sender's point of view, [privatemsg_message:recipient] is actually being replaced by the date the message was sent. Basically, if I put [privatemsg_message:recipient] in either the subject or body of a message, the sender sees "July 25, 2010 - 12:01pm" as the replacement value when viewing the message (both in preview and when viewing a sent message). I'm not sure why the date is being shown (and not something else), but perhaps it is because the date was also a token in the message? Other recipient tokens seem to be working as expected for the sender (just showing the actual token), so the problem seems related to this particular token. However, it is the only token we see for the recipient without expanding the hidden dropdown (when using the Token module's browser), so it's an important one.

C. These two tokens aren't being replaced at all: [privatemsg_message:recipient:path] and [privatemsg_message:author:path]. I found these tokens using Token module's token browser. Not sure if this is a Private Message bug, a Token module bug, or a D7 core bug.

D. Author tokens in the subject line (such as [privatemsg_message:author]) are not being replaced when previewing a message. They are successfully replaced when a message is sent, but just not during the preview.

Also, my testing of the module brought up some usability and logic issues to consider. Since I use the "Mailchimp" service to send bulk e-mail newsleters (and Mailchimp uses a lot of token replacement), I took a look at how Mailchimp handles things. Here's what Mailchimp does:

* Shows raw tokens to the sender when viewing a previously sent message. Example: [firstname]
* Shows a modified token when previewing a message so you know it's being replaced. Example: << Test firstname >>
* Has a separate link to preview a message with actual user data. Example: Bob

So with this in mind, here are my thoughts:

E. Perhaps we should refrain from replacing any tokens for the sender when viewing a previously sent message. Currently, the recipient tokens are not being replaced, but all other tokens are being replaced. This can be kind of confusing, especially when previewing a message: Why are certain tokens replaced but not other ones? If you inadvertently add a typo to a token (like I did in my testing), it's confusing whether the token just isn't being replaced or if there's an actual error in the token. Additionally, it's difficult to copy a message and use it again (something you might do when sending bulk messages to roles) because you can't see the original tokens you used (other than recipient tokens) once it has been sent. So I'm wondering if we should be consistent with this and just never replace any tokens when the sender is viewing a previously sent message. Thus, he would be able to see the actual tokens he used.

F. When a sender is previewing a message, maybe we should replace all tokens with << Test tokenname >> or something like that. This way, you would know that a token will be replaced when the message is actually sent. If there's a typo in your token, you wouldn't see this "Test" replacement (it would only work for valid tokens). Thus, you would know that something is wrong and that you need to fix something in the token that isn't working properly. And this same test replacement would be used regardless of whether it's an author or recipient token... thus keeping things consistent. Is this feasible? What do you think?

G. I'm not sure how complicated it would be to add a separate "Preview with Token Data" link to preview a message with actual user and site data in place of the tokens. Basically, we could either create a separate permission for this or else limit its usage to someone who has the "Administer Users" permission (since we could be exposing sensitive recipient data by doing this). Because we'd have to account for sending to roles with a lot of users, perhaps it could be a "Preview with Data" link with an autocomplete menu to choose a user to use as the recipient (just for the data preview). Thus, we wouldn't have to load all of the actual recipients. Or alternately, we could just load the first recipient we happen to find in the message to use in the data preview. In any event, I'm not sure how feasible this is. If you think this is too complex to implement, we could postpone this feature and re-visit it at later date. I wouldn't want it to hold up everything else.

So what do you think?

Cheers,
Ben

A. Uff, well, that's not supported by tokens by default, we'd have to do that ourself. Main problem is recursion detection. If you write "This is the [privatemsg_message:subject]" into the subject, that's going to do funny stuff ;)

B. Fixed.

C. I don't have those, maybe another modules adds them? Anyway, not an issue with privatemsg, we simply pass this through to user.module which does the recipient/author replacement stuff for use.

D. Fixed.

E. Valid points, I'm not sure how to address them. If we simply not replace tokens for the author, he doesn't see typos either and might even think that they are not working at all. But I agree, a mix is bad either. Question also is, are these privatemsg specific issues, or would they be similiar for nodes, mails and other places where you can (or will be able to) use tokens? As in, how much special handling to things that could be considered general token flaws do we want to add?

What about simply replacing the [/] with some other characters (<> for example) if the author is viewing a message and add a message at the end what it means? Suggestions are welcome on how such a message could look.

F. Erm, it would help to read your whole comment before responding. See above, doing that always for the author would automatically include preview...

G. Hm.. I like the idea but it could get rather complicated in combination with roles and other recipient types. But should be possible... Probably better as a follow-up feature though, as you suggested.

Patch attached with bugfixes for B and D, other stuff needs more discussion :)

So let's not worry about A and C... those are edge cases, anyway.

Thanks for fixing B & D!

As for E and F, how about always replacing tokens with < Token [tokenname] > (in both preview and previously sent mail).

This way, the tokens would always change, it would be clearly identified with word "Token", and the "< Token" and ">" could be manually deleted (thus leaving "[tokenname]"), if you actually wanted to use the message again.

And let's postpone G for the time being and re-visit later.

Do you want me to test the existing patch or wait until the we've incorporated the added changes?

--Ben

Just bumping this so I don't lost track of it...

Here's a try at implementing the not-replacing of tokens that we discussed.

The problem with it is that we only have control over the tokens that are defined by privatemsg.module. So if you use current user tokens or generic site tokens for example, these are still always replaced.

One way to solve that would be to create our own function that would scan for tokens and replace them with the thing and only call the default function if the user is a recipient.

Anyway, here is a patch for now that can be tested...

Hey Berdir,

I tried out the patch in #30 and it's generally working pretty well. Nice job!

Here are the bugs/issues I noticed:

A. I would add a space between the tokenname and the ending arrow. Currently, it reads like this "< Token [tokenname]>" but I would change it to "< Token [tokenname] >". I think it reads a bit better and there's no ambiguity as to what is part of the token.

B. The [privatemsg_message:recipient] token appears to work in the preview and sent mail, but it doesn't actually work properly for the message recipient. The replacement for this token, as seen the by the message recipient, is currently the word "recipient". This is in contrast to the [privatemsg_message:author] token which works properly and is replaced by the username of the message author.

C. When you preview a message, invalid tokens are still being displayed as if they are correct. The < Token [tokenname] > replacement is occurring, even if the token is wrong or made up. To test this, use [privatemsg_message:author:lastlogin] as a token instead of the correct [privatemsg_message:author:last-login] token. In preview, it looks like the token will be replaced even though it's wrong (and won't be replaced for the recipient). We need some type of validation that only shows the < Token [tokenname] > replacement in preview mode if it's an actual, recognized token.

D. The [current-page:path] token seems to work in the message preview, but doesn't work in the actual message that the recipient sees. I'm not sure if Private Message has any control over this.

As for the point you raised about generic site tokens or current user tokens always being replaced, I don't think that's a huge deal. I certainly wouldn't want us to hold up patch for this. One idea: Could Private Message supply some equivalents of those tokens? This would mean that Private Message would have its own version of generic site tokens or current user tokens. Only Private Message tokens would need a consistent < Token [tokenname] > replacement approach. Thoughts?

--Ben

Uff :) You and your tokens ;)

The problem is that the token api that in core simply doesn't provide that information, so I have to do that all myself and that is quite complicated. Anyway, here is a try.

A: Changed.

B. Fixed.

C: I added a wrapper function that does not actually call token_replace() but calls token_scan() itself (in case the user is not a recipient) and works through the tokens it found. It also validates every token (even recursive ones, so you could have a token like foo:bar:bar:foo:something and it validates part for part. Invalid tokens are displayed like this "< Invalid token @token >"

D: No idea where you got that from, I don't such a token.

Happy testing.

Hey Berdir,

Your new token scanning and validation is working great! We're getting close. Here are the bugs/issues I've noticed when testing your latest patch:

A. For the sender, tokens replacement is working properly in the preview, but not in sent messages. Basically, in sent messages the tokens are being replaced with actual user data instead of with the < Token [tokenname] > formatting. This is a problem both for sending to individual users and sending to roles. Token replacement is working fine for message recipients, however.

B. Also for the sender, recipient tokens are being replaced in sent messages with the message author's data. So not only should tokens in sent messages not be replaced with actual data, but it is being replaced with the wrong user's data (the author instead of the recipient when using recipient tokens).

C. Instead of < Invalid token [tokenname] >, I think it would be better to make "Invalid token" all caps so that it really stands out. So it would read like this: < INVALID TOKEN [tokenname] >. This way, it's hard to miss and the sender can fix. One other option would be to make it a different color via CSS, but I don't think that's really necessary if we make it all caps.

D. This is a little weird: If you use an invalid token with a "?" mark in it, the invalid token is not displayed... just the question mark. For instance, try using this token: [privatemsg_message:sent:custom:?]. Then the recipient and sender sees a "?" as the replaced token.

Let me know if any of this is unclear.... ;-)

--Ben

A + B: Yeah, I forgot that the author is technically also a recipient. Re-added the explicit author check. Should work again.

C: Changed

D: With the update, the author should now see a INVALID TOKEN for that. This seems to be a special feature that allows you to define your own date format as a token. So that you could for example only show the day or month of a date. The real problem here: Because this is a dynamic token, it is not defined and for our validation function, doesn't exist. However, for example "privatemsg_message:sent:custom:d.m", does work perfectly fine for the recipient. We might just have to live with that. I could add an exception but more tokens that do similiar stuff might be added and I can't add an exception for all of them...

- I added a note to the end of a message that has tokens when the author is reading it. Reads "

I also just found out the following problem:

The query to get the thread for listings is quite complex and does lots of grouping and uses many sql expressions. For example, it currently simply checks if any message in a thread has tokens and if yes, uses tokens for the subject. This is because the subject doesn't have it's own flag anyway and I can't get just the one for the row of the select we are using...

Now, consider the following: Someone without permission to use tokens starts a thread with [some:token]. The token is simply not replaced and used as normal text. Now, if someone with permissions answers to that thread and uses a token, it does set the has_tokens property and now, when viewing the thread in the listing, it will replace the token. I have no idea on how to fix this except dropping support for tokens in the subject.

Anyway, attaching an updated patch....

Hey Berdir,

I tried out your patch in #35 and can confirm that items A, B, C, and D are totally fixed.

There's just a few really small stuff left (including solving the problem you raised):

1) All of the "Current user" tokens are showing up as INVALID during preview and in the author's sent mail even though the tokens are being successfully replaced from the recipient's point of view. So the actual "Current user" tokens are working fine if you actually send them... but they look to be INVALID from the point of view of the sender.

2) I like the note you added to the end of a message with tokens (when the author is reading it). I'd probably make that note italic (font-style: italic) so that it stands out a little better from the message text.

3) As for the has_tokens problem you described, here are a couple of possible solutions:

a) Since you're already scanning for valid tokens for users with the "Use tokens in private messages" permission, how about using this scanning function to prevent unpermissioned users from adding actual tokens to the subject line. Basically, if an unpermissioned user tried to include [some:token] in the subject line (which is a valid token if they had permission), then the user would receive an error message saying something like "You are not allowed to use tokens in the subject line or message body." The message wouldn't be sent until they got rid of the token.

b) When replacing a tokenized subject line for a message with the has_tokens property set, we could only replace the subject line if the original thread author also has the "Use tokens in private messages" permission. This would be added overhead for the permission check at /messages, but we'd only have to run this for messages that have the has_tokens property (better than when we had to check all messages). So it would only really become an issue if most of a users messages at /messages were using tokens. But because of this, option a) may be better if it can work.

So what do you think?

That's it! We're oh so close! Looking forward to getting this committed....

--Ben

1) i just had a better idea on how to do the valid/invalid check. Something like this: 1. Scan for tokens in text and store them. 2. Do a normal token_replace() without any special checks, save the replaced text in different variable 3. Loop through the tokens returned by 1. If the token doesn't exist anymore, it is valid, it if is still there, it's invalid. That also works for dynamic tokens and is easier than the current way.

2) sure can do that, it can also easily by overriden by a theme.

3) Actually, I like b) better. Because it is possible that a token is not valid when a message is created but a module that's added later on would make it a valid one => error-prone. a) is a slight performance overhead but shouldn't be such a big issue. I don't think that many users will have that permission and even on a site where an admin sends a newsletter-like message with tokens to all users every week, that's not so problematic because it's probably always the same user and user_load() is statically cached in D7 so the user is only loaded once (And probably already is because he is displayed as a participant).

Sounds great, Berdir. I'm back from my trip and will be working daily on our various issues. I hope you are having a great time in Copenhagen and I'll look forward to reviewing more code on those three points when you have a chance.

--Ben

Ok, changes:

- Made the notice italic
- Added a span with a class on valid/invalid token strings and made valid tokens bold and invalid tokens red. We can discuss this and it can be changed by a theme if it wants to do so (very unlikely :)) but I think it looks pretty nice.
- Implemented the token check as proposed. This should have the advantage that it catches all tokens that are actually replaced, not only those that are defined.

- Haven't done anything on 3. The whole situation with the subject stuff is just too complex at this point to figure it out correctly. I don't think this is critical and we can address it if we can clean that up.

Hey Berdir,

I just tested the patch in #39 and things are working well. I really like the new span class and token check method. All of the current user tokens are fixed. Everything looks quite nice!

And I agree that we should hold off on further subject line changes related to 3... it's not important enough to warrant holding up committing this patch.

So I was hoping to mark this RTBC, but alas, I found a few minor bugs. Here they are:

A. If you use a token in the subject line, the new span classes are actually displaying as text to the sender. For example, this is what is displaying to the sender as the message title when viewing the message thread:

<span class="privatemsg-token-valid">< Token [privatemsg_message:recipient] ></span>

The span classes should be stripped out for the sender (when in the subject line) just as they are for message recipients. The span classes are also being displayed as text at /messages (with the additional problem noted in B below).

B. The message subject at /messages is changing if a recipient replies to a thread. Initially after sending a message with a token in the subject line, the token is being displayed properly at /messages (albeit with the problem mentioned in A above). But if a recipient replies to that message, then the information for the first recipient to reply is being replaced in the subject line.

To recreate:

1) Create an "editor" role and add user1 and user2 to that role.
2) Send a message to the "editor" role with [privatemsg_message:recipient] in the subject line.
3) Look at /messages and the subject line will be displayed properly as < Token [privatemsg_message:recipient] >
4) Login in as user1 and send a reply to the message.
5) Login again as the sender and go to /messages
6) The subject line will now be displayed as "user1" to the original sender
7) Even if user2 also replies, the subject of the message from the point of view of the sender will always now be user1

C. There may be an additional problem (when viewing a sent message) if the token included in the subject line is invalid. For instance, I tried to send this token as the subject line: [privatemsg_message:recipientsss]. And here's what displayed as the subject line to the sender (both at /messages and on the message thread as a sent message):

<span class="privatemsg-token-invalid">< INVALID TOKEN [privatemsg_message:recipientsss] ></span

Note two different problems above. First, there is a missing ">" at the ending span tag. Second, should we be replacing this even when it's invalid? Maybe we should we just display the subject as: [privatemsg_message:recipientsss] with no replacement since it's invalid.

D. In the body of a private message, [privatemsg_message:recipient] is showing up as invalid to the sender (during message preview and as a sent message) even though this is a valid token. From the recipient's perspective, the token is working fine and token replacement is working fine. Interestingly, it doesn't show up as invalid to the sender if it's placed in the subject line... only in the message body.

E. The previous problem with [privatemsg_message:sent:custom:?] not showing up as invalid (and being replaced with a "?" upon sending) has resurfaced. Perhaps because you changed the validation method and this brought back the error? Anyway, it's probably an edge case so we don't necessarily need to worry about this if it's going to slow us down.

Let me know if any of this is unclear.... We're getting very close! :-)

--Ben

P.S. Hope you had a fun time in Copenhagen!

A. Fixed the span stuff.

B. Nice catch, fixed. A bit hacky, but works now...

C. Fixed the missing > (It was always wrong but browser are nice and they parsed and accepted the span anyway. I see what you mean with the second point, but I'm not sure. It looks better now since the span stuff is gone, so I'm leaving it for now.

D. You already reported this before, but it's working perfectly fine for me. See attached screenshot.

E. This is not a bug and it is a valid token. Because you can use whatever you want after custom, the point of that token is that you can define your own date format. And since ? is not a possible replacement character (like d or Y or m), it is simply displayed as ?. If you use the token "[privatemsg_message:sent:custom:d?]", it will be displayed as "(current day)?"· There should be a link in the description of that token that points to the documentation page of date() which contains the possible tokens.

Some tests would be awesome for this, since this issue has shown pretty well that it is not easy to get right and is probably easy to break. I'll see if I can write a few. If you want, you can write down some use cases that I can convert into automated tests. The example in B is great, and there are probably more in this thread so you can also just point me to them since the thread has grown quite a bit.

And yes, Copenhagen was awesome! :)

Erm, would help if I would attached the patch and screenshot...

Hmmm..... D is now working for me, too. So let's not worry about that one. And thanks for the explanation on E... that makes sense. I'll test the rest in the new patch.

--Ben

I'm not exactly sure how to write up a use case for transformation into an automated test, but here goes:

Use Case: Make sure that token permissions are functioning properly.

1. Create user1 and user2
2. Create new role "token sender". Give this role the "Use tokens in private message" permission. Don't give either user this role.
3. Send message from user1 to user2 and include recipient tokens and sender tokens.
4. View message as user2 and confirm tokens are not being replaced.
5. Add user1 to the "token sender" role.
6. Login as user2 and confirm that the tokens are now being replaced.

--Ben

Second use case: Support customer service inquiry.

1. Create two users: user1 and admin1
2. Create a role "customer service". Give this role the "use tokens in private message permission". Give admin1 that role.
3. Send a message from user1 to admin1 with subject "How do I change password" and body "Where are password settings?"
4. Send a reply from admin1 to user1 with message "You can change your password here: [privatemsg_message:recipient:edit-url]."
5. Login as user1 and visit the link supplied by the [privatemsg_message:recipient:edit-url] token.
6. Confirm that this is recipient's account edit page and that password can be changed.

--Ben

I tested the patch in #42 and everything is now working great. I can confirm that A, B, and C has been fixed. Once you add the automated tests, I think this is ready to be committed! Great work!

--Ben

Thanks for the uses cases, that should help. Two minor hints:

- In tests, permissions are usually directly assigned to users. Probably best explained with a code example: "$user1 = $this->drupalCreateUser(array('write privatemsg', 'read privatemsg', 'access user profiles'));". This will automatically create a new role with the specified permissions and add them to the created user. So you can leave out that step and simply write "create a user with permission X, Y and Z" :)

- Note that the use case in #44 doesn't actually work like that, existing messages aren't affected when you give a user the tokens permission because of the has_tokens flag.

And... Yay RTBC! :) Will commit this once I've written some tests.

Thanks for all your hard testing, I've commited this together with a initial test case that covers most of the issues we had while implementing this.

Maybe I'll backport backport this to 6.x-2.x, or maybe someone else wants to do that. The tests to verify that it does work correctly are attached.

Note that I did some minor changes to the implementation as well (Mainly, use </> for when displaying HTML).

This has been commited to 7.x, as the comment in #48 says.

This has been marked as a potentional backport to 6.x-2.x if someone will port the patch.

Since this is in 7.x and 6.x is not longer supported, I'm closing this.