385643 - Add ability for describecomponents to describe all components in all products

Status: NEW

(Bugzilla :: Administration, task, P4)

Description

[timeless]

we should provide: https://bugzilla.mozilla.org/describecomponents.cgi?full=1

this is based on bugzilla.kernel.org which has this feature. given that we have a similar feature for enter_bug, it's silly that we have this for enter_bug but not for describecomponents and that kernel.org had to make its own feature.

Comment 1

[timeless]

it seems that enter_bug.cgi?full=1 is not part of cvs. I think it should be. reed?

Comment 3

[David Lawrence [:dkl]]

Attachment: Patch to all showing all products in describecomponents.cgi (v1)

This patch adds product=__all to describecomponents.cgi as well as add link in choose-products.html.tmpl. Also some slight cleanup in the CSS to cleanup the page.

dkl

Comment 4

[:glob ✱]

Comment on attachment 605979 [details] [diff] [review]
Patch to all showing all products in describecomponents.cgi (v1)

passing in an invalid product name results in:

Can't call method "name" on an undefined value at describecomponents.cgi line 38.

instead of saying:

The product cheese does not exist or you don't have access to it. The following is a list of the products you can choose from.

Comment 5

[:glob ✱]

someone on IRC mentioned this should be configurable; they have over 44k components.

Comment 6

[Asa Dotzler [:asa]]

