Summary: | GetItemIssue(s) : renewals in both items and issues tables | ||
---|---|---|---|
Product: | Koha | Reporter: | Fridolin Somers <fridolin.somers> |
Component: | Database | Assignee: | Fridolin Somers <fridolin.somers> |
Status: | CLOSED INVALID | QA Contact: | Bugs List <koha-bugs> |
Severity: | normal | ||
Priority: | P5 - low | CC: | colin.campbell, dev_patches, jonathan.druart, paul.poulain, veron |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Attachments: | Proposed patch |
Description
Fridolin Somers
2011-04-27 13:31:53 UTC
SELECT * is generally frowned on in db circles and issues.renewals and items.renewals describe the contents of these fields accurately. If one changes name theres not really a good way of recalling which one has been aliased (or why if dealing only with that table) (In reply to comment #1) > SELECT * is generally frowned on in db circles and issues.renewals and > items.renewals describe the contents of these fields accurately. If one changes > name theres not really a good way of recalling which one has been aliased (or > why if dealing only with that table) I agree, the LEFT JOIN is only to get some fields from items. We should change SELECT * in : SELECT issues.*, items.homebranch, items.holdingbranch, items.notforlaon, ... (end of previous comment) Created attachment 5225 [details] [review] Proposed patch Patch solves the problem by always adding "items.renewals AS totalrenewals" in queries where issues and items are joined. This behavior comes from Member.pm : GetPendingIssues and GetAllIssues Please test. Reads, GBSD#4 taking that. QA comment : * OK, problem is real, but where can I check the differences ? (please, a test plan !) * There's another problem (that existed before this patch SELECT * FROM issues LEFT JOIN biblioitems LEFT JOIN items will return all xml, that is useless & require memory & CPU. I suggest opening another bug for this (Fridolyn, would you please ?) * why didn't you use the solution you've suggested in comment 3 ? Marking failed QA until you've answered those concerns Is the issue still exist? Can we have an example? This issue still exists in that the query in Circulation.pm is still the same, and there are still renewals columns in both items and issues. I think we still have yet to see a test plan which shows how the bug manifests. GetItemIssue has been fixed by commit 5b4752e027ce32492166c7018d44d4a687afae36 Bug 8365: FIX Return issue values instead of item values GetItemIssues is still buggy, but the only place 'renewals' is used is in catalogue/issuehistory.tt: [% IF ( issue.renewals ) %] And it happens only if the itemnumber parameter is passed to the script. However this parameter is never passed from Koha. That means we could remove the subroutine GetItemIssues as well as the related code in the pl and tt. Would you agree to remove it? (In reply to Jonathan Druart from comment #10) > Would you agree to remove it? Yep |