Bug 426376

Summary: 3.8.2: Ajax Optimizations for Show Bug Page
Product: [Community] Bugzilla Reporter: David Lawrence <dkl>
Component: User InterfaceAssignee: David Lawrence <dkl>
Status: CLOSED NEXTRELEASE QA Contact:
Severity: medium Docs Contact:
Priority: high    
Version: 3.2   
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard: 2 hours
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2008-05-28 03:48:55 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 406071, 406181    
Attachments:
Description Flags
Patch to use ajax to update component drop down on show_bug.cgi (v1)
none
Patch to use ajax to update component drop down on show_bug.cgi (v2) kbaker: review+, nelhawar: review+

Description David Lawrence 2007-12-20 16:03:58 UTC
+++ This bug was initially created as a clone of Bug #406181 +++

Description:
On certain pages Ajax optimizations are used to speed up page loading time. Red
Hat has a high number of components for certain products. The component lists
cause a large amount of HTML having to be downloaded by the client browser. The
Ajax functions allow the UI to only show the components needed depending on
which products are selected.

Function Requirements:
Only load full list products, components, and versions on show_bug.cgi if user
requests it. Link is located next to the appropriate drop down that when clicked
will download the full list and re-populate the drop down menu with the full
list of choices.

Comment 1 David Lawrence 2007-12-20 18:50:35 UTC
LOC Estimation:

Code is shared with other pages such as query.cgi. Javascript code libraries is 
counted in bug 426374.

templates: 60
selenium tests to verify the proper operation of product/version/components: 2 hours

LOC Total: 60


Comment 2 David Lawrence 2008-04-02 16:22:30 UTC
Created attachment 300091 [details]
Patch to use ajax to update component drop down on show_bug.cgi (v1)

I have created a patch to convert the component drop down on show_bug.cgi to
update its list using ajax on demand. This cuts down the page load time for
products with large number of components such as Fedora. It starts out as a
single value which is the current component for the bug. If a use clicks on
Update Components it then updates the list to include all components,
reselecting the current component. The Update Components links should then
disappear. This is using an updated bugzilla.getProductDetails() XMLRPC call
instead of a custom ajax server side method. This allows code to be reused for
multiple purposes. I have this patch running on
https://bugdev.devel.redhat.com/bugzilla if you want to test it.

Please review.
Dave

Comment 3 Noura El hawary 2008-04-10 13:30:12 UTC
Hi Dave,, is the attached patch missing aything? I applied it to my localhost
and couldn't see update components link in show_bug.cgi also checked bugdev and
couldn't see it.



Comment 4 Noura El hawary 2008-04-10 14:37:09 UTC
Dave ,, Just in case you will wonder if i have javascript enabled ,, I doubled
checked firefox and it was enabled.

Noura

Comment 5 David Lawrence 2008-04-10 15:00:39 UTC
(In reply to comment #4)
> Dave ,, Just in case you will wonder if i have javascript enabled ,, I doubled
> checked firefox and it was enabled.
> 
> Noura

I have two many patches going on at once unfortunately :) This one requires the
user.javascript patch to be applied in bug 426386 to work. Also I need to add a
new patch for this since I have found some bugs that I have fixed.

New patch coming.
Dave

Comment 6 Kevin Baker 2008-04-16 18:15:38 UTC
Is this on bugdev? 

Comment 7 David Lawrence 2008-04-16 20:03:43 UTC
Created attachment 302663 [details]
Patch to use ajax to update component drop down on show_bug.cgi (v2)

Attaching new patch. Redesigned the way this is done pretty significantly.
Using a  new Product.get method to get the component info. Also made the
component field look similar to assignee, qa contact, etc. on the UI. It shows
the value as a readonly with the (edit) link beside it if the user can edit the
field. Once you click on the (edit) link, it displays an updated drop down with
the full list of components.

Please review
Dave

Comment 8 David Lawrence 2008-04-16 20:04:39 UTC
Comment on attachment 302663 [details]
Patch to use ajax to update component drop down on show_bug.cgi (v2)

Noura please review paying close attention to the new Product.get method I
added. Please let me know if that could be done better. 

Thanks
Dave

