517012 - MoMo Test Install of SUMO 1.3.0.1 can't create articles - get db query error

Status: VERIFIED FIXED

(support.mozilla.org :: General, defect, P1)

Description

[Roland Tanglao needinfo please :rolandtanglao, :mohnkuchen, :adobo, :sinigang, :roland]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.3) Gecko/20090824 Firefox/3.5.3
Build Identifier: 

When I try to create a new KB article on the momo test install, I get a db query error

Reproducible: Always

Steps to Reproduce:
1. create a new KB article: https://sumo-preview.mozillamessaging.com/tiki-editpage.php?locale=en-US&page=Cannot+receive+email+something+new
2. Fill in the fields as per a normal KB article
3. Click Save
Actual Results:  
An error occured in a database query!

Context:
File	tiki-editpage.php
Url	tiki-editpage.php?locale=en-US&page=Cannot+receive+email+try2
Query:
insert into tiki_polls(`title`,`active`,`publishDate`,`voteConsiderationSpan`,`votes`,`pollQuestionId`) values(?,?,?,?,?,?)
Values:
0	Cannot receive email try2
1	o
2	1253122377
3	60
4	0
5	NULL
Message:
Column 'pollQuestionId' cannot be null
Built query was probably:
insert into tiki_polls(`title`,`active`,`publishDate`,`voteConsiderationSpan`,`votes`,`pollQuestionId`) values('Cannot receive email try2','o','1253122377','60','0',NULL)

Expected Results:  
Article gets created

Comment 1

[Roland Tanglao needinfo please :rolandtanglao, :mohnkuchen, :adobo, :sinigang, :roland]

The bug appears to be triggered by the category "Knowledge Base"
Creating an article with the default category "Staging area" works. Creating or modifiying an article with "Knowledge Base" triggers the bug.

Revised steps to reproduce:
1. create a new KB article:
https://sumo-preview.mozillamessaging.com/tiki-editpage.php?locale=en-US&page=Cannot+receive+email+something+new
2. Fill in the fields as per a normal KB article, Select "Knowledge Base" and unselect "Staging area"
3. Click Save

Comment 2

[Roland Tanglao needinfo please :rolandtanglao, :mohnkuchen, :adobo, :sinigang, :roland]

fixed by disabling polls, so the real bug is that polls are broken and polls are not disabled by default, so marking as dup of polls bug 457330

Comment 4

[E. Cooper [:ecooper]]

This is not a duplicate of bug 457330. The issue lies with hardcoded poll names in webroot/poll_categorize.php and/or missing seed data for the poll tables in bug 502089 during the automatic creation of the CSAT polls for KB articles.

See <http://viewvc.svn.mozilla.org/vc/projects/sumo/trunk/webroot/poll_categorize.php?r1=8610&r2=18573> for the Mozilla specific code. 

This is an and/or sort of situation because I don't know how these auto-created polls were supposed to fit in with the productization effort.

Comment 5

[Laura Thomson :laura]

We need to fix this on the Fennec branch ASAP if we can repro there, but it doesn't block SUMO 1.4, targeting 1.4.1

Comment 6

[Paul Craciunoiu [:paulc]]

I've seen this problem for moving articles to KB while testing the schema.

Comment 7

[Paul Craciunoiu [:paulc]]

Is this fixed with the polls SQL?

Comment 8

[[:Cww]]

No, but it's fixable now.  (Before we didn't even have questions to link the CSAT polls to, now we do.)

Comment 10

[Paul Craciunoiu [:paulc]]

Was the SQL for polls run on stage?

Comment 11

[Philippe M. Chiasson (:gozer)]

(In reply to comment #10)
> Was the SQL for polls run on stage?

The only SQL script I ran was 'sumo_clean_schema.sql'

Comment 12

[[:Cww]]

https://bugzilla.mozilla.org/show_bug.cgi?id=519621 has the sql

Comment 13

[Philippe M. Chiasson (:gozer)]

SQL executed on momo staging. Roland, can you see if this fixes it?

$> wget -q -O- https://bug519621.bugzilla.mozilla.org/attachment.cgi?id=403699 | mysql tiki

If that does fix it, shouldn't this be part of the initial sumo clean setup SQL ?

Comment 14

[Roland Tanglao needinfo please :rolandtanglao, :mohnkuchen, :adobo, :sinigang, :roland]

gozer the SQL did't fix it. Still get "An error occured in a database query!"
Do I have to create a poll or is it still broken.

FULL DB QUERY ERROR:

Context:
File	tiki-editpage.php
Url	tiki-editpage.php?locale=en-US&page=Cannot+receive+email+something+newish
Query:
insert into tiki_polls(`title`,`active`,`publishDate`,`voteConsiderationSpan`,`votes`,`pollQuestionId`) values(?,?,?,?,?,?)
Values:
0	Cannot receive email something newish
1	o
2	1254854934
3	60
4	0
5	NULL
Message:
Column 'pollQuestionId' cannot be null
Built query was probably:
insert into tiki_polls(`title`,`active`,`publishDate`,`voteConsiderationSpan`,`votes`,`pollQuestionId`) values('Cannot receive email something newish','o','1254854934','60','0',NULL)

Comment 15

[Paul Craciunoiu [:paulc]]

Attachment: v1

What's strange is that this _is_ a polls issue, but the SQL didn't seem to fix it. I'm not sure what SQL was run, but there are two parts to this:
a) Polls must exist
b) Polls must be assigned to KB articles, so the SQL queries than run this can find them. 

