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!
Created attachment 189305 [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 189306 [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 189307 [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 189308 [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 189309 [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 189310 [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 189311 [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.
Created attachment 189312 [details] [review] Bug 19871: Make exception handling more explicit in add_to_bundle Replace the implicit exception translation call with an explicit safe_do pattern that clearly shows exceptions are rethrown. Changes: - Use $schema->safe_do() wrapper for automatic exception translation - Add explicit $_->rethrow() in catch block for clarity - Follow the same pattern used in Koha::Patron::Modification::approve() - Make control flow more obvious to future maintainers This change maintains identical behavior while improving code clarity by making it explicit that the catch block never returns normally.
Automatic patch test results: â ï¸ The patch applies with fuzzy option (ignoring whitespace). This may indicate minor conflicts that need to be resolved. Details: Patch applies with fuzzy option (ignoring whitespace) This is an automated message from the patch testing system.