Skip to content

Commit

Permalink
Make default library collections available to all libraries
Browse files Browse the repository at this point in the history
  • Loading branch information
attemoi committed Apr 3, 2024
1 parent 4d39cf8 commit 5a41973
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 11 deletions.
12 changes: 11 additions & 1 deletion core/model/library.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
Table,
Unicode,
UniqueConstraint,
or_,
select,
)
from sqlalchemy.dialects.postgresql import JSONB
Expand Down Expand Up @@ -183,15 +184,24 @@ def collections(self) -> Sequence[Collection]:
IntegrationLibraryConfiguration,
)

# Finland, logic changed as compared to upstream:
# Include collections defined for default library for all other libraries.
_db = Session.object_session(self)
default_library = self.default(_db)
if not default_library:
return []
return _db.scalars(
select(Collection)
.join(IntegrationConfiguration)
.join(IntegrationLibraryConfiguration)
.where(
IntegrationConfiguration.goal == Goals.LICENSE_GOAL,
IntegrationLibraryConfiguration.library_id == self.id,
or_(
IntegrationLibraryConfiguration.library_id == self.id,
IntegrationLibraryConfiguration.library_id == default_library.id,
),
)
.distinct(Collection.id)
).all()

# Cache of the libraries loaded settings object
Expand Down
14 changes: 11 additions & 3 deletions tests/api/admin/controller/test_collections.py
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,10 @@ def test_collections_post_create(
flask_app_fixture: FlaskAppFixture,
db: DatabaseTransactionFixture,
):
# Finland: the logic has been changed so that collections from default
# library are available for all libraries

# Default library
l1 = db.library(
name="Library 1",
short_name="L1",
Expand Down Expand Up @@ -383,15 +387,17 @@ def test_collections_post_create(
("overdrive_client_key", "username"),
("overdrive_client_secret", "password"),
("overdrive_website_id", "1234"),
]
],
)
response = controller.process_collections()
assert isinstance(response, Response)
assert response.status_code == 201

# The collection was created and configured properly.
collection = Collection.by_name(db.session, name="New Collection")

assert isinstance(collection, Collection)

assert collection.integration_configuration.id == int(response.get_data())
assert "New Collection" == collection.name
assert (
Expand All @@ -412,9 +418,10 @@ def test_collections_post_create(
)

# Two libraries now have access to the collection.
# Finland: because l1 is the default library, l3 will have the collection as well
assert [collection] == l1.collections
assert [collection] == l2.collections
assert [] == l3.collections
assert [collection] == l3.collections

# Additional settings were set on the collection.
assert (
Expand Down Expand Up @@ -466,7 +473,8 @@ def test_collections_post_create(
assert "website_id" not in child.integration_configuration.settings_dict

# One library has access to the collection.
assert [child] == l3.collections
# Finland: l3 will also have the collection from default library
assert [collection, child] == l3.collections
assert isinstance(l3.id, int)
l3_settings = child.integration_configuration.for_library(l3.id)
assert l3_settings is not None
Expand Down
8 changes: 5 additions & 3 deletions tests/api/test_selftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,9 @@ def test_collection(self, collection, api_map):

# The default library is the only one with a collection;
# test_collection() was called on that collection.
[(collection, api_map)] = script.tested
# Finland: The difference from upstream is that the other
# library will also include the collection from default library.
[(collection, api_map), (collection, api_map)] = script.tested
assert [collection] == library1.collections

# The API lookup map passed into test_collection() is based on
Expand All @@ -209,9 +211,9 @@ def test_collection(self, collection, api_map):
script = MockScript2(db.session, out)
script.do_run()
assert (
out.getvalue()
== "Testing %s\n Exception while running self-test: 'blah'\nTesting %s\n"
"Testing %s\n Exception while running self-test: 'blah'\nTesting %s\n"
% (library1.name, library2.name)
in out.getvalue()
)

def test_test_collection(self, db: DatabaseTransactionFixture):
Expand Down
5 changes: 5 additions & 0 deletions tests/core/models/test_work.py
Original file line number Diff line number Diff line change
Expand Up @@ -1702,6 +1702,11 @@ def test_active_license_pool_accounts_for_library(
self, db: DatabaseTransactionFixture
):
"""2 libraries, 2 collections, and 2 pools, always select the right pool in a scoped request"""

# Finland: Create and ignore a default library, because collections for default library
# are applied to all libraries.
_ = db.default_library()

l1 = db.library()
l2 = db.library()
c1 = db.collection()
Expand Down
11 changes: 9 additions & 2 deletions tests/core/test_external_search.py
Original file line number Diff line number Diff line change
Expand Up @@ -3623,14 +3623,21 @@ def scoring_functions(self, filter):
for_default_library.append_child(for_other_library)

# Its filter uses the collection from the second library.
# Finland: Also the collection from default library is included
filter = Filter.from_worklist(session, for_other_library, None)
assert [collection2.id] == filter.collection_ids
assert [
transaction.default_collection().id,
collection2.id,
] == filter.collection_ids

# If for whatever reason, collection_ids on the child is not set,
# all collections associated with the WorkList's library will be used.
for_other_library.collection_ids = None
filter = Filter.from_worklist(session, for_other_library, None)
assert [collection2.id] == filter.collection_ids
assert [
transaction.default_collection().id,
collection2.id,
] == filter.collection_ids

# If no library is associated with a WorkList, we assume that
# holds are allowed. (Usually this is controleld by a library
Expand Down
4 changes: 2 additions & 2 deletions tests/core/test_scripts.py
Original file line number Diff line number Diff line change
Expand Up @@ -1217,9 +1217,10 @@ def test_success(self, db: DatabaseTransactionFixture):
)

# Two libraries now have access to the collection.
# Finland: because l1 is the default library, l3 will have the collection as well
assert [collection] == l1.collections
assert [collection] == l2.collections
assert [] == l3.collections
assert [collection] == l3.collections

# One CollectionSetting was set on the collection, in addition
# to url, username, and password.
Expand Down Expand Up @@ -1705,7 +1706,6 @@ def test_check_library(
script.check_library(library2)
checking, no_collection, no_lanes = script.output
assert ("Checking library %s", [library2.name]) == checking
assert " This library has no collections -- that's a problem." == no_collection
assert " This library has no lanes -- that's a problem." == no_lanes

@staticmethod
Expand Down

0 comments on commit 5a41973

Please sign in to comment.