There are some reference of DBIx::Class:Exception in our code, they should be updated to use new Koha::Exceptions from bug 19828.
Affected files: Koha/Patron/Modification.pm Koha/REST/V1/Acquisitions/Vendors.pm Koha/REST/V1/Cities.pm koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shelves.tt
Created attachment 186634 [details] [review] Bug 19871: Add centralized DBIx::Class exception translation system This introduces a reusable exception translation utility that eliminates code duplication and provides consistent handling of DBIx::Class exceptions throughout the codebase. New Components: - Koha::Schema::Util::ExceptionTranslator: Core utility class for translating DBIx::Class exceptions to Koha-specific exceptions - Koha::Schema: Enhanced with safe_do() and translate_exception() methods for convenient access to centralized exception handling - Comprehensive unit tests covering all translation scenarios Benefits: - Eliminates ~90 lines of duplicated exception handling code - Centralizes maintenance of exception translation logic - Provides consistent error messages and exception types - Makes it easier to extend support for additional database engines - Follows the pattern suggested in existing FIXME comments Exception Types Handled: - Foreign key constraint violations → Koha::Exceptions::Object::FKConstraint - Duplicate key violations → Koha::Exceptions::Object::DuplicateID - Invalid data type values → Koha::Exceptions::Object::BadValue - Enum data truncation → Koha::Exceptions::Object::BadValue
Created attachment 186635 [details] [review] Bug 19871: Use centralized exception translation in Koha::Item::add_to_bundle Replace duplicated DBIx::Class exception handling code with the new centralized exception translation system introduced in the previous commit. This change: - Eliminates ~40 lines of duplicated exception handling code - Uses the new Koha::Schema::translate_exception() method - Maintains identical exception behavior and error messages - Addresses the FIXME comment requesting this refactoring The add_to_bundle method now uses consistent exception translation with the rest of the codebase, reducing maintenance burden and ensuring consistent error handling patterns.
Created attachment 186644 [details] [review] Bug 19871: Add centralized DBIx::Class exception translation system This introduces a reusable exception translation utility that eliminates code duplication and provides consistent handling of DBIx::Class exceptions throughout the codebase. New Components: - Koha::Schema::Util::ExceptionTranslator: Core utility class for translating DBIx::Class exceptions to Koha-specific exceptions - Koha::Schema: Enhanced with safe_do() and translate_exception() methods for convenient access to centralized exception handling - Comprehensive unit tests covering all translation scenarios Benefits: - Eliminates ~90 lines of duplicated exception handling code - Centralizes maintenance of exception translation logic - Provides consistent error messages and exception types - Makes it easier to extend support for additional database engines - Follows the pattern suggested in existing FIXME comments Exception Types Handled: - Foreign key constraint violations → Koha::Exceptions::Object::FKConstraint - Duplicate key violations → Koha::Exceptions::Object::DuplicateID - Invalid data type values → Koha::Exceptions::Object::BadValue - Enum data truncation → Koha::Exceptions::Object::BadValue
Created attachment 186645 [details] [review] Bug 19871: Use centralized exception translation in Koha::Item::add_to_bundle Replace duplicated DBIx::Class exception handling code with the new centralized exception translation system introduced in the previous commit. This change: - Eliminates ~40 lines of duplicated exception handling code - Uses the new Koha::Schema::translate_exception() method - Maintains identical exception behavior and error messages - Addresses the FIXME comment requesting this refactoring The add_to_bundle method now uses consistent exception translation with the rest of the codebase, reducing maintenance burden and ensuring consistent error handling patterns.
Created attachment 186646 [details] [review] Bug 19871: Move FK constraint deletion handling to centralized exception translation Move the FK constraint deletion exception handling from Koha::Object::delete to the centralized ExceptionTranslator utility. This consolidates all DBIx::Class exception translation logic in one place and simplifies the Object delete method. Changes: - Add FK constraint deletion pattern matching to ExceptionTranslator - Remove manual exception handling from Koha::Object::delete - Add test coverage for FK constraint deletion translation - Simplify Object delete method to use centralized translation The centralized system now handles both FK constraint scenarios: - Child row constraint failures (add/update operations) - Parent row constraint failures (delete operations)
Created attachment 186647 [details] [review] Bug 19871: Update virtualshelf templates to use Koha exceptions instead of DBIx::Class::Exception Replace DBIx::Class::Exception handling in shelf templates with specific Koha exception types that are now provided by the centralized exception translation system. Template changes: - Replace 'DBIx::Class::Exception' case with specific Koha exception handlers - Add user-friendly error messages for FKConstraint, DuplicateID, and BadValue - Improve error presentation with descriptive text instead of raw database errors This provides better user experience by showing meaningful error messages instead of technical database exception details.
Created attachment 186648 [details] [review] Bug 19871: Update Koha::Patron::Modification to use centralized exception handling Replace manual DBIx::Class::Exception handling in the approve() method with the centralized exception translation system while preserving domain-specific exception behavior. Changes: - Use $schema->safe_do() for database operations with automatic exception translation - Wrap safe_do in try/catch to convert any exception to domain-specific Patron::Modification exception - Simplify exception handling logic while maintaining the same API contract - Remove manual DBIx::Class::Exception type checking This maintains backward compatibility by always throwing Koha::Exceptions::Patron::Modification exceptions as before, but leverages the centralized system for consistent database exception handling.
Created attachment 186649 [details] [review] Bug 19871: Move enum data truncation handling to centralized exception translation Complete the migration of enum data truncation exception handling from Koha::Object to the centralized ExceptionTranslator utility. This eliminates the last remaining manual DBIx::Class exception handling in Object code. Changes: - Add optional object parameter to ExceptionTranslator->translate_exception() - Update Schema wrapper methods to accept and pass object parameter - Enhance enum truncation handling to extract actual property values when object is available - Remove manual enum exception handling from Koha::Object::store() - Add comprehensive test coverage for enum handling with and without object context - Maintain contextual warning behavior for store() operations The centralized system now handles ALL DBIx::Class exception scenarios: - FK constraint failures (add/update and delete operations) - Duplicate key violations - Invalid data type values - Enum data truncation with actual property values This completes the centralization effort while providing better error reporting than the previous manual approach.
Created attachment 186650 [details] [review] Bug 19871: (QA follow-up) Replace symbolic refs with proper mock object class Replace 'no strict refs' usage with a proper TestObject package to avoid Perl::Critic violations while maintaining the same test functionality. The mock object now uses standard OOP patterns instead of symbolic references for method creation.
I really love this. Great job! My only doubt is the explicit call in the catch in Koha::Item: ``` $schema->translate_exception($_); ``` I'd expect a `->rethrow` or similar instead. This is great stuff!