Back-Office UI: 67 loan details #97

topless · 2019-01-10T13:57:43Z

No description provided.

zzacharo · 2019-01-13T18:04:47Z

@topless can you squash the commits before reviewing because we are gonna forget after?

zzacharo · 2019-01-14T08:43:01Z

invenio_app_ils/ui/src/invenio_app_ils/common/api/item.js

@@ -6,7 +6,13 @@ const get = itemPid => {
  return http.get(`${itemURL}${itemPid}`);
 };

+const fetchItemsByDocPid = documentPid => {
+  const qs = `(document_pid:${documentPid} AND NOT(status:MISSING))`;


Shouldn't this status:MISSING be retrieved from the invenioConfig instead of being hardcoded?

I submitted a first attempt.

invenio_app_ils/ui/src/invenio_app_ils/common/api/loan.js

zzacharo · 2019-01-14T08:48:32Z

invenio_app_ils/ui/src/invenio_app_ils/common/components/ItemMetadata/ItemMetadata.js

+
+export default class ItemMetadata extends Component {
+  render() {
+    switch (this.props.view) {


I am not sure this is the correct way to go for this one. Why you don't extract the common part in a reusable component and just use it in the two different cases. Eitherway you could just use the two components ItemView, LoanView directly. I think that is component is redundant no?

I agree, this one requires further discussion, I implemented it this way in order to demonstrate one for the possible solutions.

invenio_app_ils/ui/src/invenio_app_ils/pages/backoffice/ItemsSearch/ItemsSearch.js

...i/src/invenio_app_ils/pages/backoffice/LoanDetails/components/DocumentItems/DocumentItems.js

zzacharo · 2019-01-14T08:59:39Z

...i/src/invenio_app_ils/pages/backoffice/LoanDetails/components/DocumentItems/state/actions.js

+      payload: newSortBy,
+    });
+
+    await dispatch(fetchDocumentItems(documentPid));


why this? You are not waiting for something after...

There is no sorting any longer so its removed. But this one was copied from ItemPendingLoans, where we have to actually address the issue.

I think we should remove also there then!

zzacharo · 2019-01-14T09:00:01Z

...i/src/invenio_app_ils/pages/backoffice/LoanDetails/components/DocumentItems/state/actions.js

+  };
+};
+
+export const documentItemsChangeSortOrder = documentPid => {


same here as in the previous action name.

.../ui/src/invenio_app_ils/pages/backoffice/LoanDetails/components/LoanMetadata/LoanMetadata.js

zzacharo · 2019-01-14T09:04:40Z

invenio_app_ils/ui/src/invenio_app_ils/pages/backoffice/LoanDetails/state/actions.js

@@ -32,6 +32,28 @@ export const fetchLoanDetails = loanPid => {
  };
 };

