301268 - Bugzilla::Product should have a ->all_versions instead of using Bugzilla::Version::get_versions_by_product directly

Status: RESOLVED WORKSFORME

(Bugzilla :: Bugzilla-General, enhancement)

Description

[Max Kanat-Alexander]

Basically, "all versions" (what get_versions_by_product) returns is a feature of
a Product. So, instead of directly calling
Bugzilla::Version::get_versions_by_product in code, we should be able to do
something like $product->all_versions.

Comment 1

[Frédéric Buclin]

I was thinking about that too while reviewing timello's patch. And I came to the
conclusion that the actual code avoid some code duplication, i.e. we have only
one way to get all versions of a given product, which I like (especially for
debugging, because you don't have to check the 25 different part of the code
which do the same thing).

mkanat, why would you like to duplicate the code?

Comment 2

[Max Kanat-Alexander]

(In reply to comment #1)
> mkanat, why would you like to duplicate the code?

  It's not really duplication. get_versions_by_product should never be called by
CGI code, and probably only once in all of Bugzilla -- inside of
Bugzilla::Product->all_versions. One calls the other, and ->all_versions is the
one that's normally used.

  A product *has a* version, and it also *has a* set of versions.
get_versions_by_product exists in Version.pm as it should, because it's code
that accesses the versions table in the DB (it has to do with Versions).

Comment 3

[Tiago Mello [:timello]]

We already have that method, it is called: versions:

sub versions {
    my $self = shift;

    if (!defined $self->{versions}) {
        $self->{versions} =
            Bugzilla::Version::get_versions_by_product($self->id);

    }
    return $self->{versions};
}

I don't think this is required.

Comment 4

[Max Kanat-Alexander]

Oh! :-) I didn't notice that. :-)

Then we should be using ->versions instead of get_versions_by_product in CGI code.