307328 - show_bug.cgi?ctype=xml should allow the option of including flags

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, defect)

Description

[Ian Roberts]

User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1; .NET CLR 1.1.4322)
Build Identifier: 

It would be useful if the newly added flags feature was accessable in the xml 
of a bug.

Reproducible: Always

Steps to Reproduce:
1. Create or find bug and with flag set on it.
2. Get the xml for that bug, https://bugzilla.mozilla.org/show_bug.cgi?
id=302669&ctype=xml
3. XML does not include flag - the DTD does not have any obvious provision for 
flags so I guess the DTD would need to be extended.

Actual Results:  
No Flag data in XML

Expected Results:  
Data relating to flags in XML.

Comment 1

[Greg Hendricks]

Need to add flag data to bugzilla DTD and to importxml.pl.

Comment 2

[Greg Hendricks]

Attachment: Udated for bugzilla.dtd

This includes a number of addtions to come up to date with the dtd

Comment 3

[Greg Hendricks]

Attachment: v2

Changed field order to match dtd

Comment 4

[Greg Hendricks]

Attachment: v3

Fixed per live review:
<LpSolit> <long_desc isprivate="[% IF c.isprivate %]1[% ELSE %]0[% END %]"> can be reduced to <long_desc isprivate="[% c.isprivate FILTER xml %]">
<ghendricks> cool
<LpSolit> ghendricks: why all these [% IF a.isprivate %]isprivate="1"[% END %] ?
<LpSolit> ghendricks: isprivate="[% a.isprivate FILTER xml %]"
<LpSolit> ghendricks: I think importxml.pl should support having isprivate="0"
<ghendricks> it does
<LpSolit> the cleaner the code, the easier to debug it ;)
<ghendricks> or rather it checks for isprivate=1
<LpSolit> ghendricks: you replace <ctype> by <type>; do you really want this change?
<LpSolit>           <attachment
<LpSolit>               isobsolete="0"
<LpSolit>               ispatch="1"
<LpSolit>               isprivate="0"
<LpSolit>           >
<LpSolit> ghendricks: I don't know what you think, but having these attributes displayed in all cases really makes sense
<LpSolit> ghendricks: this way you are sure they are 0 
<LpSolit> ghendricks: a missing attribute doesn't tell you if it's really the case
<ghendricks> LpSolit: agreed
<ghendricks> ok, I will make those changes
<ghendricks> LpSolit: the <ctype> to <type> was made in the dtd
<ghendricks> this change is to make sure in conforms
<LpSolit>           <attachment
<LpSolit>               isobsolete="[% a.isobsolete FILTER xml %]"
<LpSolit>               ispatch="[% a.ispatch FILTER xml %]"
<LpSolit>               isprivate="[% a.isprivate FILTER xml %]"
<LpSolit>           >
<LpSolit> ghendricks: this is what I wrote; works fine
<ghendricks> good
<LpSolit> ghendricks: I have a nit:
<ghendricks> are you going to go ahead and merge these into your patch then?
<LpSolit>       [% FOREACH type = bug.flag_types %]
<LpSolit>         [% FOREACH flag = type.flags %]
<LpSolit>         <flag name="[% type.name FILTER xml %]"
<LpSolit> the indentation of <flag is incorrect
<LpSolit> ghendricks: your patch is ok for checkin; I will update mine accordingly
<LpSolit> ghendricks: update your patch and I will r+ it
<LpSolit> ghendricks: include these changes in your patch
<ghendricks> ok

Comment 5

[Frédéric Buclin]

Comment on attachment 202109 [details] [diff] [review]
v3

r=LpSolit

Comment 6

[Myk Melez [:myk] [@mykmelez]]

ghendricks and lpsolit have both pointed out that without this patch importxml.pl's usefulness is seriously compromised.  That, plus the relatively simple nature of the patch, justifies taking this during the freeze for 2.22.

But note that at least one part of this patch conflicts with the patch for bug 315157, so coordinate checkin with that patch.

Comment 7

[Frédéric Buclin]

Checking in template/en/default/bug/show.xml.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/show.xml.tmpl,v  <--  show.xml.tmpl
new revision: 1.11; previous revision: 1.10
done