Comment 9 Kevin Baker 2008-04-16 20:23:04 UTC
Comment on attachment 302663 [details]
Patch to use ajax to update component drop down on show_bug.cgi (v2)

+    my @ids = ref($params->{ids}) ? @{$params->{ids}} : ($params->{ids});
+    my $obj_by_ids = Bugzilla::Product->new_from_list(\@ids) if
$params->{ids};
+
+    my @names = ref($params->{names}) =~ /ARRAY/ ? @{$params->{names}} :
+	 ($params->{names});

XXX-kab-start

Nitpik on: 
why pull @names like this and @ids above differently?
And using a regex like that is very very slightly error prone.
my @names = ref($params->{names}) =~ /ARRAY/ ? @{$params->{names}} :
Howabout
+    my @names = ref($params->{names}) eq 'ARRAY' ? @{$params->{names}} :
+	 ($params->{names});
Is $params->{names} always guaranteed to have a value? If not then this will
cause lots of annoying warnings in the log. 
And is a ref in $params->{ids} always guaranteed to be an ARRAY ref?
+    my @ids = ref($params->{ids}) eq 'ARRAY' ? @{$params->{ids}} :
($params->{ids});
nitpik off
XXX-kab-end

Comment 10 Kevin Baker 2008-04-16 20:24:50 UTC
Comment on attachment 302663 [details]
Patch to use ajax to update component drop down on show_bug.cgi (v2)

Looks ok to me. My javascript chops aren't that hot to pick up any subtle
issues but looks straight forward.

Comment 11 David Lawrence 2008-04-16 20:27:33 UTC
(In reply to comment #9)
> XXX-kab-start
> 
> Nitpik on: 
> why pull @names like this and @ids above differently?
> And using a regex like that is very very slightly error prone.
> my @names = ref($params->{names}) =~ /ARRAY/ ? @{$params->{names}} :
> Howabout
> +    my @names = ref($params->{names}) eq 'ARRAY' ? @{$params->{names}} :
> +	 ($params->{names});
> Is $params->{names} always guaranteed to have a value? If not then this will
> cause lots of annoying warnings in the log. 
> And is a ref in $params->{ids} always guaranteed to be an ARRAY ref?
> +    my @ids = ref($params->{ids}) eq 'ARRAY' ? @{$params->{ids}} :
> ($params->{ids});
> nitpik off
> XXX-kab-end
> 

Pulled from work done by Noura in
Bugzilla/WebService/Component::_get_accessible_components(). This was the code
style that was accepted upstream.

Comment 12 Noura El hawary 2008-04-17 11:49:43 UTC
Comment on attachment 302663 [details]
Patch to use ajax to update component drop down on show_bug.cgi (v2)

Hi Dave,,

Patch looks good to me and the xmlrpc function Product.get works nicely. Only
one thing i noticed while testing in bugdev.devel was that when updating
component and clicking the submit button you get redirected to
process_bug.cgi,, if you try to update the component in that process_bug.cgi
the component list fails to load which is not the case currently in RHBZ 2.18

Noura

Comment 13 David Lawrence 2008-04-17 14:41:18 UTC
(In reply to comment #12)
> (From update of attachment 302663 [details] [edit])
> Hi Dave,,
> 
> Patch looks good to me and the xmlrpc function Product.get works nicely. Only
> one thing i noticed while testing in bugdev.devel was that when updating
> component and clicking the submit button you get redirected to
> process_bug.cgi,, if you try to update the component in that process_bug.cgi
> the component list fails to load which is not the case currently in RHBZ 2.18
> 
> Noura

Thanks for the review Noura. You did in fact find a bug where I was forgetting
to load the necessary js files in bug/create/created.html.tmpl and
bug/process/header.html.tmpl in the PROCESS header section. I have added them in
and it is working fine now. Will check this in to CVS.

Dave
> 



Comment 14 David Lawrence 2008-04-17 16:31:34 UTC
Worked 8 hours.

Comment 15 Noura El hawary 2008-05-09 00:31:32 UTC
Comment on attachment 300091 [details]
Patch to use ajax to update component drop down on show_bug.cgi (v1)

newer version has been reviewed.