This patch adds three things which are necessary for #864542: Replace old-style tokens with new token integration.
- Drops the custom sql query builder, load and load hook in favor of using a controller class
- Provides an URI callback that can build the path to a message
- Provides body, url and thread_id tokens
This doesn't have an direct visual changes, this is more a backend thing ;) However, testing is very important here. Tests passed for me locally, let's see what the bot has to say.
One way to actually test the URL stuff is to either use the new tokens or to build a rule that grants userpoints when a private message is sent and privatemsg_message is used as the referenced entity. Without this patch, the reason column is not linked to the message, with this patch, it is :)
| Comment | File | Size | Author |
|---|---|---|---|
| #20 | loading_tokens_uri10.patch | 15.24 KB | berdir |
| #19 | loading_tokens_uri9.patch | 14.73 KB | berdir |
| #17 | loading_tokens_uri8.patch | 13.92 KB | berdir |
| #14 | loading_tokens_uri7.patch | 11.18 KB | berdir |
| #12 | loading_tokens_uri6.patch | 10.95 KB | berdir |
Comments
Comment #1
BenK commentedSubscribing to help with testing...
Comment #3
berdirUpdated patch, improved tokens.
Comment #5
berdir#3: loading_tokens_uri2.patch queued for re-testing.
Comment #7
berdirTrying to figure out what the actual exception is, see also #947874: Simpletest uses %message instead of !message for uncaught exceptions
Comment #8
berdirGrml, new try.
Comment #10
berdirNew patch without the test changes, these were commited in another issue.
Comment #12
berdirGrml. Debugged this literally for hours. Turns out, I just forgot to pass the constructed $conditions array to privatemsg_user_load_multiple(). This will be green!
Comment #14
berdirUh, left in a debug call.
Comment #15
BenK commentedI tested using the following tokens in a private message: body, url and thread_id. Here's what I noticed:
a) All three tokens displayed as invalid to the sender, but they seemed to be replaced fine for the recipient.
b) For the body token, all carriage returns and line spacing seems to be missing from the message text. Basically, the body token appears to put the body text in one big paragraph that is smushed together.
I'll now test using the rules method you mentioned above.
--Ben
Comment #16
BenK commentedI did the following rules test just as you suggested:
It worked exactly as you described, with the reason link activated once I used the patch.
--Ben
Comment #17
berdira) Only body shows up as invalid for me. I guess you used the body token inside the message body? That was a problem (it doesn't make sense anyway :)) with our token validation, I replaced it with the function provided by token.module now. That means that invalid token detection only works when token module is installed.
b) This is a tricky problem and we will have to change this somehow when mail tokens come into play. I formatted the body now according to the defined text format.
- I also noticed that entity.module does expose the defined properties as tokens. Some of them were actually showing up twice because of that. I fixed that by renaming our tokens so that they have the same name as the property (sent => timestamp, thread_id => thread-id). This would break existing messages that used the sent token, but I doubt that there are any :)
Comment #19
berdirOk, the test needs token.module now to work correctly.
This might work...
Comment #20
berdirOk, discussed this with boombatower, optional test dependencies are not supported yet, this is being worked on in #102102: Parse project .info files: present module list and dependency information. We need to add a test dependency to the .info and it should start working when the linked issue is resolved. Until then, the token test case will be silently ignored.
Comment #21
BenK commentedAll reported issues on this thread are fixed for me. I'm marking this RTBC, but you can change the status if modifications will still need to be made to work in conjunction with the e-mail notify thread.
--Ben
Comment #22
berdirOk, committed. We can always fix bugs in the notify, I just wanted to split this (which is working fine for the most part) from the tricky notification stuff.