Modify

Opened 3 years ago

Last modified 2 years ago

#9175 new defect

[PATCH] BackLinksMenuMacro was not working on TRAC 0.12.x

Reported by: mizraith Owned by: rjollos
Priority: normal Component: BackLinksMacro
Severity: major Keywords:
Cc: Trac Release: 0.12

Description

Fairly new TRAC 0.12 install. Although I was able to get BackLinksMacro.5 working, I was unable to get BackLinksMenuMacro working (it was only partially working, but not displaying the actual web page links.).

Code inspection between the 2 macros pointed to a few areas where there has been some changes. Gradually bringing code across from BackLinksMacro --> BackLinksMenuMacro resulted in a stable and working copy.

Submitting this ticket and trying to check in the patch has already proven to be more complicated/difficult than anticiapted for such a simple set of changes. There are a few spurious changes in the patch that could be removed, but I was trying to keep the code "lined up" with BackLinksMacro for easier 'diffing'.

Attachments (2)

BackLinksMenu_TRAC0.12_patch.diff (3.1 KB) - added by mizraith 3 years ago.
PATCH file for making BackLinksMenuMacro work for TRAC 0.12
BackLinksMenu.py (3.5 KB) - added by rjollos 2 years ago.
Previously attached to project wiki page.

Download all attachments as: .zip

Change History (9)

Changed 3 years ago by mizraith

PATCH file for making BackLinksMenuMacro work for TRAC 0.12

comment:1 Changed 3 years ago by osimons

I just came across this latest patch by accident, and thought I'd just mention two things I spotted:

  1. The patch turns all code (including code by other copyright holder) into GPL3? The plugin should of course have some stated license, but I think the original author should decide that. And, if the new patch is only be offered as GPL3 then that should be clearly stated as applying it effectively infects the plugin and turns it into GPL3 anyway.
  2. The plugin SQL is badly written as the escaping should be left to the DB backend. There are plenty of examples in the Trac code for how you just use %s placeholders in the SQL string, and pass variables for substitution along with the sql: cursor.execute(sql, args)

comment:2 Changed 3 years ago by osimons

Oh, about the SQL changes: Forgot to mention that there is also a db.like() and db.like_escape() methods for inserting the LIKE syntax used by current DB backend. Again, search the Trac source code for examples.

comment:3 follow-up: Changed 3 years ago by mizraith

Sorry about the GPL3 -- i added that because it was in trapanator's BackLinksMacro.5 I don't think it's necessary either, but that' sup to the original author.

I have no knowledge of SQL, sorry. Once again, just copied/pasted what I found in the working BackLinksMacro. If there is a better way to do this, that's cool....but I'm not the guy to do it, unfortunately. I can lend a hand by running tests on any updated patches, though.

Should I submit an updated patch without the GPL 3 added?

comment:4 in reply to: ↑ 3 Changed 3 years ago by osimons

Replying to mizraith:

Sorry about the GPL3

I have no knowledge of SQL, sorry.

No need for apologies. I just left the notes for the author of the plugin to decide. No need for updated patch, the plugin author can no doubt fix things as needed. I don't use the plugin nor do I have any particular interest in it, so I just voiced my opinion about some of the code.

Changed 2 years ago by rjollos

Previously attached to project wiki page.

comment:5 Changed 2 years ago by rjollos

  • Component changed from BackLinksMenuMacro to BackLinksMacro

The functionality of BackLinksMenuMacro is being integrated into the BackLinksMacro.

comment:6 Changed 2 years ago by rjollos

(In [11969]) Refs #5903, #9175, #9503:

  • Merged codebases for BackLinksMacro and BackLinksMenuMacro.
  • Modified SQL query to utilize Trac's database agnostic API. Possible matches are captured with a LIKE clause, and then filtered with a regex in the Python code.

comment:7 Changed 2 years ago by rjollos

  • Owner changed from trapanator to rjollos

I'm not sure that I'll do much more work with this macro since it is GPL licensed, but I'd appreciate any feedback on the approach I took with using the LIKE clause in the query and then filtering the results in Python. I couldn't see any other way to do it without using a REGEXP in the query, and the Trac DB API doesn't have an abstraction for REGEXP as far as I could see.

Anyway, I think the macros are at least in better shape than they were. I might take the additional step of merging in the TicketBackLinksMacro. The new package could use some more work if anyone wants to take over maintenance. I haven't been able to reach trapanator.

Feedback from testing is also appreciated.

Add Comment

Modify Ticket

Action
as new .
Author


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

 
Note: See TracTickets for help on using tickets.