UIU-2966 - display item title and barcode as text for dcb virtual item. #2597
Conversation
…or dcb virtual item.
Terala-Priyanka
changed the title
src/views/LoanDetails/LoanDetails.js
Outdated
| const instanceId = get(this.loan, ['item', 'instanceId'], ''); | ||
| const holdingsRecordId = get(this.loan, ['item', 'holdingsRecordId'], ''); | ||
| const isVirtualItem = isDCBItem({ instanceId, holdingsRecordId }); | ||
|
|
||
| if (title) { | ||
| const titleTodisplay = (title.length >= 77) ? `${title.substring(0, 77)}...` : title; |
There was a problem hiding this comment.
I know you didn't add this, but while you're here, can you move this magic number to a constant?
There was a problem hiding this comment.
implemented it. I hope this is the way you would suggest it to be.
| @@ -43,6 +43,7 @@ import { | |||
| accountsMatchStatus, | |||
| checkUserActive, | |||
| isDcbUser, | |||
| isDCBItem, | |||
There was a problem hiding this comment.
We need to decide between functions like isDCB... and isDcb.... I know that's not your gaffe, but it's highlighted here by adding the new import.
src/constants.js
Outdated
| export const DCB_INSTANCE_ID = '9d1b77e4-f02e-4b7f-b296-3f2042ddac54'; | ||
| export const DCB_HOLDINGS_RECORD_ID = '10cd3a5a-d36f-4c7a-bc4f-e1ae3cf820c9'; |
There was a problem hiding this comment.
I'm glad to see these values as constants, but to me they feel like the kind of thing that should be provided in an API request like /inventory/dcb-instance-id. I realize that's not on you, but it seems like bad policy to have a bunch of UUIDs copied around inside a bunch of different back-end and front-end modules.
There was a problem hiding this comment.
Unfortunately, implementing such an API was not a part of architecture.
src/views/LoanDetails/LoanDetails.js
Outdated
| const itemBarcode = get(loan, ['item', 'barcode'], ''); | ||
| const isVirtualItem = isDCBItem({ instanceId, holdingsRecordId }); | ||
|
|
||
| if (isVirtualItem) { | ||
| return itemBarcode; |
There was a problem hiding this comment.
The diff makes it hard to assess, but do you need to handle items without barcodes? If so, set the default value of itemBarcode to <NoValue /> instead of empty string up on line 338.
There was a problem hiding this comment.
This is not the code that I have written now. It has been restructured by me.
- My primary question - is it possible for a loan be created on an item without barcode? I think "NO" in that case, in other words, there can be no loan whose item will not have a barcode. Is that ok to have the default value removed from line 338?
- The idea here is no render the item barcode as text (not a link) in case of a dcb item.
|
Kudos, SonarCloud Quality Gate passed! |
Purpose
UIU-2966 - [DCB] Loan details: Disable DCB item links (Borrowing/Pickup library)
Approach
Item title and item barcode are displayed as text (not links) when item instance id and holdings record id are that of DCB instance id and DCB holdings record id.
Screencast
Learning
Pre-Merge Checklist
Before merging this PR, please go through the following list and take appropriate actions.
If there are breaking changes, please STOP and consider the following:
Ideally all of the PRs involved in breaking changes would be merged in the same day to avoid breaking the folio-testing environment. Communication is paramount if that is to be achieved, especially as the number of intermodule and inter-team dependencies increase.
While it's helpful for reviewers to help identify potential problems, ensuring that it's safe to merge is ultimately the responsibility of the PR assignee.