Bug 19871

Summary: Use new exceptions Koha::Exceptions::Object::DuplicateID and FKConstraint
Product: Koha Reporter: Josef Moravec <josef.moravec>
Component: Architecture, internals, and plumbingAssignee: Martin Renvoize (ashimema) <martin.renvoize>
Status: Needs Signoff --- QA Contact: Tomás Cohen Arazi (tcohen) <tomascohen>
Severity: enhancement    
Priority: P5 - low CC: jonathan.druart, martin.renvoize, nick, tomascohen
Version: Main   
Hardware: All   
OS: All   
GIT URL: Change sponsored?: ---
Patch complexity: --- Documentation contact:
Documentation submission: Text to go in the release notes:
Version(s) released in:
Circulation function:
Bug Depends on: 19828    
Bug Blocks: 40445    
Attachments: Bug 19871: Add centralized DBIx::Class exception translation system
Bug 19871: Use centralized exception translation in Koha::Item::add_to_bundle
Bug 19871: Add centralized DBIx::Class exception translation system
Bug 19871: Use centralized exception translation in Koha::Item::add_to_bundle
Bug 19871: Move FK constraint deletion handling to centralized exception translation
Bug 19871: Update virtualshelf templates to use Koha exceptions instead of DBIx::Class::Exception
Bug 19871: Update Koha::Patron::Modification to use centralized exception handling
Bug 19871: Move enum data truncation handling to centralized exception translation
Bug 19871: (QA follow-up) Replace symbolic refs with proper mock object class

Description Josef Moravec 2017-12-22 07:50:46 UTC
There are some reference of DBIx::Class:Exception in our code, they should be updated to use new Koha::Exceptions from bug 19828.
Comment 1 Josef Moravec 2017-12-22 07:51:31 UTC
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
Comment 2 Martin Renvoize (ashimema) 2025-09-21 11:54:21 UTC
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
Comment 3 Martin Renvoize (ashimema) 2025-09-21 11:54:23 UTC
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.
Comment 4 Martin Renvoize (ashimema) 2025-09-21 15:14:38 UTC
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
Comment 5 Martin Renvoize (ashimema) 2025-09-21 15:14:47 UTC
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.
Comment 6 Martin Renvoize (ashimema) 2025-09-21 15:14:56 UTC
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)
Comment 7 Martin Renvoize (ashimema) 2025-09-21 15:15:06 UTC
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.
Comment 8 Martin Renvoize (ashimema) 2025-09-21 15:15:11 UTC
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.
Comment 9 Martin Renvoize (ashimema) 2025-09-21 15:15:13 UTC
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.
Comment 10 Martin Renvoize (ashimema) 2025-09-21 15:15:16 UTC
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.
Comment 11 Tomás Cohen Arazi (tcohen) 2025-09-30 20:58:09 UTC
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!