314547 - [PostgreSQL] cannot check/uncheck the "Private" checkbox for comments in show_bug.cgi

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, defect)

Description

[Frédéric Buclin]

When (un)checking the "Private" checkbox using Pg, changes are not saved. I first thought it was a regression due to bug 310171, but this patch doesn't alter SQL queries. Maybe the problem is in process_bug.cgi, at line 942:

            my $private = $cgi->param("isprivate-$sequence") ? 1 : 0 ;
            if ($private != $cgi->param("oisprivate-$sequence")) {
                my $field_data = $cgi->param("$field");
                detaint_natural($field_data);
                SendSQL("UPDATE longdescs SET isprivate = $private " .
                        "WHERE bug_id = " . $cgi->param('id') . 
                        " AND bug_when = $field_data");
            }

$field_data is an integer, but longdescs.bug_when is of type datetime. Maybe is there a conversion issue on Pg?

Comment 1

[Max Kanat-Alexander]

Weird. We ran into this issue once before, also, and we fixed it. Perhaps somebody unfixed it.

Comment 2

[Frédéric Buclin]

I was right in comment 0.

select * from longdescs where bug_when = '2005-05-04 00:35:04'; works fine on both Pg and MySQL; but:

select * from longdescs where bug_when = 20050504003504; returns no result on Pg.

The second form is the one used in process_bug.cgi.

Note that we wouldn't have this problem if 'longdescs' had a primary key, for instance (bug_id, comment_id) ;)

Comment 3

[Frédéric Buclin]

Attachment: patch, v1

The format used for when-xxx in bug/comments.html.tmpl cannot be converted back to a valid date. I tried it using Bugzilla::Util::format_time() but the conversion failed. So YYYYMMddHHmmss is only understood by MySQL, but Pg has no idea what it is.

I scanned the whole code to see where $bug->longdescs (and bug.longdescs in templates) was used, and I found only 4 places:

enter_bug.cgi:

    $cloned_bug->longdescs();
    my $isprivate             = $cloned_bug->{'longdescs'}->[0]->{'isprivate'};
    $vars->{'comment'}        = $cloned_bug->{'longdescs'}->[0]->{'body'};

template/en/default/bug/edit.html.tmpl:637:
    comments = bug.longdescs

template/en/default/bug/show-multiple.html.tmpl:194:
    comments = bug.longdescs %]

template/en/default/bug/show.xml.tmpl:55:
    [% FOREACH c = bug.longdescs %]


enter_bug.cgi doesn't care neither about longdescs.time nor about longdescs.(bug_)when. show.xml.tmpl doesn't use longdescs.(bug_)when, but it uses longdecs.time to display when a comment was added. And the remaining two templates both call bug/comments.html.tmpl which uses both fields, among other to indicate to process_bug.cgi which comments we are talking about.

In this last (and unique) case, I now use longdescs.time instead of longdescs.when as an ID for the comment, which can then be correctly handled by Util::format_time() in process_bug.cgi. This means that longdescs.(bug_)when is no longer required as the only place where it was used doesn't work on PostgreSQL.

My patch was successfully tested on MySQL 4.1.11, PostgreSQL 7.4.8 and 8.0.1.

Comment 4

[Max Kanat-Alexander]

Comment on attachment 201738 [details] [diff] [review]
patch, v1

I like this patch. :-) It works (I tested on landfill), it cleans stuff up, and it fixes a bug. :-) That's great. :-)

Comment 5

[Frédéric Buclin]

Checking in process_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v  <--  process_bug.cgi
new revision: 1.296; previous revision: 1.295
done
Checking in Bugzilla/Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v  <--  Bug.pm
new revision: 1.100; previous revision: 1.99
done
Checking in template/en/default/filterexceptions.pl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/filterexceptions.pl,v  <--  filterexceptions.pl
new revision: 1.58; previous revision: 1.57
done
Checking in template/en/default/bug/comments.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/comments.html.tmpl,v  <--  comments.html.tmpl
new revision: 1.23; previous revision: 1.22
done