I can take a closer look at this next week, but James, feel free to investigate it if you can get to it before that. Right now, though, this quick patch might help. It's untested, since I don't have time to set up a clean schema right now. Anyone: feel free to review this patch :)

Comment 16

[Philippe M. Chiasson (:gozer)]

(In reply to comment #15)
> Created an attachment (id=404879) [details]
> v1
> 
> What's strange is that this _is_ a polls issue, but the SQL didn't seem to fix
> it. I'm not sure what SQL was run, but there are two parts to this:
> a) Polls must exist
> b) Polls must be assigned to KB articles, so the SQL queries than run this can
> find them. 
> 
> I can take a closer look at this next week, but James, feel free to investigate
> it if you can get to it before that. Right now, though, this quick patch might
> help. It's untested, since I don't have time to set up a clean schema right
> now. Anyone: feel free to review this patch :)

Applied patch in Momo staging

Comment 17

[James Socol [:jsocol, :james]]

(In reply to comment #16)
> Applied patch in Momo staging

Did it help?

Comment 18

[Roland Tanglao needinfo please :rolandtanglao, :mohnkuchen, :adobo, :sinigang, :roland]

it helped, it is working now on MoMo staging, the kb article gets created on momo staging, if more qa is needed, please do it otherwise please close this ticket

Comment 19

[James Socol [:jsocol, :james]]

Closing this as a fix is in place for MoMo. Not sure how compatible that fix is with CSAT polls but we'll cross that bridge when we come to it.

Comment 20

[E. Cooper [:ecooper]]

(In reply to comment #19)
> Closing this as a fix is in place for MoMo. Not sure how compatible that fix is
> with CSAT polls but we'll cross that bridge when we come to it.

Just a note on the patch, the error is because get_templateIdFromQ() is returning NULL and it shouldn't be. If the SQL didn't fix the problem, the db is still missing seed data for the polls tables. 

Using INSERT IGNORE on replace_poll() just turns the error from trying to insert a NULL value into a NOT NULL column into a warning. This doesn't fix the problem.

http://viewvc.svn.mozilla.org/vc/projects/sumo/trunk/webroot/lib/polls/polllib_shared.php?revision=21756&view=markup#l293

get_templateIdFromQ() uses tiki_polls with an inner join on tiki_poll_questions. https://bug519621.bugzilla.mozilla.org/attachment.cgi?id=403699 only inserts data for tiki_poll_questions and not tiki_polls. So, if the seed data for tiki_polls hasn't been imported to MoMo, then that's likely the problem.

Sorry for the lateness. :(

Comment 21

[James Socol [:jsocol, :james]]

(In reply to comment #20)
> get_templateIdFromQ() uses tiki_polls with an inner join on
> tiki_poll_questions.
> https://bug519621.bugzilla.mozilla.org/attachment.cgi?id=403699 only inserts
> data for tiki_poll_questions and not tiki_polls. So, if the seed data for
> tiki_polls hasn't been imported to MoMo, then that's likely the problem.

I was afraid of that. tiki_polls has around 10k rows. How can we figure out which ones we need?

Comment 22

[E. Cooper [:ecooper]]

Going by the attachment in bug 452829, the following SQL statement will get the rows you need for tiki_polls:

SELECT * FROM tiki_polls WHERE pollQuestionId IN (1,2,3) and active= 't';

If run against a current SUMO db, it will get you what you need. Most likely, if all the other data for polls isn't in the schema, then tiki_poll_options will need its seed data too. Off the top of my head (so keep in mind it _might_ need tweaked to work):

SELECT tpo.* FROM tiki_poll_options tpo INNER JOIN tiki_polls tp on tpo.pollId = tp.pollId WHERE tp.pollQuestionId IN (1,2,3) AND tp.active = 't';

Comment 23

[[:Cww]]

Would it make more sense to just run the script in bug 452829? : https://bugzilla.mozilla.org/attachment.cgi?id=340372

Comment 24

[E. Cooper [:ecooper]]

(In reply to comment #23)
> Would it make more sense to just run the script in bug 452829? :
> https://bugzilla.mozilla.org/attachment.cgi?id=340372

That script does a bunch of things, mainly schema changes and updating existing polls to fit those schema changes. Running it again will likely cause problems (or do nothing if the legacy check works). It was from when Nelson had initially implemented the expanded CSAT for the KB ages ago. I only used it to find the pollQuestionId's needed.

The queries in comment 22 return 3 and 9 rows respectively, and it should get the polls for KB CSAT working automagically as they should be.

Comment 25

[Stephen Donner [:stephend] Not actively reading bugmail]

Reopening until MoMo runs the SQL from comment 22 (and comment 24).

Comment 26

[Philippe M. Chiasson (:gozer)]

(In reply to comment #25)
> Reopening until MoMo runs the SQL from comment 22 (and comment 24).

Did you mean MoCo, maybe?


mysql> SELECT * FROM tiki_polls WHERE pollQuestionId IN (1,2,3) and active= 't';
Empty set (0.00 sec)

mysql> SELECT tpo.* FROM tiki_poll_options tpo INNER JOIN tiki_polls tp on tpo.pollId
    -> = tp.pollId WHERE tp.pollQuestionId IN (1,2,3) AND tp.active = 't';
Empty set (0.00 sec)

Does look like seed data is simply not in there.

mysql> select * from tiki_polls;
+--------+----------------------------------------+-------+--------+-------------+-----------------------+----------------+-------------+
| pollId | title                                  | votes | active | publishDate | voteConsiderationSpan | pollQuestionId | is_feedback |
+--------+----------------------------------------+-------+--------+-------------+-----------------------+----------------+-------------+
|      1 | Cannot receive email something newish2 |     0 | o      |  1254865751 |                    60 |              0 |           0 | 
|      2 | Cannot receive email something newish2 |     0 | o      |  1254865751 |                    60 |              0 |           0 | 
|      3 | Cannot receive email something newish2 |     0 | o      |  1254865751 |                    60 |              0 |           0 | 
|      4 | Cannot receive email something 3new    |     0 | o      |  1254865887 |                    60 |              0 |           0 | 
|      5 | Cannot receive email something 3new    |     0 | o      |  1254865887 |                    60 |              0 |           0 | 
|      6 | Cannot receive email something 3new    |     0 | o      |  1254865887 |                    60 |              0 |           0 |

Comment 27

[Stephen Donner [:stephend] Not actively reading bugmail]

@ gozer: no, I meant "MoMo": https://wiki.mozilla.org/Thunderbird/Support/Thunderbird3/SupportPlan#MoMo_Web_Presence

Comment 28

[James Socol [:jsocol, :james]]

Attachment: sql, part 1

Comment 29

[James Socol [:jsocol, :james]]

Attachment: sql, part 2

These two SQL patches should contain the necessary CSAT seed data, based on comment 22 and comment 24.

Comment 30

[Philippe M. Chiasson (:gozer)]

Both SQL scripts executed on momo sumo staging.

@roland: can you check if it worked
@jsocol: if it does work, this sql should make its way to sumo_clean_schema.sql, yes ?

Comment 31

[James Socol [:jsocol, :james]]

(In reply to comment #30)
> @jsocol: if it does work, this sql should make its way to
> sumo_clean_schema.sql, yes ?

Absolutely. The clean schema data has fallen a bit out of date anyway. 1.4 has at least one schema change.

Comment 32

[James Socol [:jsocol, :james]]

This isn't really tied to our milestone.

Roland, are the CSAT polls working now?

Comment 33

[Chris Ilias [:cilias]]

If I go to <https://sumo-preview.mozillamessaging.com/en-US/kb/Hotmail>, I don't see any CSAT polls.

Comment 34

[E. Cooper [:ecooper]]

(In reply to comment #33)
> If I go to <https://sumo-preview.mozillamessaging.com/en-US/kb/Hotmail>, I
> don't see any CSAT polls.

That article was last modified Sept. 24th. To test if this problem has been resolved, you need to check a page created after Oct. 14th.

Articles moved to the KB before all of the seed data was present don't have CSAT polls because the information needed to create them wasn't available. It's also important to note that only when an article is moved to the KB are these polls created. At least, AFIK.

Comment 35

[Roland Tanglao needinfo please :rolandtanglao, :mohnkuchen, :adobo, :sinigang, :roland]

verified fixed on sumo-preview.mozillamessaging.com running 1.4.0.1

Comment 36

[Chris Ilias [:cilias]]

Can we get this done on mobile.support.mozilla.com? I still get this error when trying to move an article to the KB on mobile.support.mozilla.com.
(file a separate bug?)

Comment 37

[James Socol [:jsocol, :james]]

Filed bug 523844 to get the SQL patches run on mo.su.mo

Comment 38

[Chris Ilias [:cilias]]

Is there anything else that needs to be done? I'm still getting the error, when trying to move an article to the KB.

Comment 39

[James Socol [:jsocol, :james]]

On mo.su.mo stage, I just created an article in the staging area then moved it into the KB without any errors. Chris, is there another step I need to trigger the error?

https://mobile.support-stage.mozilla.org/en-US/kb/Bug+517012

Comment 40

[Chris Ilias [:cilias]]

Thanks James. I was testing the bug on mobile.support.mozilla.com. I don't know of any plan for Fennec code pushes, so I filed bug 524159.

Comment 41

[David Tenser [:djst]]

Seems this is already verified per comment 35, but some real QA love wouldn't hurt. Stephen, what's your take?

Comment 42

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED; I tested on https://sumo-preview.mozillamessaging.com/en-US/kb/ohai and https://mobile.support-stage.mozilla.org/en-US/kb/o+hai?bl=n.