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

@jeanyves: will you look into this or do we need to find another volunteer?

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 6 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 6 years ago by crumbking

While we work on it, let's get it into build/admin/newsletter ??? What do you think? ;-)

comment:6 Changed 6 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: Changed 6 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 6 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 6 years ago by globetrotter_tt

  • Owner set to globetrotter_tt
  • Status changed from new to assigned

comment:10 in reply to: ↑ 7 Changed 6 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: Changed 6 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? :(

Last edited 6 years ago by jsfan (previous) (diff)

comment:12 Changed 6 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.

Last edited 6 years ago by globetrotter_tt (previous) (diff)

comment:13 Changed 6 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 6 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 6 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

Last edited 6 years ago by globetrotter_tt (previous) (diff)

comment:16 Changed 6 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:19 Changed 6 years ago by jsfan

Deployed on alpha. :)

comment:20 Changed 6 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: Changed 6 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)?

Last edited 6 years ago by jsfan (previous) (diff)

comment:22 in reply to: ↑ 21 Changed 6 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

Note: See TracTickets for help on using tickets.