https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33105 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #40 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (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.