Bug 445607 - RFE: XML-RPC method bug.setAttachmentsMIMETypes(bug.structData["attachments"])
Summary: RFE: XML-RPC method bug.setAttachmentsMIMETypes(bug.structData["attachments"])
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Bugzilla
Classification: Community
Component: WebService
Version: devel
Hardware: All
OS: Linux
low
low
Target Milestone: ---
Assignee: David Lawrence
QA Contact:
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-05-07 22:35 UTC by Matěj Cepl
Modified: 2008-06-02 05:00 UTC (History)
0 users

Fixed In Version: 2.18
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-05-29 15:57:53 UTC
Embargoed:


Attachments (Terms of Use)
script using the requested method (1.57 KB, text/x-python)
2008-05-07 22:35 UTC, Matěj Cepl
no flags Details
Patch adding bugzilla.updateAttachMimeType() to 3.2 (v1) (3.44 KB, patch)
2008-05-28 16:40 UTC, David Lawrence
no flags Details | Diff
Patch adding bugzilla.updateAttachMimeType() to 2.18 (v1) (3.69 KB, patch)
2008-05-28 16:42 UTC, David Lawrence
nelhawar: review+
Details | Diff
Sample perl script showing use of bugzilla.updateAttachMimeType() (1.35 KB, text/plain)
2008-05-28 16:45 UTC, David Lawrence
no flags Details
Patch adding bugzilla.updateAttachMimeType() to 3.2 (v2) (3.53 KB, patch)
2008-05-29 03:45 UTC, David Lawrence
no flags Details | Diff
Patch adding bugzilla.updateAttachMimeType() to 2.18 (v2) (3.70 KB, patch)
2008-05-29 03:50 UTC, David Lawrence
no flags Details | Diff

Description Matěj Cepl 2008-05-07 22:35:48 UTC
Description of problem:
The method in the XML-RPC interface for setting all attachments MIME types
according to changes in the bug.structData["attachments"]. Then resolving the
issue in bug 208714 (which ISABUG IMHO) would be workarounded by using the
attached Python script (uses python-bugzilla).

Comment 1 Matěj Cepl 2008-05-07 22:35:48 UTC
Created attachment 304813 [details]
script using the requested method

Comment 2 David Lawrence 2008-05-28 16:40:40 UTC
Created attachment 306946 [details]
Patch adding bugzilla.updateAttachMimeType() to 3.2 (v1)

Attaching patch to add support for updating mime type of an attachment based on
attach_id to Bugzilla 3.2 (devel).

Noura, can you take a look at this please?

Thanks
Dave

Comment 3 David Lawrence 2008-05-28 16:42:42 UTC
Created attachment 306947 [details]
Patch adding bugzilla.updateAttachMimeType() to 2.18 (v1)

Attaching patch to add support for updating mime type of an attachment based on

attach_id to Bugzilla 2.18 (production).

Noura, can you take a look at this please?

Thanks
Dave

Comment 4 David Lawrence 2008-05-28 16:45:01 UTC
Created attachment 306948 [details]
Sample perl script showing use of bugzilla.updateAttachMimeType()

Comment 5 Noura El hawary 2008-05-29 03:13:53 UTC
Comment on attachment 306946 [details]
Patch adding bugzilla.updateAttachMimeType() to 3.2 (v1)

Hi Dave,

I reviewed the patch and few comments are inline.

>+    $attach_id || ThrowUserError('invalid_attach_id');
>+    $mime_type || ThrowUserError('invalid_content_type');
>+

should those 2 be:

$attach_id || ThrowCodeError('param_required', { param => 'attach_id' });
$mime_type || ThrowCodeError('param_required', { param => 'mime_type' });


>+    if ( not Bugzilla->user->in_group("editbugs") 
>+            && Bugzilla->user->id != $submitter_id ) {

also the not needs to be in brackets or it is applied to both conditions:

    if ( (not Bugzilla->user->in_group("editbugs"))
	    && Bugzilla->user->id != $submitter_id ) {


>+        
>+        $data_ref->{nomail} ||= 0;
>+        
>+        # Generate email
>+        return Bugzilla::BugMail::Send( $bug_id,
>+            { changer => Bugzilla->user->login }, $data_ref->{nomail} );
>+    }
>+} 
>+

the nomail is not part of the BugMail args in 3.2 so I guess this can be
implemented as an if condition:

	$data_ref->{nomail} ||= 0;

	# Generate email
	if(not $data_ref->{nomail}) {
	    return Bugzilla::BugMail::Send( $bug_id,
		{ changer => Bugzilla->user->login });
	}


The rest looks good to me, I tested it and it works as expected with the above
fixes.

Thanks,
Noura

Comment 6 Noura El hawary 2008-05-29 03:29:02 UTC
Comment on attachment 306947 [details]
Patch adding bugzilla.updateAttachMimeType() to 2.18 (v1)


>+    if ( not ::UserInGroup("editbugs") 
>+            && Bugzilla->user->id != $submitter_id ) {
>+        ThrowUserError( "illegal_attachment_edit",
>+            { attach_id => $attach_id } );
>

Hi Dave , that patch works pretty good except for the not bit :

    if ( (not ::UserInGroup("editbugs"))
	    && Bugzilla->user->id != $submitter_id ) {


Cheers,
Noura

Comment 7 David Lawrence 2008-05-29 03:45:09 UTC
Created attachment 307011 [details]
Patch adding bugzilla.updateAttachMimeType() to 3.2 (v2)

Thanks Noura, I have made the changes you suggested.

Please review
Dave

Comment 8 David Lawrence 2008-05-29 03:50:30 UTC
Created attachment 307012 [details]
Patch adding bugzilla.updateAttachMimeType() to 2.18 (v2)

Thanks Noura. I have made your suggested changes.

Please review.
Dave

Comment 9 David Lawrence 2008-05-29 03:51:38 UTC
(In reply to comment #8)
> Created an attachment (id=307012) [edit]
> Patch adding bugzilla.updateAttachMimeType() to 2.18 (v2)
> 
> Thanks Noura. I have made your suggested changes.
> 
> Please review.
> Dave

Actually nevermind this one. You already gave a review+.

Will check in.
Dave

Comment 10 Matěj Cepl 2008-05-29 07:35:12 UTC
Thanks a lot.

Comment 11 David Lawrence 2008-05-29 15:57:53 UTC
This has now been pushed live to both 2.18 and 3.2. The following gives some
info on how to use the new method.

Dave

"updateAttachMimeType($data_ref, $username, $password)"
           Update the attachment mime type of an attachment. The first argument
is a data hash containing information on the new MIME type and the attachment id
that you want to
           act on.

               $data_ref = {
                   "attach_id" => "<Attachment ID>",
                       # Attachment ID to perform MIME type change on.
                   "mime_type"  => "<New MIME Type Value>",
                       # Legal MIME type value that you want to change the
attachment to.
                   "nomail" => 0,
                       # OPTIONAL Flag that is either 1 or 0 if you want email
to be sent or not for this change
               };

Comment 12 Noura El hawary 2008-06-02 05:00:41 UTC
Comment on attachment 307011 [details]
Patch adding bugzilla.updateAttachMimeType() to 3.2 (v2)

reviewed a previous patch


Note You need to log in before you can comment on or make changes to this bug.