-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
acquisition: create invoice resource #729
Conversation
rero_ils/modules/acq_invoices/jsonschemas/acq_invoices/acq_invoice-v0.0.1.json
Show resolved
Hide resolved
36c7767
to
e9918e2
Compare
ed0a605
to
4051f67
Compare
rero_ils/modules/acq_invoices/jsonschemas/acq_invoices/acq_invoice-v0.0.1.json
Outdated
Show resolved
Hide resolved
"title": "Acquisition order line", | ||
"type": "object", | ||
"properties": { | ||
"$ref": { | ||
"title": "Acquisition order line URI", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about simplifiying with Order line
instead of Acquisition order line
?
"title": "Acquisition account", | ||
"type": "object", | ||
"properties": { | ||
"$ref": { | ||
"title": "Acquisition account URI", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just Account
? (same thing as previously and for all Acquisition some words
to some words
everywhere).
rero_ils/modules/acq_invoices/jsonschemas/acq_invoices/acq_invoice-v0.0.1.json
Show resolved
Hide resolved
rero_ils/modules/acq_invoices/jsonschemas/acq_invoices/acq_invoice-v0.0.1.json
Show resolved
Hide resolved
"ger", | ||
"spa" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there no way to list all possible languages? What happens the day where a new vendor is an arabic one? Chinese one? Etc.
@@ -230,8 +234,11 @@ | |||
"type": "string", | |||
"enum": [ | |||
"CHF", | |||
"CAD", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as languages: how could we display all known currencies?
rero_ils/modules/acq_invoices/jsonschemas/acq_invoices/acq_invoice-v0.0.1.json
Outdated
Show resolved
Hide resolved
rero_ils/modules/acq_invoices/jsonschemas/acq_invoices/acq_invoice-v0.0.1.json
Outdated
Show resolved
Hide resolved
61ae3f3
to
bb1febe
Compare
bb1febe
to
5f0d8de
Compare
@@ -55,17 +57,24 @@ class Vendor(IlsRecord): | |||
|
|||
def get_number_of_acq_orders(self): | |||
"""Get number of acq orders.""" | |||
from ..acq_orders.api import AcqOrdersSearch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method can be a class method.
return AcqOrdersSearch().filter( | ||
'term', vendor__pid=self.pid).source().count() | ||
|
||
def get_number_of_acq_invoices(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can be a class method
rero_ils/modules/acq_invoices/jsonschemas/acq_invoices/acq_invoice-v0.0.1.json
Outdated
Show resolved
Hide resolved
5f0d8de
to
92bc265
Compare
101bf53
to
f1f371c
Compare
* Adds acquisition invoice resource. * Adds a function to check if invoice exists before deleting a vendor. * Adds more currencies to vendors. * Adds fixtures for unit tests. * Updates tests for vendors. Co-Authored-by: Lauren-D <[email protected]>
f1f371c
to
64d6b58
Compare
Co-Authored-by: Lauren-D [email protected]
Why are you opening this PR?
https://tree.taiga.io/project/rero21-reroils/us/1237?milestone=252915
How to test?
There is no way to test backend at this time
Code review check list