diff --git a/core/configuration/library.py b/core/configuration/library.py index a72d14fd6..d36f44e7c 100644 --- a/core/configuration/library.py +++ b/core/configuration/library.py @@ -17,12 +17,7 @@ from pydantic.fields import ModelField from sqlalchemy.orm import Session -from api.admin.problem_details import ( - INCOMPLETE_CONFIGURATION, - INVALID_CONFIGURATION_OPTION, - UNKNOWN_LANGUAGE, -) -from core.config import Configuration +from api.admin.problem_details import INVALID_CONFIGURATION_OPTION, UNKNOWN_LANGUAGE from core.entrypoint import EntryPoint from core.facets import FacetConstants from core.integration.settings import ( @@ -58,6 +53,8 @@ class LibraryConfFormItem(ConfigurationFormItem): read_only: bool | None = None skip: bool | None = None paired: str | None = None + default_library_only: bool | None = False + non_default_library_only: bool | None = False def to_dict( self, db: Session, key: str, required: bool = False, default: Any = None @@ -73,6 +70,12 @@ def to_dict( if self.paired is not None: item["paired"] = self.paired + # Finland, hiding settings for default or other libraries + if self.default_library_only is not None: + item["defaultLibraryOnly"] = self.default_library_only + if self.non_default_library_only is not None: + item["nonDefaultLibraryOnly"] = self.non_default_library_only + if ( "default" in item and isinstance(item["default"], list) @@ -136,10 +139,10 @@ class LibrarySettings(BaseSettings): None, form=LibraryConfFormItem( label="Consortium", - description="If selected, the municipalities are kept automatically " - "in sync with Kirkanta. Has no effect if set for the default library.", + description="If selected, the municipalities are kept automatically in sync with Kirkanta.", category="Kirkanta Synchronization", type=ConfigurationFormItemType.SELECT, + non_default_library_only=True, # The keys here should match the Kirkanta consortium slug # See https://api.kirjastot.fi/v4/consortium?limit=9999 options={ @@ -193,6 +196,7 @@ class LibrarySettings(BaseSettings): form=LibraryConfFormItem( label="The municipalities belonging to this consortium", type=ConfigurationFormItemType.LIST, + non_default_library_only=True, description="Each value should be a valid " '' "municipality code. This list is populated automatically if the consortium is selected.", @@ -212,6 +216,7 @@ class LibrarySettings(BaseSettings): }, category="Loans, Holds, & Fines", level=Level.SYS_ADMIN_ONLY, + default_library_only=True, ), ) enabled_entry_points: list[str] = FormField( @@ -232,6 +237,7 @@ class LibrarySettings(BaseSettings): format="narrow", read_only=True, level=Level.SYS_ADMIN_ONLY, + default_library_only=True, ), ) featured_lane_size: PositiveInt = FormField( @@ -240,15 +246,17 @@ class LibrarySettings(BaseSettings): label="Maximum number of books in the 'featured' lanes", category="Lanes & Filters", level=Level.ALL_ACCESS, + default_library_only=True, ), ) minimum_featured_quality: PercentFloat = FormField( - Configuration.DEFAULT_MINIMUM_FEATURED_QUALITY, + 0.65, form=LibraryConfFormItem( label="Minimum quality for books that show up in 'featured' lanes", description="Between 0 and 1.", category="Lanes & Filters", level=Level.ALL_ACCESS, + default_library_only=True, ), ) facets_enabled_order: list[str] = FormField( @@ -263,6 +271,7 @@ class LibrarySettings(BaseSettings): category="Lanes & Filters", paired="facets_default_order", level=Level.SYS_ADMIN_OR_MANAGER, + default_library_only=True, ), ) facets_default_order: str = FormField( @@ -276,6 +285,7 @@ class LibrarySettings(BaseSettings): }, category="Lanes & Filters", skip=True, + default_library_only=True, ), ) facets_enabled_available: list[str] = FormField( @@ -292,6 +302,7 @@ class LibrarySettings(BaseSettings): category="Lanes & Filters", paired="facets_default_available", level=Level.SYS_ADMIN_OR_MANAGER, + default_library_only=True, ), ) facets_default_available: str = FormField( @@ -305,6 +316,7 @@ class LibrarySettings(BaseSettings): }, category="Lanes & Filters", skip=True, + default_library_only=True, ), ) facets_enabled_collection: list[str] = FormField( @@ -321,6 +333,7 @@ class LibrarySettings(BaseSettings): category="Lanes & Filters", paired="facets_default_collection", level=Level.SYS_ADMIN_OR_MANAGER, + default_library_only=True, ), ) facets_default_collection: str = FormField( @@ -334,6 +347,7 @@ class LibrarySettings(BaseSettings): }, category="Lanes & Filters", skip=True, + default_library_only=True, ), ) # Finland @@ -349,6 +363,7 @@ class LibrarySettings(BaseSettings): category="Lanes & Filters", paired="facets_default_language", level=Level.SYS_ADMIN_OR_MANAGER, + default_library_only=True, ), ) facets_default_language: str = FormField( @@ -362,6 +377,7 @@ class LibrarySettings(BaseSettings): }, category="Lanes & Filters", skip=True, + default_library_only=True, ), ) library_description: str | None = FormField( @@ -432,6 +448,7 @@ class LibrarySettings(BaseSettings): "
The default address will work, but for greater security, set up your own address that " "trashes all incoming email.", level=Level.SYS_ADMIN_OR_MANAGER, + default_library_only=True, ), ) color_scheme: str = FormField( @@ -462,6 +479,7 @@ class LibrarySettings(BaseSettings): }, category="Client Interface Customization", level=Level.SYS_ADMIN_OR_MANAGER, + default_library_only=True, ), ) web_primary_color: str = FormField( @@ -473,6 +491,7 @@ class LibrarySettings(BaseSettings): category="Client Interface Customization", type=ConfigurationFormItemType.COLOR, level=Level.SYS_ADMIN_OR_MANAGER, + default_library_only=True, ), alias="web-primary-color", ) @@ -485,6 +504,7 @@ class LibrarySettings(BaseSettings): category="Client Interface Customization", type=ConfigurationFormItemType.COLOR, level=Level.SYS_ADMIN_OR_MANAGER, + default_library_only=True, ), alias="web-secondary-color", ) @@ -495,6 +515,7 @@ class LibrarySettings(BaseSettings): description="Give web applications a CSS file to customize the catalog display.", category="Client Interface Customization", level=Level.SYS_ADMIN_ONLY, + default_library_only=True, ), alias="web-css-file", ) @@ -507,6 +528,7 @@ class LibrarySettings(BaseSettings): category="Client Interface Customization", type=ConfigurationFormItemType.LIST, level=Level.SYS_ADMIN_OR_MANAGER, + default_library_only=True, ), alias="web-header-links", ) @@ -518,6 +540,7 @@ class LibrarySettings(BaseSettings): category="Client Interface Customization", type=ConfigurationFormItemType.LIST, level=Level.SYS_ADMIN_OR_MANAGER, + default_library_only=True, ), alias="web-header-labels", ) @@ -636,6 +659,7 @@ class LibrarySettings(BaseSettings): "ISO-639-2 language code.", category="Languages", level=Level.ALL_ACCESS, + default_library_only=True, ), alias="large_collections", ) @@ -650,6 +674,7 @@ class LibrarySettings(BaseSettings): "ISO-639-2 language code.", category="Languages", level=Level.ALL_ACCESS, + default_library_only=True, ), alias="small_collections", ) @@ -664,25 +689,11 @@ class LibrarySettings(BaseSettings): "ISO-639-2 language code.", category="Languages", level=Level.ALL_ACCESS, + default_library_only=True, ), alias="tiny_collections", ) - @root_validator - def validate_require_help_email_or_website( - cls, values: dict[str, Any] - ) -> dict[str, Any]: - if not values.get("help_email") and not values.get("help_web"): - help_email_label = cls.get_form_field_label("help_email") - help_website_label = cls.get_form_field_label("help_web") - raise SettingsValidationError( - problem_detail=INCOMPLETE_CONFIGURATION.detailed( - f"You must provide either '{help_email_label}' or '{help_website_label}'." - ) - ) - - return values - @root_validator def validate_header_links(cls, values: dict[str, Any]) -> dict[str, Any]: """Verify that header links and labels are the same length.""" @@ -776,3 +787,11 @@ def validate_language_codes( languages.append(validated_language) return languages return value + + def merge_with(self, other_instance: LibrarySettings) -> LibrarySettings: + """Where there is no value for an attribute, use value from the other instance""" + merged_attrs = self.__dict__.copy() + for key, value in other_instance.__dict__.items(): + if value is not None and getattr(self, key, None) is None: + merged_attrs[key] = value + return LibrarySettings(**merged_attrs) diff --git a/core/model/library.py b/core/model/library.py index 54833aec1..209c6ee4d 100644 --- a/core/model/library.py +++ b/core/model/library.py @@ -334,7 +334,18 @@ def settings(self) -> LibrarySettings: ) settings = integration_settings_load(LibrarySettings, self) self._settings = settings - return settings + + # Finland, use default library settings as defaults + + # This is the default, don't need to merge anything + if self.is_default: + return settings + + default_library: Library | None = self.default(Session.object_session(self)) + if not default_library: + return settings + + return settings.merge_with(default_library.settings) def update_settings(self, new_settings: LibrarySettings) -> None: """Update the settings for this integration""" diff --git a/tests/api/admin/controller/test_library.py b/tests/api/admin/controller/test_library.py index 0cf36be50..2d7eac150 100644 --- a/tests/api/admin/controller/test_library.py +++ b/tests/api/admin/controller/test_library.py @@ -306,25 +306,6 @@ def test_libraries_post_errors( controller.process_post() assert excinfo.value.problem_detail.uri == INCOMPLETE_CONFIGURATION.uri - # Either patron support email or website MUST be present - with flask_app_fixture.test_request_context_system_admin("/", method="POST"): - flask.request.form = ImmutableMultiDict( - [ - ("name", "Email or Website Library"), - ("short_name", "Email or Website"), - ("website", "http://example.org"), - ("default_notification_email_address", "email@example.org"), - ] - ) - with pytest.raises(ProblemError) as excinfo: - controller.process_post() - assert excinfo.value.problem_detail.uri == INCOMPLETE_CONFIGURATION.uri - assert excinfo.value.problem_detail.detail is not None - assert ( - "'Patron support email address' or 'Patron support website'" - in excinfo.value.problem_detail.detail - ) - # Test a web primary and secondary color that doesn't contrast # well on white. Here primary will, secondary should not. with flask_app_fixture.test_request_context_system_admin("/", method="POST"): diff --git a/tests/api/test_authenticator.py b/tests/api/test_authenticator.py index ecf4b35af..f1553f066 100644 --- a/tests/api/test_authenticator.py +++ b/tests/api/test_authenticator.py @@ -1087,8 +1087,8 @@ def annotate_authentication_document(library, doc, url_for): library_settings.color_scheme = "plaid" # Set the colors a web client should use. - library_settings.web_primary_color = "#012345" - library_settings.web_secondary_color = "#abcdef" + library_settings.web_primary_color = "#000001" + library_settings.web_secondary_color = "#000002" # Configure the various ways a patron can get help. library_settings.help_email = "help@library.org" # type: ignore[assignment] @@ -1165,8 +1165,8 @@ def annotate_authentication_document(library, doc, url_for): # The mobile color scheme and web colors are correctly reported. assert "plaid" == doc["color_scheme"] - assert "#012345" == doc["web_color_scheme"]["primary"] - assert "#abcdef" == doc["web_color_scheme"]["secondary"] + assert "#000001" == doc["web_color_scheme"]["primary"] + assert "#000002" == doc["web_color_scheme"]["secondary"] # We also need to test that the links got pulled in # from the configuration. diff --git a/tests/api/test_circulationapi.py b/tests/api/test_circulationapi.py index f6fc8e8bc..63e432f72 100644 --- a/tests/api/test_circulationapi.py +++ b/tests/api/test_circulationapi.py @@ -1560,12 +1560,12 @@ def test_default_notification_email_address( # Test the ability to get the default notification email address # for a patron or a library. settings = library_fixture.mock_settings() - settings.default_notification_email_address = "help@library" # type: ignore[assignment] + settings.default_notification_email_address = "help@example.com" # type: ignore[assignment] library = library_fixture.library(settings=settings) patron = db.patron(library=library) m = BaseCirculationAPI.default_notification_email_address - assert "help@library" == m(library, "") - assert "help@library" == m(patron, "") + assert "help@example.com" == m(library, "") + assert "help@example.com" == m(patron, "") other_library = library_fixture.library() assert "noreply@thepalaceproject.org" == m(other_library, "") diff --git a/tests/core/models/test_library.py b/tests/core/models/test_library.py index 0f66bbdd0..ebd6ef1ee 100644 --- a/tests/core/models/test_library.py +++ b/tests/core/models/test_library.py @@ -234,7 +234,3 @@ def test_settings(self, db: DatabaseTransactionFixture): library.settings_dict = [] with pytest.raises(ValueError): library.settings - - # Test with a properly formatted settings dict. - library2 = db.library() - assert library2.settings.website == "http://library.com" diff --git a/tests/fixtures/library.py b/tests/fixtures/library.py index e4056b49e..e8aca1ca3 100644 --- a/tests/fixtures/library.py +++ b/tests/fixtures/library.py @@ -54,7 +54,10 @@ def settings(self, library: Library) -> MockLibrarySettings: return settings def mock_settings(self) -> MockLibrarySettings: - return MockLibrarySettings.construct() + settings = MockLibrarySettings.construct() + settings.color_scheme = "blue" + settings.website = "https://example.com" # type: ignore[assignment] + return settings def set_mock_on_library( self, library: Library, settings: MockLibrarySettings