-
Notifications
You must be signed in to change notification settings - Fork 18
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
global: expose get loan for item logic and refactoring #75
global: expose get loan for item logic and refactoring #75
Conversation
ntarocco
commented
Oct 20, 2018
- extracts the logic to retrieve the loan status for an item in api
- changes schema and record creation to have mandatory loan_pid field
- fixes bug when searching loans by pid
- fixes db commit and indexing after loan change
- moves PIDs constants in a common module
- moves blueprints registration to entrypoints
- improves tests and fixtures
9b4019c
to
09adad6
Compare
* extracts the logic to retrieve the loan status for an item in api * changes schema and record creation to have mandatory loan_pid field * fixes bug when searching loans by pid * fixes db commit and indexing after loan change * moves PIDs constants in a common module * moves blueprints registration to entrypoints * improves tests and fixtures * closes inveniosoftware#59
09adad6
to
0efa574
Compare
@@ -12,12 +12,11 @@ | |||
"$schema": { | |||
"type": "keyword" | |||
}, | |||
"loanid": { | |||
"loan_pid": { |
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.
👍 thanks for that, I was lost in different naming for ids
@@ -12,12 +12,11 @@ | |||
"$schema": { | |||
"type": "keyword" | |||
}, | |||
"loanid": { | |||
"loan_pid": { |
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.
👍 thanks for that, I was lost in different naming for ids
@@ -18,8 +18,7 @@ | |||
"index": "not_analyzed" | |||
}, | |||
"transaction_date": { | |||
"type": "date", | |||
"format": "yyyy-MM-dd" |
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.
Do we have a global date format now?
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.
So, dates are in this format for now:
2018-02-01T09:30:00+02:00
ES will automatically pick the right date format, but it could be that we will change it if it not good enough.
Reviewed also with Zac |