Opened 7 years ago
Closed 6 years ago
#1653 closed bug (fixed)
Admin newsletter tool does not work properly
Reported by: | globetrotter_tt | Owned by: | globetrotter_tt |
---|---|---|---|
Priority: | blocker | Milestone: | 0.9 |
Component: | BW Admin | Keywords: | newsletter, adminmassmail |
Cc: | jeanyves, planetcruiser |
Description
The tool for sending out newsletter (bw/adminmassmail.php) does not work properly.
Issues so far:
- Problems with not escaped queries when ' in group names is used
- Not every member received the last newsletter. According to JY it was sent only to 8796 from more than 19000 members
Change History (27)
comment:1 Changed 7 years ago by planetcruiser
- Cc jeanyves added; jeany-yves removed
comment:2 Changed 7 years ago by jeanyves
Meinhard, I will look at it, within the next week and will propose a patch.
comment:3 Changed 7 years ago by jeanyves
To fix the bug with groups name having some quote inside
in htdocs/bw/layout/adminmassmails.php
1) find the line (line 163 or near of it): echo ">",$TGroupList[$ii]->Name,":",ww("Group_".$TGroupList[$ii]->Name) ;
2) replace it with echo ">",$TGroupList[$ii]->Name,":",$TGroupList[$ii]->Name ;
The group name will not be translated anymore but will be perfectly readable.
The origin of this situation is that I initially designed group name and description to be translatable (ie words code without ' ) but when it has been improved this was forgotten and the way group are now created allows people to enter a real group name where the old bw was designe to create a translation and add a code has group name.
The above fix will not damage anything
For the other problem (only half members enqueud in the delivery list) I need to figure how it happen. I suspect some buggy members or some buggy member-location fix which has been fix by now
comment:4 Changed 7 years ago by globetrotter_tt
- Milestone changed from unassigned to 1.0
Moved in to 1.0 that we don't forget about this.
comment:5 Changed 7 years ago by crumbking
While we work on it, let's get it into build/admin/newsletter ??? What do you think? ;-)
comment:6 Changed 7 years ago by planetcruiser
- Milestone changed from 1.0 to unassigned
globi: is this needed and urgent? if yes, assign to 0.9
crumbking: i'd say we should just fix it in the existing code. migrating this to mvc will be a lengthy nightmare
comment:7 follow-up: ↓ 10 Changed 7 years ago by planetcruiser
- Milestone changed from unassigned to 0.9
oops, just noticed the "blocker" priority. ;)
globi: when will the next newsletter go out?
comment:8 Changed 7 years ago by crumbking
http://bw/bw/admin/adminmassmails.php?action=createbroadcast
While creating a new newsletter I get this error:
#0 debug( query problem with insert into words(code,ShortCode,IdLanguage,Sentence,updated,IdMember,Description) values('BroadCast_Title_NewsAugust2012','en',0,'Hello guys',now(),1,'This will be the August Newsletter') mysql_error: Duplicate entry '0' for key 'PRIMARY' ) called at [bw/htdocs/bw/lib/bwdb.php:126] #1 sql_query(insert into words(code,ShortCode,IdLanguage,Sentence,updated,IdMember,Description) values('BroadCast_Title_NewsAugust2012','en',0,'Hello guys',now(),1,'This will be the August Newsletter')) called at [bw/htdocs/bw/admin/adminmassmails.php:188] query problem with insert into words(code,ShortCode,IdLanguage,Sentence,updated,IdMember,Description) values('BroadCast_Title_NewsAugust2012','en',0,'Hello guys',now(),1,'This will be the August Newsletter') mysql_error: Duplicate entry '0' for key 'PRIMARY' System error, please report the following timestamp along the error: [1343736607]
Anyone any suggestion what's the problem?
comment:9 Changed 7 years ago by globetrotter_tt
- Owner set to globetrotter_tt
- Status changed from new to assigned
i commited the patch in
http://gitorious.org/bewelcome/rox/commit/f273cbd6504524378a9bb218c072b88dbd4657e7
comment:10 in reply to: ↑ 7 Changed 7 years ago by globetrotter_tt
Replying to planetcruiser:
oops, just noticed the "blocker" priority. ;)
globi: when will the next newsletter go out?
The next newsletter should go out this August. A draft is already written: http://www.bewelcome.org/wiki/Workpage_Newsletter_2012
Does anyone know how to test sending out newsletters locally?
comment:11 follow-up: ↓ 14 Changed 7 years ago by jsfan
Not sure how to test this. I can create a newsletter but I have no clue how to send it.
If I click on "prepare enqueue", I get a page where I can fill out a few fields but can't submit. How does this work? :(
comment:12 Changed 7 years ago by globetrotter_tt
I think you need to click on "Trigger mass mails" and then mailbot should do the rest.?
But to be really sure you better ask Jean-Yves
Maybe you also need to search for the identifier of the NL and edit it in AdminWords.
comment:13 Changed 7 years ago by globetrotter_tt
Can it be that we have a broken words table in the test database?
I tried to create a new newsletter and get the same error as crumbking described above.
comment:14 in reply to: ↑ 11 Changed 7 years ago by globetrotter_tt
Replying to jsfan:
Not sure how to test this. I can create a newsletter but I have no clue how to send it.
If I click on "prepare enqueue", I get a page where I can fill out a few fields but can't submit. How does this work? :(
I think i figured it out. You need to give the user
- right: MassMail
- level:1
- scope: "test","Send","enqueue"
using the AdminRights page bw/admin/adminrights.php
comment:15 Changed 7 years ago by globetrotter_tt
To get rid of the error message i did some modification on the words table:
ALTER TABLE `words` CHANGE `id` `id` INT( 11 ) NOT NULL AUTO_INCREMENT
But i still think we need to change the query. When i tried to sent a newsletter to all members in my testdatabase, this query is used:
select members.id as id,Username,cities.IdCountry,members.Status as Status from members,cities where members.IdCity=cities.id and members.Status='Active'
I get 0 results
comment:16 Changed 7 years ago by mahouni
Is that cities.IdCountry? necessary? Maybe try to change the code so that it uses this query:
select members.id as id,Username,members.Status as Status from members where members.Status='Active';
comment:17 Changed 7 years ago by globetrotter_tt
changed the queries in
https://gitorious.org/bewelcome/rox/commit/d352192005c566ef78eff3f5e198cf8873fea97f
comment:18 Changed 7 years ago by globetrotter_tt
small layout changes in
https://gitorious.org/bewelcome/rox/commit/f3fd56afbfd8cf008b9f0569d405cd12987793c9
comment:19 Changed 7 years ago by jsfan
Deployed on alpha. :)
comment:20 Changed 7 years ago by planetcruiser
needs testing in "dry mode" on live database. for example by modifying the line that passes the mails on to the mta and make it write to a debug log instead
comment:21 follow-up: ↓ 22 Changed 7 years ago by jsfan
So, how do we go about this? Do we make a temporary change to alpha when the newsletter is due and then send it via www after successful tests on alpha (and revert the change on alpha, of course)?
comment:22 in reply to: ↑ 21 Changed 7 years ago by planetcruiser
Replying to jsfan:
So, how do we go about this. Do we make a temporary change to alpha when the newsletter is due and then send it via www after successful tests on alpha (and revert the change on alpha, of course)?
ideally there would be no fiddling in working copies, but a config switch for "newsletter dry mode", which has different settings between alpha and www.
i am going out now for a while and try to work on this once i am back in. in about 8h or so
comment:23 Changed 6 years ago by jsfan
Well, that sounds really good but don't forget that this is bw/ code. Really, this should probably be rewritten in build/admin with the functionality you suggest. I'm a bit reluctant to do more than simple hacks when it comes to bw/...
comment:24 Changed 6 years ago by planetcruiser
will wrangle with the /bw beast a little bit now. let's hope adding a config switch is not such a big deal ;)
comment:25 Changed 6 years ago by planetcruiser
reviewing the functionality revealed that there is a "test" mode already, which essentially is a "dry run" for the newsletter. using this test mode on a newsletter created on the live database should show different recipient counts between alpha and www now that the sql queries have been fixed for alpha.
@globi: could you test this please and close the ticket if the test mode on alpha shows the correct member count?
comment:26 Changed 6 years ago by planetcruiser
added a small bonus fix that will help the testing: https://gitorious.org/bewelcome/rox/commit/e960c8d8926a1743fbbd38d228c941bd582e428f "add possibility to hide recipient list when testing newsletter - saves waiting time"
:)
deployed to alpha.
comment:27 Changed 6 years ago by planetcruiser
- Resolution set to fixed
- Status changed from assigned to closed
testing the april newsletter now says: "This newsletter will be sent to 19447 members" - looking good. :)
i am closing this ticket for now. re-open if the next newsletter still shows similar issues
@jeanyves: will you look into this or do we need to find another volunteer?