Edgewall Software
Modify

Opened 7 years ago

Closed 7 years ago

#12905 closed enhancement (fixed)

Make it easier to get file upload from request

Reported by: Ryan J Ollos Owned by: Ryan J Ollos
Priority: normal Milestone: 1.3.3
Component: attachment Version:
Severity: normal Keywords:
Cc: Branch:
Release Notes:
API Changes:

Added getfile and getfilelist arguments to req.args.

Internal Changes:

Description

Discussed starting in comment:9:ticket:11395.

Attachments (2)

awesomeattachments.patch (3.8 KB ) - added by Ryan J Ollos 7 years ago.
awesomeattachments.1.patch (4.3 KB ) - added by Ryan J Ollos 7 years ago.

Download all attachments as: .zip

Change History (7)

by Ryan J Ollos, 7 years ago

Attachment: awesomeattachments.patch added

comment:1 by Ryan J Ollos, 7 years ago

I thought it would be better to add a method to _RequestArgs: log:rjollos.git:t12905_request_getfile.

Refactoring of AwesomeAttachmentsPlugin in awesomeattachments.patch

comment:2 by Jun Omae, 7 years ago

3 things.

  1. _normalized_filename would be removed from trac/attachment.py, however some plugins can use the method. At least, it is used from my private plugin.
    # Compatibility for Trac 1.2. Will be removed in 1.5.1.
    _normalized_filename = normalized_filename
    
  2. I don't consider we should raise an exception from getfile(). Rejecting an empty file and no filename is just requirement of AttachmentModule, is not requirement of another component.
  3. It might be good to be normalize_filename rather than normalized_filename.

comment:3 by Ryan J Ollos, 7 years ago

I added the comment:2 suggestions. I'll also add some additional tests.

About the method naming, I'd prefer get_file and get_file_list to getfile and getfilelist, but wanted to remain consistent with existing method naming: getbool, getint, getfirst, getlist.

by Ryan J Ollos, 7 years ago

Attachment: awesomeattachments.1.patch added

comment:4 by Ryan J Ollos, 7 years ago

Rebased and additional tests added: [cc9cace6a/rjollos.git]. Tests are currently running on hosted CI and I'll revise if any failures.

Revised refactoring of AwesomeAttachmentsPlugin in awesomeattachments.1.patch.

comment:5 by Ryan J Ollos, 7 years ago

API Changes: modified (diff)
Resolution: fixed
Status: assignedclosed

Committed to trunk in r16304.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Ryan J Ollos.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Ryan J Ollos to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.