custom field info is not imported into database from xml file

RESOLVED FIXED in Bugzilla 3.0

Status

()

RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: maddenjo99, Assigned: erik)

Tracking

2.23.3
Bugzilla 3.0
Bug Flags:
approval +
approval3.0 +

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

12 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.8.1.1) Gecko/20061204 Firefox/2.0.0.1
Build Identifier: Bugzilla 2.23.3

Test export/import: An xml file containing the bug information was generated from the show_bug form.  The xml file was modified slightly to allow it to be imported back into the original database.
The importxml.pl was used on the command line to import the new bug.  No errors occurred.  In verbose mode, the importxml script indicated that the custom field was parsed however the SQL statement to update the bug database does not contain the field name or field data.
On examination of the new bug in show_bugs, the custom field is empty.  

Reproducible: Always

Steps to Reproduce:
1.Select bug in database and export to xml file.
2.Modify file slightly i.e. new bug id, url etc.
3.import bug from file using importxml.pl on command line.
Actual Results:  
The bug imported using importxml.pl does not contain the information associated with its custom fields.

Expected Results:  
The custom field information is also imported.

Comment 1

12 years ago
Valid request. Note that importxml.pl will have to make sure the imported custom fields exist on the target installation, and if the field is a dropdown menu, then the imported value must be a valid one.
Severity: normal → enhancement
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
Hardware: PC → All
Version: unspecified → 2.23.3
(Assignee)

Comment 2

12 years ago
Created attachment 259457 [details] [diff] [review]
Add custom field data to bug query and values, or add error line if illegal value encountered.

Skip to next custom field  if this field isn't in this bug record.

Please review. I've tested this patch on a live 2.23.4 installation.
Added work e-mail address as contributor since this was paid for by them.
Any style or additional pointers appreciated.
-e
Attachment #259457 - Flags: review?(LpSolit)

Comment 3

12 years ago
Comment on attachment 259457 [details] [diff] [review]
Add custom field data to bug query and values, or add error line if illegal value encountered.

Skip to next custom field  if this field isn't in this bug record.

>+    foreach my $custom_field (Bugzilla->custom_field_names) {
>+        my $is_defined = defined( $bug_fields{$custom_field} );

Instead of defining $is_defined, you should call "next" if the value is undefined. This would make your code smaller and cleaner.


>+        my $is_well_formed = check_field( $custom_field, scalar $bug_fields{$custom_field}, undef, ERR_LEVEL );

This is an invalid call for all free-text fields. check_field() will look at a table named $custom_field which doesn't exist. You get a SQL crash.
Attachment #259457 - Flags: review?(LpSolit) → review-
(Assignee)

Comment 4

12 years ago
Thanks, Frederic. I only used this with dropdown fields. I'll take a look at handling all types of custom fields.

Regarding $is_defined, code is laid out so I can inspect the contents of each value to boolean in the debugger. The code I recycled from inside importxml.pl (I think it was the component section) originally uses the style of putting the code inside the boolean, but it makes inspection difficult.

If I call next to skip to next item in loop, won't I skip the opportunity to insert an error message into the log?

Comment 5

12 years ago
(In reply to comment #4)
> If I call next to skip to next item in loop, won't I skip the opportunity to
> insert an error message into the log?

No, because you already skip the error message if the value is undefined.
(Assignee)

Comment 6

12 years ago
Comment on attachment 259457 [details] [diff] [review]
Add custom field data to bug query and values, or add error line if illegal value encountered.

Skip to next custom field  if this field isn't in this bug record.

>Index: importxml.pl
>===================================================================
>RCS file: /cvsroot/mozilla/webtools/bugzilla/importxml.pl,v
>retrieving revision 1.74
>diff -u -r1.74 importxml.pl
>--- importxml.pl	6 Feb 2007 14:57:01 -0000	1.74
>+++ importxml.pl	23 Mar 2007 21:52:58 -0000
>@@ -22,6 +22,7 @@
> #                 Gregary Hendricks <ghendricks@novell.com>
> #                 Vance Baarda <vrb@novell.com>
> #                 Guzman Braso <gbn@hqso.net>
>+#                 Erik Purins <epurins@day1studios.com>
> 
> # This script reads in xml bug data from standard input and inserts
> # a new bug into bugzilla. Everything before the beginning <?xml line
>@@ -1007,6 +1008,19 @@
>     push( @query,  "bug_status" );
>     push( @values, $status );
> 
>+    # Custom fields
>+    foreach my $custom_field (Bugzilla->custom_field_names) {
>+        next unless my $is_defined = defined( $bug_fields{$custom_field} );
>+        my $is_well_formed = check_field( $custom_field, scalar $bug_fields{$custom_field}, undef, ERR_LEVEL );
>+        if ($is_defined && $is_well_formed) {
>+            push( @query,  $custom_field );
>+            push( @values, $bug_fields{$custom_field} );
>+        } else {
>+            if ($is_defined) {
>+                $err .= "Skipping illegal value \"$bug_fields{$custom_field}\" in $custom_field.\n" ;
>+            }
>+        }
>+    }
> 
>     # For the sake of sanitycheck.cgi we do this.
>     # Update lastdiffed if you do not want to have mail sent
>
Attachment #259457 - Attachment description: Add custom field data to bug query and values, or add error line if illegal value encountered. → Add custom field data to bug query and values, or add error line if illegal value encountered. Skip to next custom field if this field isn't in this bug record.
(Assignee)

Comment 7

12 years ago
Frederic, does that look ok?
(Assignee)

Comment 8

12 years ago
No, I see. The boolean is no longer needed because I'll be in the next loop. I'll take another look on monday.
(Assignee)

Comment 9

12 years ago
Created attachment 259818 [details] [diff] [review]
updated to check field type, next if field is undefined in this record.

Follow-up patch based on review. Any additional pointers and comments appreciated.
Attachment #259457 - Attachment is obsolete: true
Attachment #259818 - Flags: review?(LpSolit)

Comment 10

12 years ago
Comment on attachment 259818 [details] [diff] [review]
updated to check field type, next if field is undefined in this record.

>+                push( @values, $bug_fields{$custom_field} ); # Should I clean_text() on this text?

Yes, you should.


>+            $err .= "Type of custom field $custom_field is an unhandled FIELD_TYPE_: $field->type\n" ;

Nit: put $field->type outside quotes.


Your patch is working fine and both comments can be addressed on checkin. r=LpSolit
Attachment #259818 - Flags: review?(LpSolit) → review+

Comment 11

12 years ago
Let's take it for 3.0 as we may consider this as dataloss. And the code is safe anyway.
Assignee: import-export → shaggy+bugzilla
Flags: approval3.0+
Flags: approval+
Target Milestone: --- → Bugzilla 3.0

Comment 12

12 years ago
Created attachment 260529 [details] [diff] [review]
patch with comments fixed

Here is the patch I'm going to commit, which removes tabs and fixes the comments I made.
Attachment #259818 - Attachment is obsolete: true

Comment 13

12 years ago
tip:

Checking in importxml.pl;
/cvsroot/mozilla/webtools/bugzilla/importxml.pl,v  <--  importxml.pl
new revision: 1.75; previous revision: 1.74
done

3.0 RC1:

Checking in importxml.pl;
/cvsroot/mozilla/webtools/bugzilla/importxml.pl,v  <--  importxml.pl
new revision: 1.74.2.1; previous revision: 1.74
done
Severity: enhancement → normal
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.