Hi Dmitri, thanks for diving into this.

On Tue, 2007-06-26 at 22:58 +1000, Dmitri Colebatch wrote:
> > Yep, I tried that, and then I tried deleting all the versions in
> > bugzilla.  My guess is that its a genuine failure - I'll try to make
> > time to take a look.

I suspect there's something we're missing. Perhaps the way the version
is added.

> I think the difference here is that the previous code removed the
> version via SQL, meaning there were no checks to see if any bugs were
> raised against that version.  We're now removing it via the
> remove_from_db method which looks like this:
> 
> sub remove_from_db {
>    my $self = shift;
>    my $dbh = Bugzilla->dbh;
> 
>    # The version cannot be removed if there are bugs
>    # associated with it.
>    if ($self->bug_count) {
>        ThrowUserError("version_has_bugs", { nb => $self->bug_count });
>    }
> 
>    $dbh->do(q{DELETE FROM versions WHERE product_id = ? AND value = ?},
>              undef, ($self->product_id, $self->name));
> }
> 
> So obviously this isn't going to work if bugs are still raised against
> that version.  I see three options here:

Right, the code makes sense. What I'm unsure of is whether the Bugzilla
backend is adding the version in Bugzilla using the correct API call. I
mean when a tag is applied, are we just adding the version in the
versions list, or are we (accidentally) adding the version
against ...the particular bug! I'm saying that the testsuite currently
seems to cause bugs to be associated with versions, while it shouldn't.
It's not the testsuite that's doing it really -- it must be the Scmbug
Bugzilla backend.

Yeah, have another look and see what's going on there.

I see that the create function in Bugzilla/Version.pm reads:

sub create {
    my ($name, $product) = @_;
    my $dbh = Bugzilla->dbh;

    # Cleanups and validity checks
    $name || ThrowUserError('version_blank_name');

    # Remove unprintable characters
    $name = clean_text($name);

    my $version = new Bugzilla::Version({ product => $product, name =>
$name });
    if ($version) {
        ThrowUserError('version_already_exists',
                       {'name' => $version->name,
                        'product' => $product->name});
    }

    # Add the new version
    trick_taint($name);
    $dbh->do(q{INSERT INTO versions (value, product_id)
               VALUES (?, ?)}, undef, ($name, $product->id));

    return new Bugzilla::Version($dbh->bz_last_key('versions', 'id'));
}

so it looks like it's implemented correctly. Hmmmm.... there's something
fishy here.

> 1. Leave it as is - don't allow people to remove tags that have bugs
> raised against them.  This to me seems valid - if there's a bug raised
> against a version then it has been used by users and to me that would
> mean it shouldn't be deleted.  Having said all this, different
> organisations have different policies.

I think this option is reasonable, and also enforces change management:
if users want to remove the version, they should scrub their bugtracker
first. I just don't understand how the testsuite associated a bug with a
version.

> 2. Go back to SQL and delete the version in the database as per
> previous behaviour.

Well, we'd rather use the published Bugzilla API.

> 3. Go through the bugs and change the version to null before removing
> the version.

No, that'd be too presumptious on our part.

Attachment: signature.asc
Description: This is a digitally signed message part

_______________________________________________
scmbug-users mailing list
[email protected]
http://lists.mkgnu.net/cgi-bin/mailman/listinfo/scmbug-users

Reply via email to