phplist

NOTE:: Before reporting an issue, make sure you are running the latest version, currently 3.3.1


View Issue Details Jump to Notes ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0016707phplist applicationMessage Managementpublic31-10-12 14:1522-05-16 14:16
Reporterduncanc 
PrioritynormalSeverityminorReproducibilityalways
StatusresolvedResolutionfixed 
PlatformOSOS Version
Product Version2.11.7 
Target Versionnext minorFixed in Version3.2.5 
Summary0016707: Difficult to use admin attributes as placeholders
DescriptionIt is currently very difficult to use admin attributes as placeholders in a message. It doesn't appear to be documented but seems to work this way:
include ##LISTOWNER=n in the body of the message to identify the admin whose attributes are to be used. n is the id of the admin.
use the syntax [LISTOWNER.attribute] as the placeholder in the body of the message

I don't know the background to why it is like this, it might have something to do with prepared messages, but it doesn't seem very intuitive. I suggest that by default the attributes of the message owner should be used.

In file admin/sendemaillib.php lines 1422 - 1427 are
  if (preg_match("/##LISTOWNER=(.*)/",$cached[$messageid]['content'],$regs)) {
    $cached[$messageid]['listowner'] = $regs[1];
    $cached[$messageid]['content'] = str_replace($regs[0],"",$cached[$messageid]['content']);
  } else {
    $cached[$messageid]['listowner'] = 0;
  }

The regular expression is incorrect as it includes everything up to the end of the line. The default FCK editor adds some html to each line making it difficult to enter exactly the right text. You need to go to html source mode to enter exactly, say, ##LISTOWNER=2 with no surrounding html.
As the intention is to extract the admin user id, the regexp should try to match a sequence of digits.
Also the match can be made case insensitive, so the line can be changed to:
  if (preg_match("/##LISTOWNER=(\d+)/i",$cached[$messageid]['content'],$regs)) {

To make the message owner's attributes be used as default the else can be change to
    $cached[$messageid]['listowner'] = $message["owner"];

Consequently lines 378-383 can be changed. They are currently
  if (!empty($cached[$messageid]['listowner'])) {
    $att_req = Sql_Query("select name,value from {$GLOBALS["tables"]["adminattribute"]},{$GLOBALS["tables"]["admin_attribute"]} where {$GLOBALS["tables"]["adminattribute"]}.id = {$GLOBALS["tables"]["admin_attribute"]}.adminattributeid and {$GLOBALS["tables"]["admin_attribute"]}.adminid = ".$cached[$messageid]['listowner']);
    while ($att = Sql_Fetch_Array($att_req)) {
      $htmlmessage = preg_replace("#\[LISTOWNER.".strtoupper(preg_quote($att["name"]))."\]#i",$att["value"],$htmlmessage);
    }
  }
As $cached[$messageid]['listowner'] will now always be set then the outer if statement can be removed.
Again the match can be made case insensitive, so line 381 can be changed to
      $htmlmessage = preg_replace("#\[LISTOWNER.".preg_quote($att["name"])."]#i",$att["value"],$htmlmessage);

Two similar lines in file admin/lib.php would then need to be changed in line:
line 505
      $template = preg_replace("#\[LISTOWNER.".strtoupper(preg_quote($att["name"]))."\]#",$att["value"],$template);
line 536
    $template = preg_replace("#\[LISTOWNER.".strtoupper(preg_quote($att["name"]))."\]#",$att["value"],$template);


The same code is used in the 2.10.x version but had been restructured for 2.11.7.
Steps To ReproduceCreate an admin attribute called say name and populate the value for the first administrator.
Include that as a placeholder in a message

##LISTOWNER=1
[LISTOWNER.NAME]

I get a mysql error on the syntax of the query and the placeholder is not replaced.
TagsConfiguration and sending
Attached Files

- Relationships
related to 0018121new LISTOWNER attribute when listowner is "admin" not filled untill list ownership re-saved 

-  Notes
(0051868)
michiel (manager)
07-11-12 11:45


hmm, yes, that can do with a review. It's a bit of a leftover of the "prepare message" era, which is now completely obsolete.

But being able to have admin details as placeholders is still fairly useful and sensible.
(0051902)
michiel (manager)
14-11-12 16:33

It may be better to completely avoid the ##LISTOWNER=1 code and simply remember the "listowner" from the database.

The "##LISTOWNER=1" was added in the "Prepare a message" system, which is disfunctional. It was never meant to be added manually by the person who is sending the campaign.

The main issue is how to handle this. It used to be such that say a superuser could send a campaign to a list owned by "Subadmin A" and the details of "Subadmin A" would be included in the mails. I guess that would work if the campaign is sent to one list only. As soon as you send it to more than one lists, there's the chance that list A is owner by Subadmin A and list B by Subadmin B, in which case you'd have to reparse the message when sending. As the message is cached, that's not great for performance.

I think we can do it like this:

IF and only IF a campaign is sent to one list, we look up the "owner" of the list (which is set on list level) and allow [ADMIN.NAME] type placeholders.

What do you think?
(0051909)
duncanc (developer)
16-11-12 19:20

That would rule out what I think is a common scenario. An admin sending a message to more than one list each of which he owns.

I had a quick look at the code in actions/processqueue.php for 2.11.7 and couldn't see a point at which the processing knows all the lists to which the message is being sent. The query at line 720 selects users who belong to any list to which the message is being sent, so there would need to be extra processing there to get the list ids from the retrieved user ids.

Alternatively, if the query is changed to return user ids in list order then there wouldn't be much overhead in processing the placeholders for each message but only when there is a change of list id.
However, as I don't know the code that might be more difficult than I imagine.
(0051910)
duncanc (developer)
16-11-12 22:35

"processing the placeholders for each message " should be
"processing the placeholders for each user"
(0051984)
michiel (manager)
26-03-13 13:13


removing from 2.11.8 target, as it's not stabilisation, but more feature enhancement.
(0052056)
michiel (manager)
30-05-13 00:56

ok, so the spec becomes, "if the campaign is sent to lists owned all by the same admin, we can add admin placeholders that will be parsed"

I recently moved the parsing of admin attributes to the pre-caching stage, which means it's no problem for performance.

will mark it for the next dev, and hope to find some time for it.
(0057460)
duncanc (developer)
17-01-16 22:31

See GitHub pull request https://github.com/phpList/phplist3/pull/42 [^]
(0057472)
michiel (manager)
21-01-16 18:00

merged. Nice update, would be good to document this.
(0057473)
gingerling (administrator)
21-01-16 18:09

anyone have any preference over where the docs go? Is it simple enough for main manual attributes chapter? or more for recources.phplist.com?
(0057494)
duncanc (developer)
08-02-16 13:23

I hadn't realised that my change would replace placeholders only in the message, and not in the template.
There is a new pull request to replace in the combined template/message, and add placeholders for the message owning admin - https://github.com/phpList/phplist3/pull/45 [^]
(0057533)
michiel (manager)
22-02-16 21:34

PR merged
(0057683)
gingerling (administrator)
28-04-16 13:42

Hi

I have made an admin, made a list and set it owned by them, made an admin attribute for FIRSTNAME and given the admin an entry for that attribute.

So far though [ADMIN.FIRSTNAME] and [LISTOWNER.FIRSTNAME] just print as is in mails.

Can either of you spare 5 mins to talk me through this feature? I would like to blog it up as documentation and talk about it in the release notes.


Copyright © 2000 - 2017 MantisBT Team
Powered by Mantis Bugtracker