Provide filled out item data when returning hold list#224
Conversation
| .map { | ||
| case Left(elasticError) => | ||
| warn(s"Unable to look up $hold in Elasticsearch") | ||
| case Left(elasticError: ElasticsearchError) => |
There was a problem hiding this comment.
Previously we just chucked this error, not sure why - i've added it to the warning here.
…n/catalogue-api into fill-item-requests-api
…n/catalogue-api into fill-item-requests-api
| item = new DisplayItem( | ||
| id = hold.canonicalId.map { _.underlying }, | ||
| identifiers = Some(List(DisplayIdentifier(hold.sourceIdentifier))) | ||
| // TODO: This .get should always be Some here |
There was a problem hiding this comment.
I'll refactor this next, but to speed up getting this data into the front-end we could merge this PR, deploy and put the refactor into a new PR.
| warn(s"Unable to look up $hold in Elasticsearch") | ||
| case Left(elasticError: ElasticsearchError) => | ||
| warn( | ||
| s"Unable to look up $hold in Elasticsearch: ${elasticError}" |
There was a problem hiding this comment.
Let's not address it here for the sake of getting this PR out quickly, but having a sourceIdentifier on the hold feels a bit odd to me – a hold is a thing in Sierra and could have a source identifier, it's just not the identifier we're using here.
There was a problem hiding this comment.
What will happen in this branch if we can't find the item?
There was a problem hiding this comment.
but having a sourceIdentifier on the hold feels a bit odd to me
I agree, we use it to look up the item later and fill the Item option on StacksHold which itself feels wrong. I expect that it'll get removed refactoring out the .get.
What will happen in this branch if we can't find the item?
Good point. I think this is actually a case that will blow up the .get on the option added in this PR (theStacksHold is returned with a None for an Item. It's still quite unlikely but we should handle this by doing a multi-item get and returning a list of the items we can get while logging that things are missing with a warn.
| | "type" : "Identifier" | ||
| | } | ||
| | ], | ||
| | "title" : "${titleString}", |
There was a problem hiding this comment.
nit:
| | "title" : "${titleString}", | |
| | "title" : "$titleString", |
3 participants
Put the full item into a holds list for the front-end.
Closes #223