Should default return type for is_loaned_out_on_ia should be False not None?

[RayBB]

Problem

Evidence / Screenshot

openlibrary/openlibrary/core/lending.py

Lines 564 to 572 in ce273d8

	def is_loaned_out_on_ia(identifier):
	"""Returns True if the item is checked out on Internet Archive."""
	url = "https://archive.org/services/borrow/%s?action=status" % identifier
	try:
	response = requests.get(url).json()
	return response and response.get('checkedout')
	except Exception: # TODO: Narrow exception scope
	logger.exception("is_loaned_out_on_ia(%s)" % identifier)
	return None

This code returns None.
I'm fairly certain we should not return None. Based on how it is used, it is evaluating to False. As in "false, the book isn't loaned out because there was an error". In my opinion, we should probably default to returning True instead.

I've gotten a bit stumped a few times trying to follow the code. However, at least the code path ending here with waitinglists seems like it would get an error from this logic.

openlibrary/openlibrary/templates/admin/waitinglists.html

Lines 34 to 39 in 1f614a1

	$ checkedout = book.get
	$ borrow_status = get_borrow_status(book.ocaid, edition=book)
	$if borrow_status.has_loan or borrow_status.resource_epub == 'checkedout' or borrow_status.resource_pdf == 'checkedout':
	$ klass = "has-loan"
	$else:
	$ klass = ""

Relevant URL(s)

Reproducing the bug

No response

Context

Found during #9117

Notes from this Issue's Lead

Proposal & constraints

Related files

Stakeholders

[mekarpeles]

There's an open question about what to do when a title is not borrowable at all, which is probably what None was trying to communicate. True is actually misleading in this case, because it's not the case that the item is lent out, it's the case that the title is simply not borrowable and we don't want the UI to communicate that the title is checked out.

[RayBB]

@mekarpeles I see what you mean.

In that case I think the solution is one level up is to check if is_loaned_out_on_ia is True.
This way it when we return None is_loaned_out doesn't think it's available.

Should we make that change?

openlibrary/openlibrary/core/lending.py

Lines 543 to 556 in ce273d8

	def is_loaned_out(identifier):
	"""Returns True if the given identifier is loaned out.
	This doesn't worry about waiting lists.
	"""
	# is_loaned_out_on_acs4 is to be deprecated, this logic (in PR)
	# should be handled by is_loaned_out_on_ia which calls
	# BorrowBooks.inc in petabox
	return (
	is_loaned_out_on_ol(identifier)
	or is_loaned_out_on_acs4(identifier)
	or is_loaned_out_on_ia(identifier)
	)

[mekarpeles]

Yes I think that's reasonable

[RayBB]

@mekarpeles I made a PR for this here #9139

Though I'm not totally sure how to test it.