Modify

Opened 6 years ago

Closed 5 years ago

#11968 closed enhancement (fixed)

Add method TagSystem.get_taggable_realms

Reported by: Ryan J Ollos Owned by: Steffen Hoffmann
Priority: normal Component: TagsPlugin
Severity: normal Keywords: refactoring
Cc: Trac Release:

Description

While reviewing some of the code, I noticed the possibility for some refactoring by adding a method to the TagSystem. I'll attach the proposed changes as a patch file.

Attachments (1)

t11968.patch (6.8 KB) - added by Ryan J Ollos 6 years ago.

Download all attachments as: .zip

Change History (9)

Changed 6 years ago by Ryan J Ollos

Attachment: t11968.patch added

comment:1 Changed 6 years ago by Steffen Hoffmann

Yes, I know all these, even if I've been unaware of the three-fold extension point definition.

The new TagSystem API method is straight-forward too. Will push this this weekend with a few more changes. Thanks for taking care.

comment:2 Changed 6 years ago by Steffen Hoffmann

In 14153:

TagsPlugin: Add generic getter method for taggable realms to API, refs #11968.

The method replaces similar code in several plugin components.
Thanks to Ryan J Ollos for proposing most of these changes.

comment:3 Changed 6 years ago by Steffen Hoffmann

In 14154:

TagsPlugin: More clean-up after API review, refs #11968.

IMHO the only significant change in here is an earlier definition of all_realms
in TagWikiSyntaxProvider, but still it triggered something related to tags
Trac db table setup, so that I've been forced to fix many tests - wired.

comment:4 Changed 6 years ago by Steffen Hoffmann

In 14155:

TagsPlugin: Add more permission checks after further review, refs #11968.

Looking at the optional permission-based filtering for taggable realms made me
watch out for places where we might have been too permissive until now.

comment:5 in reply to:  2 ; Changed 6 years ago by Steffen Hoffmann

Replying to hasienda: One small but significant alteration was using built-in list() for the XMLRPC method. [set(..)] would have returned a list with the set as element.

comment:6 Changed 6 years ago by Steffen Hoffmann

Thanks for the inspiration. IMHO this has yielded considerable improvements.

comment:7 in reply to:  5 Changed 6 years ago by Ryan J Ollos

Replying to hasienda:

One small but significant alteration was using built-in list() for the XMLRPC method. [set(..)] would have returned a list with the set as element.

Good catch. I hadn't stopped to consider the difference in behavior before, between list() and [].

comment:8 Changed 5 years ago by Ryan J Ollos

Resolution: fixed
Status: newclosed

In 14945:

0.8: Tag 0.8 release

Fixes #1304, #1344, #3660, #3891, #9064, #9797, #11661, #11690, #11695, #11888, #11950, #11954, #11968, #12202, #12292, #12434, #12486.

Refs #12415.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Steffen Hoffmann.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


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

 
Note: See TracTickets for help on using tickets.