View Issue Details

IDProjectCategoryView StatusLast Update
0016940phplist applicationMessage Managementpublic19-07-14 16:24
Reporterduncanc 
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Version3.0.5 
Target Version3.0.XFixed in Version3.0.7 
Summary0016940: Row added to listmessage for list 0 when composing a message
DescriptionWhen composing a message and selecting the lists to send to, a row is added to listmessage table for list 0 in addition to the selected lists.

It seems to be caused by this hidden input field
<input type="hidden" name="targetlist[unselect]" value="-1" />

function setMessageData() in file admin/lib.php probably needs to explicitly ignore $value['unselect']. See the foreach at line 65.
TagsNo tags attached.

Activities

michiel

31-10-13 23:30

manager   ~0052389


but are there any undesired side effects ?

The reason is that without it, you cannot uncheck all lists.

duncanc

01-11-13 08:35

developer   ~0052391

Last edited: 01-11-13 08:36

View 2 revisions

I understand the need to have a hidden checkbox field, but the current result means that any code such as a plugin, which is how I noticed this, needs to be aware of the listmessage table having these extra rows for list 0.
So a feature to show all lists that a message has been sent to becomes more complicated.

If the hidden value is changed from -1 to 0, which is an equally invalid list id, then array_filter can be used to remove the unwanted value.

In file lib.php change line 65 to
      foreach(array_filter($value) as $listid => $val) {

in file connect.php change line 1109 to
  
  • <input type="hidden" name="'.$fieldname.'[unselect]" value="0" /><input type="checkbox" name="'.$fieldname.'[all]"';

  • michiel

    01-11-13 11:34

    manager   ~0052394

    problem is 0 as a value stops it from being filled in the $_POST array, which is why I made it -1

    I think the best thing is to not store it in the DB as you write in the original report.

    duncanc

    01-11-13 11:50

    developer   ~0052395

    Do you mean that the browser doesn't send a hidden field with a 0 value?
    That is not the case. The changes that I outlined do work.

    If the only reason for the targetlist[unselect] field is to ensure that $_POST['targetlist'] is populated then ideally it should be removed as early as possible in the processing so that other code does not have to be aware of it.

    michiel

    01-11-13 12:37

    manager   ~0052397

    Ah, no, you're right, I'm mistaken to think that. Yes, indeed, but where to remove it then. The idea is that the listSelectHTML function can be called with any fieldname.

    To centrally remove any POST with index "unselect" can also have unexpected results.

    I guess for now, your suggestion to " function setMessageData() in file admin/lib.php probably needs to explicitly ignore $value['unselect']. See the foreach at line 65." is the best option.