Modify

Opened 13 years ago

Last modified 8 years ago

#9276 new enhancement

Add additional options for other attachment types and actions

Reported by: David.Byrne@… Owned by: Franz
Priority: normal Component: AttachmentPolicyPlugin
Severity: normal Keywords:
Cc: Trac Release: 0.12

Description

I could use something like this if it was extended further. I need to allow some users the ability to attach files to milestones without being able to modify milestones. I was wondering if this could be enhanced to match the permissions map. This would create 9 total permissions - TICKET_ATTACHMENT_CREATE, WIKI_ATTACHMENT_CREATE, MILESTONE_ATTACHMENT_CREATE, TICKET_ATTACHMENT_VIEW, WIKI_ATTACHMENT_VIEW, MILESTONE_ATTACHMENT_VIEW, TICKET_ATTACHMENT_DELETE, WIKI_ATTACHMENT_DELETE, MILESTONE_ATTACHMENT_DELETE. The view ones are probably not as important since they map to being able to view the associated entity. Your plugin already handles the TICKET_ATTACHMENT_DELETE. My need is on the MILESTONE_ATTACHMENT_CREATE. If this is something that you cannot get to shortly, please let me know. I may be able to give you a patch file for handling this.

Attachments (2)

attachment_policy.py (2.8 KB) - added by David.Byrne@… 12 years ago.
modified attachment_policy.py
att_pol_patch.diff (4.9 KB) - added by David.Byrne@… 12 years ago.
unified diff of change

Download all attachments as: .zip

Change History (11)

comment:1 in reply to:  description Changed 13 years ago by Franz

Well, most of the mentioned permissions are already in some other permissions included:

  • TICKET_ATTACHMENT_CREATE: TICKET_APPEND
  • MILESTONE_ATTACHMENT_CREATE: MILESTONE_MODIFY

and some more, see TracPermissions for all permissions for trac

The AttachmentPolicyPlugin works also for attachments of milestones and wiki-pages - so it implies these permissions: TICKET_ATTACHMENT_DELETE, WIKI_ATTACHMENT_DELETE, MILESTONE_ATTACHMENT_DELETE

Last edited 8 years ago by Ryan J Ollos (previous) (diff)

comment:2 Changed 13 years ago by David.Byrne@…

I understand, but you're missing my point. I need to allow certain users to attach files to a milestone without being able to modify the milestone. This is similar to the reason that you created this plugin to begin with. You needed to allow users to delete ticket attachments without giving them TICKET_ADMIN. I've seen the mapping in the LegacyAttachmentPolicy. Also, it appears to me that you specifically reference TICKET_ATTACHMENT_DELETE in your code, so I'm not sure how it already handles MILESTONE_ATTACHMENT_DELETE or WIKI_ATTACHMENT_DELETE permissions. As I said prior, if you cannot get to this, I will just make the modifications locally. I just hate to see multiple plugins on trac-hacks that do pretty much the same thing.

comment:3 in reply to:  2 Changed 13 years ago by Franz

Replying to David.Byrne@us.xchanging.com:

I understand, but you're missing my point.

I understand your point and it shouldn't be very difficult, at least for all *_DELETE permissions, since this is just a copy of the existing code. It is a bit of work though. Please note that if you extend the plugin there should be an ATTACHMENT_ALL_DELETE permission, which includes all *_DELETE permissions.

Also, it appears to me that you specifically reference TICKET_ATTACHMENT_DELETE in your code, so I'm not sure how it already handles MILESTONE_ATTACHMENT_DELETE or WIKI_ATTACHMENT_DELETE permissions.

I haven't known until yesterday that I'm able to attach documents to milestones, so I tried to attach a file and delete it with a different user and it worked with the AttachmentPolicyPlugin. But don't ask me for internas - maybe Trac uses just TICKET_ATTACHMENT_DELETE for every attachment action.

comment:4 Changed 13 years ago by anonymous

Actually, the reason it somewhat works is because you are not checking the parent node. The attachment is being passed in and your code is not looking to see what the attachment is attached to. I looked at the LegacyAttachment code and there is a parent check via resource.parent.realm. If the attachment is attached to a ticket, this value will be 'ticket'. If attached to a wiki page, the value will be 'wiki'. The actual permission that Trac is looking for is 'ATTACHMENT_DELETE' in every delete case.

comment:5 Changed 12 years ago by David.Byrne@…

Priority: highnormal

I was the one to originally create this ticket (my email address has changed). I implemented this functionality. If you would like, I can attach the modified code to this ticket. I implemented all of the above (except for the view permissions) and added ATTACHMENT_ADMIN that would allow CREATE or DELETE for all of the types. I've also upgraded to Trac 1.0.1 recently.

Changed 12 years ago by David.Byrne@…

Attachment: attachment_policy.py added

modified attachment_policy.py

comment:6 in reply to:  5 Changed 12 years ago by Franz

Replying to David.Byrne@cam-ohio.com:

I was the one to originally create this ticket (my email address has changed). I implemented this functionality. If you would like, I can attach the modified code to this ticket. I implemented all of the above (except for the view permissions) and added ATTACHMENT_ADMIN that would allow CREATE or DELETE for all of the types. I've also upgraded to Trac 1.0.1 recently.

Yes, that would be great. You could attach the patch to this ticket as described in SubmittingPatches on Trac. Thus I could merge it to the existing source code and test your it, before submitting it to existing plugin.

Thank you.

Changed 12 years ago by David.Byrne@…

Attachment: att_pol_patch.diff added

unified diff of change

comment:7 Changed 12 years ago by David.Byrne@…

I've attached a unified diff of the changes, but I'm not sure if it is set up right. I have your original file in my packages directory along with my modified file. I did the diff of the two files, but I'm not sure if it is usable in that state or not. Please let me know if this works. If it doesn't, I will see what I can do to get you the patch in a usable format.

comment:8 in reply to:  7 ; Changed 12 years ago by Franz

Replying to David.Byrne@cam-ohio.com:

I've attached a unified diff of the changes, but I'm not sure if it is set up right. I have your original file in my packages directory along with my modified file. I did the diff of the two files, but I'm not sure if it is usable in that state or not. Please let me know if this works. If it doesn't, I will see what I can do to get you the patch in a usable format.

Sorry, I have overlooked your attached file attachment_policy.py. Your patch cannot be displayed by Trac-Hacks (don#T know why), but when downloading in raw format it looks good. Thank you for your work - at a first sight it looks well implemented.

However, I am not convinced about that solution, because it needs so many permissions. On the other hand it's the most flexible way of setting user rights.

For everybody who loves / needs David's solution, can download attachment_policy.py and replace it in source code and rebuild the plugin.

@David, if you like you can also attach an egg to this ticket so users can install the plugin easier. If you do so, please use a different version (e.g. 0.1.1) in setup.py and consider adding your name in the license clause (for license, see ticket #11085).

Last edited 11 years ago by Ryan J Ollos (previous) (diff)

comment:9 in reply to:  8 Changed 12 years ago by Ryan J Ollos

Replying to framay:

Your patch cannot be displayed by Trac-Hacks (don#T know why), ...

It might be captured in #6840, which I'm not sure of the cause (real fix is #10193). Anyway, it seems to be fairly common for the preview to fail on t-h.o.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The owner will remain Franz.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.