https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33105
Jonathan Druart <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #40 from Jonathan Druart <[email protected]> --- (In reply to Marcel de Rooy from comment #29) > acqui/vendor_issues.pl demonstrates a point that is larger than this script: > $issue = Koha::Acquisition::Bookseller::Issues->find($issue_id); > We have a name clash with the issues table, so it would have been nicer to > specify vendor_issue here in naming variables. > No blocker just noting. well, 'issue' in the code should be checkout when it's about checkouts. If you see $vendor->issue you know given the context that it's about vendor's issues, not checkout. I can rename if you want but I don't think it's needed. (In reply to Marcel de Rooy from comment #30) > vendor_id: > type: > - string > started_on: > type: > - string > ended_on: > type: > - string > > Was expecting a numeric id and dates ? vendor_id must be integer, yes. dates have a type=string. Could have a format=date however, I totally overlooked that. Done in commit "Fix vendor api spec" (In reply to Marcel de Rooy from comment #31) > Plural or singular? Confusion ! Where? vendor_issue.yaml is the definition file for a given issue, hence the singular. acquisitions_vendor_issues.yaml is the path for the /issues endpoint. It's following the convention. > + vendor_issue: > + $ref: ./definitions/vendor_issue.yaml > + "/acquisitions/vendors/{vendor_id}/issues": > + $ref: > "./paths/acquisitions_vendor_issues.yaml#/ > ~1acquisitions~1vendors~1{vendor_id}~1issues" > > Wny vendor and acquisition_vendor btw ? "acquisitions_vendor_issues" to copy "acquisitions_orders.yaml" and keep consistency. Same for the definition file: vendor.yaml vendor_alias.yaml vendor_issue.yaml order.yaml We could/should all prefixed them with 'acquisitions', but that's not for this bug. (In reply to Marcel de Rooy from comment #32) > TODO kohastructure What's missing there? (In reply to Marcel de Rooy from comment #33) > Can't call method "issues" on an undefined value at > /usr/share/koha/acqui/vendor_issues.pl line 103. at > /usr/share/koha/acqui/vendor_issues.pl line 102 > When there is no vendor. Tried to use output_and_exit. But it needs more > attention. Done in "Redirect to 404 if no vendor exists" (In reply to Marcel de Rooy from comment #37) > This is not ready yet. Wonder why this got signed off? Because it was working according to the test plan? :) Thanks for the follow-ups, they all make sense! -- You are receiving this mail because: You are watching all bug changes. _______________________________________________ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