+export const assignLoanItem = (loanId, itemId) => {


assignItemToLoan?

Yeap, and also flipped the arguments to match the naming

setup.py

* Item metadata is a common component, accepts a view param and renders the relevant view for available item data. * Loan actions display a message when there is no action available.

* Extended existing item search in the loan view

* Adjusted sample data to match real life logic for pending loans * Conditional rendering of item list in loan details, when a loan has an item assigned or when we want to change it.

* Loans now get a document id from available documents * Integrated search component and configured it to search items by documents id. * Adjusted layout to accommodate changes

* removed sort from items * filtered items so only LOANABLE show up * removed cursor when hovering over table row * #resolves 67

ntarocco · 2019-01-14T09:27:13Z

invenio_app_ils/ui/src/invenio_app_ils/common/api/loan.js

@@ -44,6 +44,20 @@ const buildPendingQuery = (documentPid, itemPid) => {
  return `${qsDocItem} AND state:PENDING`;


And probably even here we should think about avoid harcoding, my mistake

ntarocco · 2019-01-14T09:28:36Z

invenio_app_ils/ui/src/invenio_app_ils/common/api/loan.js

@@ -44,6 +44,20 @@ const buildPendingQuery = (documentPid, itemPid) => {
  return `${qsDocItem} AND state:PENDING`;
 };

+const assignLoanItem = (loanPid, itemPid) => {
+  const newItemRef = {
+    $ref: `https://127.0.0.1:5000/api/resolver/circulation/items/${itemPid}`,


I think we should re-think how we do this... we cannot hardcode this ref here.
Probably, instead of doing a patch, we need to create a new API endpoint, let's say /loan/changeItem, passing the itemPid, and server side it performs a patch.
WDYT?

zzacharo · 2019-01-14T13:46:53Z

...p_ils/ui/src/invenio_app_ils/pages/backoffice/LoanDetails/components/AvailableItems/index.js

+});
+
+export const AvailableItems = compose(
+  withRouter,


Still this one where do we need it?

zzacharo · 2019-01-14T13:49:21Z

setup.py

@@ -59,6 +59,7 @@
    # because of https://github.com/requests/requests-oauthlib/commit/1b9fe1a630eb1c91bf12fd70aa2e10ca74ffc0b6
    "oauthlib>=2.1.0,<3.0.0",
    # https://github.com/inveniosoftware/invenio-indexer/commit/9749c2fe4e2cbaabc167ad7fb12ade945a2d580c
+    "oauthlib==2.1.0",


no need of this anymore :)

topless · 2019-01-16T10:45:04Z

I have opened a new clean one #109

topless added the review label Jan 10, 2019

topless force-pushed the 67-loan-details branch from b7a80e0 to 85c3aae Compare January 10, 2019 17:05

zzacharo reviewed Jan 14, 2019

View reviewed changes

topless added 10 commits January 14, 2019 10:21

Loan details with layout same as Item

7962cdc

Back-Office UI: Factory for item detail views

4564592

* Item metadata is a common component, accepts a view param and renders the relevant view for available item data. * Loan actions display a message when there is no action available.

Back-Office UI: Item search view for loan details

a08b668

* Extended existing item search in the loan view

Back-office UI: Conditional item search render

5ae8e7c

* Adjusted sample data to match real life logic for pending loans * Conditional rendering of item list in loan details, when a loan has an item assigned or when we want to change it.

Back-Office UI: Replicate ItemSearch for loan details

ea488f7

Back-Office UI: Document Item search results

0ad4108

* Loans now get a document id from available documents * Integrated search component and configured it to search items by documents id. * Adjusted layout to accommodate changes

backoffice: Loan patch request to update item_id

c4a63c5

backoffice: Patch request to update Loan Item

ddbb366

dependencies: pinned oauthlib version

c1dae1e

backoffice: available items for loan

622b33c

* removed sort from items * filtered items so only LOANABLE show up * removed cursor when hovering over table row * #resolves 67

ntarocco reviewed Jan 14, 2019

View reviewed changes

topless force-pushed the 67-loan-details branch from 18aa0f8 to 622b33c Compare January 14, 2019 09:30

ntarocco reviewed Jan 14, 2019

View reviewed changes

backoffice: refactor for review

84609a8

topless force-pushed the 67-loan-details branch from 7487020 to 84609a8 Compare January 14, 2019 11:37

topless self-assigned this Jan 14, 2019

topless added 2 commits January 14, 2019 13:24

backoffice: removed conditional render for ItemSearch

e95ad46

backoffice: status for available items from config

bacef3a

zzacharo reviewed Jan 14, 2019

View reviewed changes

topless closed this Jan 16, 2019

topless removed the review label Jan 16, 2019

topless mentioned this pull request Jan 16, 2019

backoffice: Loan view details #109

Merged

topless deleted the 67-loan-details branch February 27, 2019 14:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Back-Office UI: 67 loan details #97

Back-Office UI: 67 loan details #97

topless commented Jan 10, 2019

zzacharo commented Jan 13, 2019

zzacharo Jan 14, 2019

topless Jan 14, 2019

zzacharo Jan 14, 2019

topless Jan 14, 2019

zzacharo Jan 14, 2019

topless Jan 14, 2019 •

edited

zzacharo Jan 14, 2019

zzacharo Jan 14, 2019

topless Jan 14, 2019

zzacharo Jan 14, 2019

topless Jan 14, 2019

ntarocco Jan 14, 2019

ntarocco Jan 14, 2019

zzacharo Jan 14, 2019

zzacharo Jan 14, 2019

topless commented Jan 16, 2019

		@@ -44,6 +44,20 @@ const buildPendingQuery = (documentPid, itemPid) => {
		return `${qsDocItem} AND state:PENDING`;

Back-Office UI: 67 loan details #97

Back-Office UI: 67 loan details #97

Conversation

topless commented Jan 10, 2019

zzacharo commented Jan 13, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

topless Jan 14, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

topless commented Jan 16, 2019

topless Jan 14, 2019 •

edited