This mail is an automated notification from the bugs tracker
of the project: Savane.
/**************************************************************************/
[bugs #314] Latest Modifications:
Changes by:
Yves Perrin <[EMAIL PROTECTED]>
'Date:
Mon 03/29/2004 at 08:27 (GMT)
------------------ Additional Follow-up Comments ----------------------------
Hi,
Although I don't claim this piece of code is a model of
programming, I don't quite understand the real/potential
bug you found in it.
Here is the logic:
$notif_scope holds the value of the radio button displayed
on the Administration/Email Notification Settings form
It can have three on only three values:
'global', 'category', 'both'
The variable $notif_value holds the numeric equivalent of
the radio button string value and actually is the value
which is kept in the 'tracker_name'._glnotif field of the
'groups' table.
Now for what concerns the few lines you found confusing/buggy:
if ($notif_scope != "global") {
if ($notif_scope == "category") {
$notif_value = 0;
}
if ($notif_scope == "both") {
$notif_value = 2;
}
} else {
$notif_value = 1;
}
it is just an if statement with an other two ifs in the 'if' block:
In other words:
IF $notif_scope is NOT EQUAL to 'global',
we check consecutively if is EQUAL to either 'category' or 'both'
and set $notif_value accordingly
ELSE (meaning it is EQUAL to 'global')
we set $notif value = to 1
Maybe this way of coding without using a suite of if elseif elseif
might be confusing to you. It is just that personally I am more
at ease with the way I coded it.
Now if there is a fundamental logic error in it please let me know
and we will fix it.
For what concerns checking whether a notification e-mail address is set
this is done in the function: trackers_data_get_item_notification_info
in the same file data.php when building the appropriate list of people
to be notified depending on the tracker settings and the item category.
In my opinion the type of notification to be done (global/category/both)
has to be kept separately from the fact corresponding email addresses
have been defined or not. It is the piece of code which build the
mail recipients list to check these various piece of information and
act accordingly.
I hope these few lines of explanation will help understanding.
I imagine this was triggered by observing that tracker notification
was not done properly in certain cases.
- Was this before or after the bug fix [bug #273] I uploaded in the repository?
- Was that fix put online?
Cheers,
Yves
/**************************************************************************/
[bugs #314] Full Item Snapshot:
URL: <http://gna.org/bugs/?func=detailitem&item_id=314>
Project: Savane
Submitted by: Sylvain Beucler
On: Fri 03/26/2004 at 07:15
Category: Web Frontend
Severity: 1 - Trivial
Priority: C - Normal
Resolution: Fixed
Assigned to: yeupou
Status: Closed
Release:
Planned Release: 1.0.2
Summary: global notification settings cancel notification
Original Submission: In www/include/trackers/data.php:
if ($notif_scope != "global") {
if ($notif_scope == "category") {
$notif_value = 0;
}
if ($notif_scope == "both") {
$notif_value = 2;
}
} else {
$notif_value = 1;
}
should be
if ($notif_scope != "global") {
if ($notif_scope == "category") {
$notif_value = 0;
}
if ($notif_scope == "both") {
$notif_value = 2;
}
else {
$notif_value = 1;
}
}
In the first version, the 'else' part is never reached, and causes projects
with global notification settings to have their _glnotif field at 0 in the
database, preventing tracker notification.
It is still not perfect because it does not check whether a notification e-mail
address is set (otherwise it should stay at 0), but at least it works :)
Follow-up Comments
------------------
-------------------------------------------------------
Date: Mon 03/29/2004 at 08:27 By: ype
Hi,
Although I don't claim this piece of code is a model of
programming, I don't quite understand the real/potential
bug you found in it.
Here is the logic:
$notif_scope holds the value of the radio button displayed
on the Administration/Email Notification Settings form
It can have three on only three values:
'global', 'category', 'both'
The variable $notif_value holds the numeric equivalent of
the radio button string value and actually is the value
which is kept in the 'tracker_name'._glnotif field of the
'groups' table.
Now for what concerns the few lines you found confusing/buggy:
if ($notif_scope != "global") {
if ($notif_scope == "category") {
$notif_value = 0;
}
if ($notif_scope == "both") {
$notif_value = 2;
}
} else {
$notif_value = 1;
}
it is just an if statement with an other two ifs in the 'if' block:
In other words:
IF $notif_scope is NOT EQUAL to 'global',
we check consecutively if is EQUAL to either 'category' or 'both'
and set $notif_value accordingly
ELSE (meaning it is EQUAL to 'global')
we set $notif value = to 1
Maybe this way of coding without using a suite of if elseif elseif
might be confusing to you. It is just that personally I am more
at ease with the way I coded it.
Now if there is a fundamental logic error in it please let me know
and we will fix it.
For what concerns checking whether a notification e-mail address is set
this is done in the function: trackers_data_get_item_notification_info
in the same file data.php when building the appropriate list of people
to be notified depending on the tracker settings and the item category.
In my opinion the type of notification to be done (global/category/both)
has to be kept separately from the fact corresponding email addresses
have been defined or not. It is the piece of code which build the
mail recipients list to check these various piece of information and
act accordingly.
I hope these few lines of explanation will help understanding.
I imagine this was triggered by observing that tracker notification
was not done properly in certain cases.
- Was this before or after the bug fix [bug #273] I uploaded in the repository?
- Was that fix put online?
Cheers,
Yves
-------------------------------------------------------
Date: Sat 03/27/2004 at 10:41 By: yeupou
You are right, but I do not now the specific enough to explain this code. Maybe
Yves could help.
-------------------------------------------------------
Date: Sat 03/27/2004 at 09:58 By: beuc
if ($notif_scope == "category") {
$notif_value = 0;
}
elseif ($notif_scope == "both") {
$notif_value = 2;
}
else { // $notif_scope == "global" or empty... or any other value
$notif_value = 1;
}
would be more concise.
It is a bit strange that $notif_scope can be empty or "global". I should test
in which case $notif_scope is set to those values.
-------------------------------------------------------
Date: Fri 03/26/2004 at 17:40 By: yeupou
Hum, the else should be reached if $notif_scope == global. I do not remember at
all the specifics of that issue, but it does make sense to me to have this
ending else.
However, if I correctly understood what you mean, considering we are in the
global notif scope, we should get the value set to 1 if we are not in
"category" or "both" case.
So in the end, it would looks like :
if ($notif_scope != "global")
{
if ($notif_scope == "category")
{ $notif_value = 0; }
if ($notif_scope == "both")
{ $notif_value = 2; }
else { $notif_value = 1; }
}
else
{ $notif_value = 1; }
Tell me if you think I'm wrong on that.
-------------------------------------------------------
Date: Fri 03/26/2004 at 07:19 By: beuc
Sorry, I meant:
if ($notif_scope != "global") {
if ($notif_scope == "category") {
$notif_value = 0;
}
elseif ($notif_scope == "both") {
$notif_value = 2;
}
else {
$notif_value = 1;
}
}
CC List
-------
CC Address | Comment
------------------------------------+-----------------------------
ype | usually works on the notification stuff
For detailed info, follow this link:
<http://gna.org/bugs/?func=detailitem&item_id=314>
_______________________________________________
Message sent via/by Gna!
http://gna.org/
