Bug 445607 - RFE: XML-RPC method bug.setAttachmentsMIMETypes(bug.structData["attachments"])
RFE: XML-RPC method bug.setAttachmentsMIMETypes(bug.structData["attachments"])
Status: CLOSED CURRENTRELEASE
Product: Bugzilla
Classification: Community
Component: WebService (Show other bugs)
devel
All Linux
low Severity low (vote)
: ---
: ---
Assigned To: David Lawrence
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-05-07 18:35 EDT by Matěj Cepl
Modified: 2008-06-02 01:00 EDT (History)
0 users

See Also:
Fixed In Version: 2.18
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-05-29 11:57:53 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


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

  None (edit)
Description Matěj Cepl 2008-05-07 18:35:48 EDT
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 18:35:48 EDT
Created attachment 304813 [details]
script using the requested method
Comment 2 David Lawrence 2008-05-28 12:40:40 EDT
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 12:42:42 EDT
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 12:45:01 EDT
Created attachment 306948 [details]
Sample perl script showing use of bugzilla.updateAttachMimeType()
Comment 5 Noura El hawary 2008-05-28 23:13:53 EDT
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-28 23:29:02 EDT
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-28 23:45:09 EDT
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-28 23:50:30 EDT
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-28 23:51:38 EDT
(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 03:35:12 EDT
Thanks a lot.
Comment 11 David Lawrence 2008-05-29 11:57:53 EDT
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 01:00:41 EDT
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.