Bug 35288 - ApplyFrameworkDefaults add plugin to add more fine grained defaults handling
Summary: ApplyFrameworkDefaults add plugin to add more fine grained defaults handling
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Mark Hofstetter
QA Contact: Testopia
URL:
Keywords:
Depends on: 35289
Blocks:
  Show dependency treegraph
 
Reported: 2023-11-08 12:41 UTC by Mark Hofstetter
Modified: 2025-05-02 16:21 UTC (History)
4 users (show)

See Also:
GIT URL:
Change sponsored?: Sponsored
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 35288: ApplyFrameworkDefaults add plugin to add more fine grained defaults handling (4.69 KB, patch)
2023-11-09 11:04 UTC, Mark Hofstetter
Details | Diff | Splinter Review
Bug 35288: ApplyFrameworkDefaults add plugin to add more fine grained defaults handling (4.74 KB, patch)
2025-01-02 19:08 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 35288: ApplyFrameworkDefaults add plugin to add more fine grained defaults handling (3.86 KB, patch)
2025-02-21 15:10 UTC, Mark Hofstetter
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Hofstetter 2023-11-08 12:41:47 UTC
The "new" ApplyFrameworkDefaults option is to coarse sometimes

ie quite often you also want to apply default values even if then field wasn't empty before e.g. on duplicating a new controlnumber should be generated etc

so I suggest (and already wrote) a new plugin hook

(see https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33669)
Comment 1 Mark Hofstetter 2023-11-09 11:04:39 UTC
Created attachment 158710 [details] [review]
Bug 35288: ApplyFrameworkDefaults add plugin to add more fine grained defaults handling

The "new" ApplyFrameworkDefaults option is to coarse sometimes
ie quite often you also want to apply default values even if then field wasn't empty before
e.g. on duplicating a new controlnumber should be generated etc

This patch adds a new plugin call 'framework_defaults_override' and tidies up the code a little

Plugin call:
Koha::Plugins->call( 'framework_defaults_override', $tag, $subfield, $value, $form_mode );

ie marc tag, marc subfield, the "before value", the action which triggered the change
possible actions are new, duplicate, changed (Framework), imported (via SRU)

the plugin has to return a hashref containing two keys

apply_override - undef or a true if the default value from the framework should be applied
override_default_value - a completly new derived value

sub framework_defaults_override {
    my ( $self, $tag, $subfield, $value, $form_mode ) = @_;
    # $framework_override->{override_default_value}
    # $framework_override->{apply_override}
    my $r = {override_default_value => undef, apply_override => undef};
    if ($tag eq '245' && $subfield eq 'a') {
        $r->{apply_override} = 1;
        $r->{override_default_value} =  $value .' added';
    }
    return $r;
}

for a demo implementation please see
https://github.com/HKS3/hks3-munde-framework-defaults-demo/blob/main/hks3-munde-framework-defaults-demo.kpz

To test:
1) apply patch
2) install demo plugin (and don't enable it yet)
3) create a record with a 245a field set to a value eg "My Title"
4) enable ApplyFrameworkDefaults for all 4 cases "Select all"
5) in Cataloging Edit the previously generated record
6) change framework via settings
7) nothing should happen to the title in 245a
8) enable plugin
9) repeat point 5+6
10) the title in 245a should change to  "My Title added"
11) please sign off

Sponsored-by: Styrian State Library
Co-authored-by: Thomas Klausner <domm@plix.at>
Comment 2 Mark Hofstetter 2023-11-09 14:41:42 UTC
another real life example using this plugin hook

https://github.com/HKS3/ApplyFrameworkDefaultsLibelle
Comment 3 Lucas Gass (lukeg) 2023-11-09 15:23:21 UTC
Mark,

Don't we need unit tests for this change?
Comment 4 Mark Hofstetter 2023-11-09 18:20:15 UTC
yes definitely, (there is always a but here, so here it comes) but

1. I just wanted to build a quick working prototype (needed for customer)
2. for the community to evaluate, if it's generally considered usefull
3. at the moment I am not sure how to test, as it's highly dependent on the UI 

if you could point me to a useful test strategy or a similar test which relies on changing the framework in Biblio Edit I'd be glad to make a test
Comment 5 Lucas Gass (lukeg) 2025-01-02 19:07:32 UTC
This works, I'd like to move it along. I'll let the QA decide about the need for unit tests or not.
Comment 6 Lucas Gass (lukeg) 2025-01-02 19:08:15 UTC
Created attachment 176105 [details] [review]
Bug 35288: ApplyFrameworkDefaults add plugin to add more fine grained defaults handling

The "new" ApplyFrameworkDefaults option is to coarse sometimes
ie quite often you also want to apply default values even if then field wasn't empty before
e.g. on duplicating a new controlnumber should be generated etc

