Opened 6 years ago

Closed 16 months ago

## #9175 closed defect (fixed)

Reported by: Owned by: Red Ryan J Ollos normal BackLinksMacro major 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'.

### comment:1 Changed 6 years ago by Odd Simon Simonsen

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 6 years ago by Odd Simon Simonsen

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:  4 Changed 6 years ago by Red

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 6 years ago by Odd Simon Simonsen

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 5 years ago by Ryan J Ollos

Previously attached to project wiki page.

### comment:6 Changed 5 years ago by Ryan J Ollos

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

• 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 5 years ago by Ryan J Ollos

Owner: changed from Trap to Ryan J Ollos

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.

### comment:8 Changed 16 months ago by Ryan J Ollos

Resolution: → fixed new → closed

In 15134:

7.0dev: Pass env rather than db object

This will make it easier to eventually adapt the code
to the Trac 1.0 DB API.

Fixes #9175.

### Modify Ticket

Change Properties