(In reply to Byron Jones ‹:glob› from comment #5)
> someone on IRC mentioned this should be configurable; they have over 44k
> components.

That sounds like optimizing for an outlier to me.

Comment 7

[:glob ✱]

(In reply to Asa Dotzler [:asa] from comment #6)
> That sounds like optimizing for an outlier to me.

i don't think it's that uncommon -- bmo has over 1,000 components and we're not component heavy.

Comment 8

[David Lawrence [:dkl]]

(In reply to Byron Jones ‹:glob› from comment #5)
> someone on IRC mentioned this should be configurable; they have over 44k
> components.

Don't hit "All Products"? ;) Are you suggesting making it an admin param to allow turning the "All Products" functionality off, or are you suggesting to make something like a multi-select product list (with All as an option) where a user can select only the products they want to show components for? The former would not take too much effort really, the latter slightly more.

Unfortunately doing the former (param) would cause sites with a "few" products that have large amounts of components to lose out on the benefit for the other products. So I would be more inclined to make it a multi-select list so smaller products could be viewed. Of course we could go even further and make the admin param one to select which type of form to display; simple "All" link, or a multi-select product list. Thoughts?

Also describecomponents.cgi should also have other output formats such as csv, xml etc. but topic for another bug.

dkl

Comment 9

[David Lawrence [:dkl]]

Attachment: Patch to all showing all products in describecomponents.cgi (v2)

Throws proper error when invalid product name provided.

dkl

Comment 10

[:glob ✱]

(In reply to David Lawrence [:dkl] from comment #8)
> Are you suggesting making it an admin param to allow turning the
> "All Products" functionality off

yes.

> Unfortunately doing the former (param) would cause sites with a "few"
> products that have large amounts of components to lose out on the benefit
> for the other products.

in that configuration, there's probably no great need to have an all-products link as it wouldn't take long to cycle through all products.

> So I would be more inclined to make it a multi-select list so smaller products
> could be viewed. Of course we could go even further and make the admin param one
> to select which type of form to display; simple "All" link, or a multi-select
> product list. Thoughts?

part of my concerns are with the UI, but also with the resources in terms of time and memory required to build a page with a large amount of components.

i think keeping this page simple would be better (avoid the multi-select), and look at bug 198408 as a better way to find components.

> Also describecomponents.cgi should also have other output formats such as
> csv, xml etc. but topic for another bug.

can't you get that from config.cgi?

Comment 11

[David Lawrence [:dkl]]

(In reply to Byron Jones ‹:glob› from comment #10)
> (In reply to David Lawrence [:dkl] from comment #8)
> > Are you suggesting making it an admin param to allow turning the
> > "All Products" functionality off
> 
> yes.
> 

[snip]

Ok, I will do a new patch with an admin param to disble "All Products".

> > Also describecomponents.cgi should also have other output formats such as
> > csv, xml etc. but topic for another bug.
> 
> can't you get that from config.cgi?

No. config.cgi just includes component names. Not descriptions, assignees, qacontacts, etc.

dkl

Comment 12

[David Lawrence [:dkl]]

Attachment: Patch to show all products in describecomponents.cgi (v3)

Added admin param to disable all product feature.

dkl

Comment 13

[Frédéric Buclin]

Comment on attachment 607265 [details] [diff] [review]
Patch to show all products in describecomponents.cgi (v3)

>=== modified file 'Bugzilla/Config/General.pm'

>+   name => 'describe_comps_all_prods', 

This is not the right place for this parameter. Bug Fields would be a better place than General (this parameters is really not general, but very specific to describecomponents.cgi). Also, we removed showallproducts in bug 399079; it doesn't make sense to reintroduce another parameter for a similar purpose. We tried very hard to kill such parameters in the past, and I still don't see a valid use case to let a user view all components of all products.

Comment 14

[David Lawrence [:dkl]]

Attachment: Patch to show all products in describecomponents.cgi (v4)

Move param from General to Bug Fields.

dkl

Comment 15

[Frédéric Buclin]

Comment on attachment 607301 [details] [diff] [review]
Patch to show all products in describecomponents.cgi (v4)

No, I was saying that moving the parameter to Bug Fields would make more sense, but I still don't want a parameter for that. We already list all components from all products in query.cgi. I see no reason to have a parameter for describecomponents.cgi only.

Comment 16

[:glob ✱]

(In reply to Frédéric Buclin from comment #15)
> No, I was saying that moving the parameter to Bug Fields would make more
> sense, but I still don't want a parameter for that. We already list all
> components from all products in query.cgi. I see no reason to have a
> parameter for describecomponents.cgi only.

from a UI point of view these have very different presentations; query lists just the component name in a select, while this patch will add a "wall of text" with a lot more than just the name.  it isn't a reasonable comparison.

once a bugzilla install starts getting a large amount of components, which isn't that hard to do (again, bmo has over 1000), we're looking at a potentially useless page which may harm the server while its being generated due to a lack of progressive template building.

Comment 17

[Frédéric Buclin]

(In reply to Byron Jones ‹:glob› from comment #16)
> once a bugzilla install starts getting a large amount of components, which
> isn't that hard to do (again, bmo has over 1000), we're looking at a
> potentially useless page which may harm the server while its being generated
> due to a lack of progressive template building.

What you are saying is that you wouldn't turn this feature on on bmo (and you are probably right) due to its large number of components, but those who are requesting this feature are timeless (this bug) and asa (the dupe), both having bmo in mind when suggesting this feature. So for now, I see no reason to implement this feature at all as those requesting this feature wouldn't have it enabled anyway.

I want more evidence that this feature would be useful to others (i.e. other smaller installations) before adding this feature, with or without a parameter. With fewer products, the confusion is generally smaller, and so this feature becomes useless as the user already knows which product to look at.

Comment 18

[:glob ✱]

(In reply to Frédéric Buclin from comment #17)
> What you are saying is that you wouldn't turn this feature on on bmo (and
> you are probably right) due to its large number of components, 

i never said this feature won't be enabled on bmo.  the only reason i mentioned bmo is because we're not a very component heavy installation, yet we still manage over 1000 components.

> With fewer products, the confusion is generally smaller, and so
> this feature becomes useless as the user already knows which product to look
> at.

agreed.

i suspect bug 198408 is probably the real fix for the issue this request is trying to address.

Comment 19

[David Lawrence [:dkl]]

For reference. When applying this patch to my recent BMO snapshot and showing all products:

mod_cgi:
Transactions:		          10 hits
Availability:		      100.00 %
Elapsed time:		       37.18 secs
Data transferred:	        9.92 MB
Response time:		        3.62 secs
Transaction rate:	        0.27 trans/sec
Throughput:		        0.27 MB/sec
Concurrency:		        0.97
Successful transactions:          10
Failed transactions:	           0
Longest transaction:	        3.71
Shortest transaction:	        3.54

mod_perl:
Transactions:		          10 hits
Availability:		      100.00 %
Elapsed time:		       36.38 secs
Data transferred:	        9.92 MB
Response time:		        3.14 secs
Transaction rate:	        0.27 trans/sec
Throughput:		        0.27 MB/sec
Concurrency:		        0.86
Successful transactions:          10
Failed transactions:	           0
Longest transaction:	        4.88
Shortest transaction:	        2.88

dkl

Comment 20

[Andre Klapper]

(In reply to Frédéric Buclin from comment #17)
> I want more evidence that this feature would be useful to others (i.e. other
> smaller installations) before adding this feature, with or without a parameter.

I would have needed this feature several times in the past for getting an overview of the existing product/component/assignee/QAcontact structure in certain Bugzilla installations (in order to reorganize them, after a few more weeks of thinking and analyzing problems with the existing structure) so I ended up concatenating all per-product describecomponents.cgi pages, stripping some of the HTML noise around, and printing the resulting document (as such stuff works best for me on paper).
And I see the same problem coming up soon again for one of "my" Bugzillas so I'd appreciate an easier way.
Maybe make it only available to some groups that have usecases (admin group?) to reduce the load?

Comment 21

[David Lawrence [:dkl]]

I would not be opposed to making it only display for members of a particular group. Any others want to comment?

dkl

Comment 22

[Frédéric Buclin]

(In reply to David Lawrence [:dkl] from comment #21)
> I would not be opposed to making it only display for members of a particular
> group. Any others want to comment?

I want! :) If the "Show all components from all products" link is controlled by a group, then it should work the same way as the chartgroup parameter. If this parameter is left empty, then nobody can display all components at once, else only members of that group can display them all. So large installations can easily leave this parameter empty, and small ones can set it to e.g. canconfirm, or create a "everyone" group and put all users in it.

In the backend, you shouldn't pass product=__all as it could match a product name. Better is to pass full=1 to not mess the existing logic.

Comment 23

[David Lawrence [:dkl]]

Attachment: Patch to show all products in describecomponents.cgi (v5)

Changed to no longer require a new param and instead uses the chartgroup param to determine if all products are available. Also I changed to use full=1 instead of product=__all. 

dkl

Comment 25

[Frédéric Buclin]

(In reply to David Lawrence [:dkl] from comment #23)
> Changed to no longer require a new param and instead uses the chartgroup
> param to determine if all products are available.

When I mentioned chartgroup in my previous comment, I meant that it should depend on a group in the same way New Charts depend on chartgroup. I didn't mean we should reuse this group here. We still don't have a good idea of which target audience would need this feature. I think it's a bad idea to let everybody view all components from all products at once. As I said in comment 17, with few products, the confusion is almost non-existent, and with many products, the list would be so long that it would be mostly useless. So the question is: who would benefit from this feature? The chartgroup is certainly not the right choice. If this feature is useful to admins managing products and components, then requiring editcomponents privs would make more sense. If this feature is not really useful to anyone but just cool to have, then it should be wontfix.

Comment 26

[Andre Klapper]

(In reply to Frédéric Buclin from comment #25)
> If this feature is useful to
> admins managing products and components, then requiring editcomponents privs
> would make more sense. If this feature is not really useful to anyone but
> just cool to have, then it should be wontfix.

My usecase is in comment 20. It's rare, but it would be useful to me when I 
* try to understand the structure of a Bugzilla instance that I've become an admin of
* would like to reorganize the "organically grown" structure of products and components.

IMHO this feature can be restricted to editcomponents or even admingroup.

Comment 27

[Frédéric Buclin]

Comment on attachment 674772 [details] [diff] [review]
Patch to show all products in describecomponents.cgi (v5)

>=== modified file 'describecomponents.cgi'

>+if ($cgi->param('full')
>+    && Bugzilla->params->{'chartgroup'}
>+    && $user->in_group(Bugzilla->params->{'chartgroup'}))

It seems this feature is only useful to admins having to manage products and components, so let's look at editcomponents privs instead.


>+    @products = @{$user->get_selectable_products};

We want $user->get_accessible_products here, else we are going to regress bug 544434 again.


>+elsif ($product_name) {
>+    @products = ( Bugzilla::Product->new({'name' => $product_name}) );
>+    $products[0] || ThrowUserError("no_products");

This error message is wrong. Just because you tried to access a product you cannot see doesn't mean you cannot access any product. Instead, you must call ->check instead of ->new. It will throw the appropriate error message for us.


>+unless (@products && $user->can_access_product($products[0]->name)) {

There is no need to call $user->can_access_product if we call ->check above. This makes the code more robust.


>=== modified file 'template/en/default/reports/components.html.tmpl'

The UI doesn't look nice. First of all, each product has its own width for columns (depending on the length of component names and descriptions) and it's very hard to parse the page. Also, there is no link at the top of the page and it's hard to find some specific product in the page. Products should be listed at the top of the page and clicking on the name of a product should bring you to the correct section of the page. Bonus point if we can collapse each section indidivually. I have very few products and components, but it's already difficult to find something in the page. Another bonus point to start the description of the component in the Component column (i.e. use all 3 columns to display component descriptions. This would make it clearer that the text is the description of the component).