From f394b2ed4f175bf6076747ba7792e182841091ab Mon Sep 17 00:00:00 2001 From: "lpsolit%gmail.com" <> Date: Fri, 30 Nov 2007 01:49:12 +0000 Subject: Bug 99215: Attachments have no midair collision protection - Patch by Frédéric Buclin r=mkanat r=justdave a=justdave MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Bugzilla/Attachment.pm | 27 +++++++-- Bugzilla/Bug.pm | 11 +++- Bugzilla/DB/Schema.pm | 2 + Bugzilla/Install/DB.pm | 28 +++++++++ attachment.cgi | 26 ++++++++- process_bug.cgi | 2 +- template/en/default/attachment/edit.html.tmpl | 1 + template/en/default/attachment/midair.html.tmpl | 76 +++++++++++++++++++++++++ template/en/default/filterexceptions.pl | 4 ++ 9 files changed, 167 insertions(+), 10 deletions(-) create mode 100644 template/en/default/attachment/midair.html.tmpl diff --git a/Bugzilla/Attachment.pm b/Bugzilla/Attachment.pm index cc3e16893..e3fe39f3a 100644 --- a/Bugzilla/Attachment.pm +++ b/Bugzilla/Attachment.pm @@ -91,6 +91,7 @@ sub _retrieve { 'attachments.submitter_id AS _attacher_id', Bugzilla->dbh->sql_date_format('attachments.creation_ts', '%Y.%m.%d %H:%i') . " AS attached", + 'attachments.modification_time', 'attachments.filename AS filename', 'attachments.ispatch AS ispatch', 'attachments.isurl AS isurl', @@ -208,6 +209,21 @@ sub attached { =over +=item C + +the date and time on which the attachment was last modified. + +=back + +=cut + +sub modification_time { + my $self = shift; + return $self->{modification_time}; +} + +=over + =item C the name of the file the attacher attached @@ -826,10 +842,10 @@ sub insert_attachment_for_bug { # Insert the attachment into the database. my $sth = $dbh->do( "INSERT INTO attachments - (bug_id, creation_ts, filename, description, + (bug_id, creation_ts, modification_time, filename, description, mimetype, ispatch, isurl, isprivate, submitter_id) - VALUES (?,?,?,?,?,?,?,?,?)", undef, ($bug->bug_id, $timestamp, $filename, - $description, $contenttype, $cgi->param('ispatch'), + VALUES (?,?,?,?,?,?,?,?,?,?)", undef, ($bug->bug_id, $timestamp, $timestamp, + $filename, $description, $contenttype, $cgi->param('ispatch'), $isurl, $isprivate, $user->id)); # Retrieve the ID of the newly created attachment record. my $attachid = $dbh->bz_last_key('attachments', 'attach_id'); @@ -877,8 +893,9 @@ sub insert_attachment_for_bug { # This call must be done before updating the 'attachments' table. Bugzilla::Flag::CancelRequests($bug, $obsolete_attachment, $timestamp); - $dbh->do('UPDATE attachments SET isobsolete = 1 WHERE attach_id = ?', - undef, $obsolete_attachment->id); + $dbh->do('UPDATE attachments SET isobsolete = 1, modification_time = ? + WHERE attach_id = ?', + undef, ($timestamp, $obsolete_attachment->id)); $dbh->do('INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when, fieldid, removed, added) diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index a8f1ede5d..0a45daf14 100755 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -2573,11 +2573,11 @@ sub format_comment { # Get the activity of a bug, starting from $starttime (if given). # This routine assumes ValidateBugID has been previously called. sub GetBugActivity { - my ($id, $starttime) = @_; + my ($bug_id, $attach_id, $starttime) = @_; my $dbh = Bugzilla->dbh; # Arguments passed to the SQL query. - my @args = ($id); + my @args = ($bug_id); # Only consider changes since $starttime, if given. my $datepart = ""; @@ -2587,6 +2587,12 @@ sub GetBugActivity { $datepart = "AND bugs_activity.bug_when > ?"; } + my $attachpart = ""; + if ($attach_id) { + push(@args, $attach_id); + $attachpart = "AND bugs_activity.attach_id = ?"; + } + # Only includes attachments the user is allowed to see. my $suppjoins = ""; my $suppwhere = ""; @@ -2616,6 +2622,7 @@ sub GetBugActivity { ON profiles.userid = bugs_activity.who WHERE bugs_activity.bug_id = ? $datepart + $attachpart $suppwhere ORDER BY bugs_activity.bug_when"; diff --git a/Bugzilla/DB/Schema.pm b/Bugzilla/DB/Schema.pm index 1c6ee352e..61f894f83 100644 --- a/Bugzilla/DB/Schema.pm +++ b/Bugzilla/DB/Schema.pm @@ -372,6 +372,7 @@ use constant ABSTRACT_SCHEMA => { PRIMARYKEY => 1}, bug_id => {TYPE => 'INT3', NOTNULL => 1}, creation_ts => {TYPE => 'DATETIME', NOTNULL => 1}, + modification_time => {TYPE => 'DATETIME', NOTNULL => 1}, description => {TYPE => 'MEDIUMTEXT', NOTNULL => 1}, mimetype => {TYPE => 'MEDIUMTEXT', NOTNULL => 1}, ispatch => {TYPE => 'BOOLEAN'}, @@ -389,6 +390,7 @@ use constant ABSTRACT_SCHEMA => { INDEXES => [ attachments_bug_id_idx => ['bug_id'], attachments_creation_ts_idx => ['creation_ts'], + attachments_modification_time_idx => ['modification_time'], attachments_submitter_id_idx => ['submitter_id', 'bug_id'], ], }, diff --git a/Bugzilla/Install/DB.pm b/Bugzilla/Install/DB.pm index 9934e058f..485c771b1 100644 --- a/Bugzilla/Install/DB.pm +++ b/Bugzilla/Install/DB.pm @@ -515,6 +515,9 @@ sub update_table_definitions { # 2007-08-21 wurblzap@gmail.com - Bug 365378 _make_lang_setting_dynamic(); + # 2007-09-09 LpSolit@gmail.com - Bug 99215 + _fix_attachment_modification_date(); + ################################################################ # New --TABLE-- changes should go *** A B O V E *** this point # ################################################################ @@ -2898,6 +2901,31 @@ sub _make_lang_setting_dynamic { } } +sub _fix_attachment_modification_date { + my $dbh = Bugzilla->dbh; + if (!$dbh->bz_column_info('attachments', 'modification_time')) { + # Allow NULL values till the modification time has been set. + $dbh->bz_add_column('attachments', 'modification_time', {TYPE => 'DATETIME'}); + + print "Setting the modification time for attachments...\n"; + $dbh->do('UPDATE attachments SET modification_time = creation_ts'); + + # Now force values to be always defined. + $dbh->bz_alter_column('attachments', 'modification_time', + {TYPE => 'DATETIME', NOTNULL => 1}); + + # Update the modification time for attachments which have been modified. + my $attachments = + $dbh->selectall_arrayref('SELECT attach_id, MAX(bug_when) FROM bugs_activity + WHERE attach_id IS NOT NULL ' . + $dbh->sql_group_by('attach_id')); + + my $sth = $dbh->prepare('UPDATE attachments SET modification_time = ? + WHERE attach_id = ?'); + $sth->execute($_->[1], $_->[0]) foreach (@$attachments); + } +} + 1; __END__ diff --git a/attachment.cgi b/attachment.cgi index 23841571c..768653c31 100755 --- a/attachment.cgi +++ b/attachment.cgi @@ -460,6 +460,27 @@ sub update { $cgi->param('isobsolete', $cgi->param('isobsolete') ? 1 : 0); $cgi->param('isprivate', $cgi->param('isprivate') ? 1 : 0); + # Now make sure the attachment has not been edited since we loaded the page. + if (defined $cgi->param('delta_ts') + && $cgi->param('delta_ts') ne $attachment->modification_time) + { + ($vars->{'operations'}) = + Bugzilla::Bug::GetBugActivity($bug->id, $attachment->id, $cgi->param('delta_ts')); + + # If the modification date changed but there is no entry in + # the activity table, this means someone commented only. + # In this case, there is no reason to midair. + if (scalar(@{$vars->{'operations'}})) { + $cgi->param('delta_ts', $attachment->modification_time); + $vars->{'attachment'} = $attachment; + + print $cgi->header(); + # Warn the user about the mid-air collision and ask them what to do. + $template->process("attachment/midair.html.tmpl", $vars) + || ThrowTemplateError($template->error()); + exit; + } + } # If the submitter of the attachment is not in the insidergroup, # be sure that he cannot overwrite the private bit. # This check must be done before calling Bugzilla::Flag*::validate(), @@ -507,11 +528,12 @@ sub update { filename = ?, ispatch = ?, isobsolete = ?, - isprivate = ? + isprivate = ?, + modification_time = ? WHERE attach_id = ?", undef, ($description, $contenttype, $filename, $cgi->param('ispatch'), $cgi->param('isobsolete'), - $cgi->param('isprivate'), $attachment->id)); + $cgi->param('isprivate'), $timestamp, $attachment->id)); my $updated_attachment = Bugzilla::Attachment->get($attachment->id); # Record changes in the activity table. diff --git a/process_bug.cgi b/process_bug.cgi index 99ee5ed57..a8df416cd 100755 --- a/process_bug.cgi +++ b/process_bug.cgi @@ -878,7 +878,7 @@ foreach my $id (@idlist) { if (defined $cgi->param('delta_ts') && $cgi->param('delta_ts') ne $delta_ts) { ($vars->{'operations'}) = - Bugzilla::Bug::GetBugActivity($id, $cgi->param('delta_ts')); + Bugzilla::Bug::GetBugActivity($id, undef, $cgi->param('delta_ts')); $vars->{'start_at'} = $cgi->param('longdesclength'); diff --git a/template/en/default/attachment/edit.html.tmpl b/template/en/default/attachment/edit.html.tmpl index 23e104d86..3796b5650 100644 --- a/template/en/default/attachment/edit.html.tmpl +++ b/template/en/default/attachment/edit.html.tmpl @@ -202,6 +202,7 @@ + diff --git a/template/en/default/attachment/midair.html.tmpl b/template/en/default/attachment/midair.html.tmpl new file mode 100644 index 000000000..8cde9f2f5 --- /dev/null +++ b/template/en/default/attachment/midair.html.tmpl @@ -0,0 +1,76 @@ +[%# The contents of this file are subject to the Mozilla Public + # License Version 1.1 (the "License"); you may not use this file + # except in compliance with the License. You may obtain a copy of + # the License at http://www.mozilla.org/MPL/ + # + # Software distributed under the License is distributed on an "AS + # IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or + # implied. See the License for the specific language governing + # rights and limitations under the License. + # + # The Original Code is the Bugzilla Bug Tracking System. + # + # The Initial Developer of the Original Code is Netscape Communications + # Corporation. Portions created by Netscape are + # Copyright (C) 1998 Netscape Communications Corporation. All + # Rights Reserved. + # + # Contributor(s): Myk Melez + # Frédéric Buclin + #%] + +[%# INTERFACE: + # operations: array; bug activity since the user last displayed the attachment form, + # used by bug/activity/table.html.tmpl to display recent changes that will + # be overwritten if the user submits these changes. See that template + # for further documentation. + # attachment: object; the attachment being changed. + #%] + +[%# The global Bugzilla->cgi object is used to obtain form variable values. %] +[% USE Bugzilla %] +[% cgi = Bugzilla.cgi %] + +[% PROCESS global/variables.none.tmpl %] +[% PROCESS global/header.html.tmpl title = "Mid-air collision!" %] + +

Mid-air collision detected!

+ +

+ Someone else has made changes to + attachment [% attachment.id %] + of [% "$terms.bug $attachment.bug_id" FILTER bug_link(attachment.bug_id) FILTER none %] + at the same time you were trying to. The changes made were: +

+ +

+ [% PROCESS "bug/activity/table.html.tmpl" incomplete_data=0 %] +

+ +[% IF cgi.param("comment") %] +

+ Your comment was:
+ 

[% cgi.param("comment") FILTER wrap_comment FILTER html %]
+

+[% END %] + +

+You have the following choices: +

+ +
    +
  • +
    + [% PROCESS "global/hidden-fields.html.tmpl" exclude="^Bugzilla_(login|password)$" %] + + This will cause all of the above changes to be overwritten. + +
    • +
  • + Throw away my changes, and + revisit + attachment [% attachment.id %] +
    • +
+ +[% PROCESS global/footer.html.tmpl %] diff --git a/template/en/default/filterexceptions.pl b/template/en/default/filterexceptions.pl index 80957e6c7..e2acdcbca 100644 --- a/template/en/default/filterexceptions.pl +++ b/template/en/default/filterexceptions.pl @@ -416,6 +416,10 @@ 'obsolete_attachments', ], +'attachment/midair.html.tmpl' => [ + 'attachment.id', +], + 'attachment/show-multiple.html.tmpl' => [ 'a.id', 'flag.status' -- cgit v1.2.3-24-g4f1b