This patch adds a new plugin call 'framework_defaults_override' and tidies up the code a little

Plugin call:
Koha::Plugins->call( 'framework_defaults_override', $tag, $subfield, $value, $form_mode );

ie marc tag, marc subfield, the "before value", the action which triggered the change
possible actions are new, duplicate, changed (Framework), imported (via SRU)

the plugin has to return a hashref containing two keys

apply_override - undef or a true if the default value from the framework should be applied
override_default_value - a completly new derived value

sub framework_defaults_override {
    my ( $self, $tag, $subfield, $value, $form_mode ) = @_;
    # $framework_override->{override_default_value}
    # $framework_override->{apply_override}
    my $r = {override_default_value => undef, apply_override => undef};
    if ($tag eq '245' && $subfield eq 'a') {
        $r->{apply_override} = 1;
        $r->{override_default_value} =  $value .' added';
    }
    return $r;
}

for a demo implementation please see
https://github.com/HKS3/hks3-munde-framework-defaults-demo/blob/main/hks3-munde-framework-defaults-demo.kpz

To test:
1) apply patch
2) install demo plugin (and don't enable it yet)
3) create a record with a 245a field set to a value eg "My Title"
4) enable ApplyFrameworkDefaults for all 4 cases "Select all"
5) in Cataloging Edit the previously generated record
6) change framework via settings
7) nothing should happen to the title in 245a
8) enable plugin
9) repeat point 5+6
10) the title in 245a should change to  "My Title added"
11) please sign off

Sponsored-by: Styrian State Library
Co-authored-by: Thomas Klausner <domm@plix.at>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 7 Mark Hofstetter 2025-02-21 15:10:37 UTC
Created attachment 178506 [details] [review]
Bug 35288: ApplyFrameworkDefaults add plugin to add more fine grained defaults handling

The "new" ApplyFrameworkDefaults option is to coarse sometimes
ie quite often you also want to apply default values even if then field wasn't empty before
e.g. on duplicating a new controlnumber should be generated etc

This patch adds a new plugin call 'framework_defaults_override' and tidies up the code a little

Plugin call:
Koha::Plugins->call( 'framework_defaults_override', $tag, $subfield, $value, $form_mode );

ie marc tag, marc subfield, the "before value", the action which triggered the change
possible actions are new, duplicate, changed (Framework), imported (via SRU)

the plugin has to return a hashref containing two keys

apply_override - undef or a true if the default value from the framework should be applied
override_default_value - a completly new derived value

sub framework_defaults_override {
    my ( $self, $tag, $subfield, $value, $form_mode ) = @_;
    # $framework_override->{override_default_value}
    # $framework_override->{apply_override}
    my $r = {override_default_value => undef, apply_override => undef};
    if ($tag eq '245' && $subfield eq 'a') {
        $r->{apply_override} = 1;
        $r->{override_default_value} =  $value .' added';
    }
    return $r;
}

for a demo implementation please see
https://github.com/HKS3/hks3-munde-framework-defaults-demo/blob/main/hks3-munde-framework-defaults-demo.kpz

To test:
1) apply patch
2) install demo plugin (and don't enable it yet)
3) create a record with a 245a field set to a value eg "My Title"
4) enable ApplyFrameworkDefaults for all 4 cases "Select all"
5) in Cataloging Edit the previously generated record
6) change framework via settings
7) nothing should happen to the title in 245a
8) enable plugin
9) repeat point 5+6
10) the title in 245a should change to  "My Title added"
11) please sign off

Sponsored-by: Styrian State Library
Co-authored-by: Thomas Klausner <domm@plix.at>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 8 Mark Hofstetter 2025-02-21 15:11:23 UTC
should apply again
Comment 9 Janusz Kaczmarek 2025-05-02 16:21:06 UTC
There is a problem with this patch: the patch sets $form_mode to 'new' when one is importing record through Z39.50 ($op is not defined for Z39.50 import so the first condition $op eq '' is true, and $form_mode is set to 'new'). In this case $form_mode should be set to 'imported' but is not.

To confirm:
1. set ApplyFrameworkDefaults to 'when cataloguing new records only'
2. in the default framework set any default value that you are not expecting to be present in a record imported from a Z39.50 source
3. import a record from z Z39.50 server
4. you will see the defined default value in the editor while you should not

The problem of not identifying properly Z39.50 context for default values is present also in the current main branch, and could be easily corrected by changing line 96 of Koha/UI/Form/Builder/Biblio.pm from

     && (   ( $applydefaults =~ /new/ && !$self->{biblionumber} )

into

     && (   ( $applydefaults =~ /new/ && ! defined $self->{biblionumber} )


But with this patch (35288) the logic of setting the context changes completely. Having this patch there is no sense to make a separate patch for the spotted problem, but it would be more wise IMO to correct this patch to set the context for Z39.50 import properly.