[Koha-bugs] [Bug 33105] Add vendor issues

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33105

Jonathan Druart <jonathan.druart+koha at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|Failed QA                   |Signed Off

--- Comment #40 from Jonathan Druart <jonathan.druart+koha at 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.