Bug 32484 - Enable framework plugins when UseACQFrameworkForBiblioRecords is set
Summary: Enable framework plugins when UseACQFrameworkForBiblioRecords is set
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Julian Maurice
QA Contact: Marcel de Rooy
URL:
Keywords:
: 31582 (view as bug list)
Depends on:
Blocks: 34993
  Show dependency treegraph
 
Reported: 2022-12-16 15:31 UTC by Julian Maurice
Modified: 2023-10-12 15:43 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This bugfix enables the use of framework plugins when: - `UseACQFrameworkForBiblioRecords` is enabled, and - entering catalog details when adding items to a basket from a new (empty) record. This requires plugins to be enabled for fields in the `ACQ` framework.
Version(s) released in:
23.05.00,22.11.06


Attachments
Bug 32484: Enable plugins when UseACQFrameworkForBiblioRecords is set (64.28 KB, patch)
2022-12-16 15:32 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 32484: Enable plugins when UseACQFrameworkForBiblioRecords is set (64.32 KB, patch)
2022-12-18 04:19 UTC, David Nind
Details | Diff | Splinter Review
Bug 32484: Replace DateTime->now by Koha::DateUtils::dt_from_string (1.07 KB, patch)
2023-02-24 12:01 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 32484: Enable plugins when UseACQFrameworkForBiblioRecords is set (64.42 KB, patch)
2023-04-18 13:05 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 32484: Replace DateTime->now by Koha::DateUtils::dt_from_string (1.16 KB, patch)
2023-04-18 13:05 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 32484: (QA follow-up) Fixes to POD (1.41 KB, patch)
2023-04-18 13:05 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 32484: (QA follow-up) Polishing unit test (2.08 KB, patch)
2023-04-18 13:05 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 32484: (QA follow-up) Few typos (2.53 KB, patch)
2023-04-18 13:05 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Julian Maurice 2022-12-16 15:31:00 UTC

    
Comment 1 Julian Maurice 2022-12-16 15:32:54 UTC Comment hidden (obsolete)
Comment 2 David Nind 2022-12-18 04:19:47 UTC
Created attachment 144706 [details] [review]
Bug 32484: Enable plugins when UseACQFrameworkForBiblioRecords is set

Test plan:
1. Do not apply the patch yet
2. Make sure you have an ACQ biblio framework with some framework
   plugins enabled. Create it if you don't.
3. Enable UseACQFrameworkForBiblioRecords system preference
4. Create a new acquisition basket.
5. On this new basket, click on "+ Add to basket", then "From a new
   (empty) record"
6. You should see a simplified MARC editor based on ACQ framework.
   Confirm that the plugins are not enabled (no visible buttons, nothing
   happening on focus/blur)
7. Apply patch
8. Repeat step 5
9. You should now see the plugins' buttons (only if you enabled plugins
   that use the 'click' event)
10. Confirm that the enabled plugins work correctly

Signed-off-by: David Nind <david@davidnind.com>
Comment 3 David Nind 2022-12-18 04:43:53 UTC
Testing notes (using KTD):

1. MARC21 doesn't have as many framework plugins as UNIMARC.

2. I tested as follows - hope this is sufficient:

   2.1 Using the 'Fixed length control field (00000)': before the patch is applied it is automatically generated, but you can't select any options - after the patch you can select options.

  2.2 Added 005 (Date and time of last transaction) to the ACQ framework and used the marc21_field_005.pl plugin: before the patch is applied there is no value automatically applied to the field, after the patch is applied you can click on the field and it is populated.

3. The catalog details label width is very narrow - I think this should be widened, as was done as an exception for the item fields.
Comment 4 Marcel de Rooy 2023-02-24 10:48:48 UTC
 FAIL   koha-tmpl/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt
   FAIL   js_in_body
                A <script> tag found inside head, must be moved to the body (see bug 17858)


 FAIL   t/db_dependent/Koha/UI/Form/Builder/Biblio.t
   FAIL   forbidden patterns
                forbidden pattern: Use of DateTime->now should certainly be replaced with dt_from_string (bug 24840) (line 36)
Comment 5 Julian Maurice 2023-02-24 12:01:52 UTC
Created attachment 147298 [details] [review]
Bug 32484: Replace DateTime->now by Koha::DateUtils::dt_from_string
Comment 6 Julian Maurice 2023-02-24 12:03:33 UTC
(In reply to Marcel de Rooy from comment #4)
>  FAIL   koha-tmpl/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt
>    FAIL   js_in_body
>                 A <script> tag found inside head, must be moved to the body
> (see bug 17858)
Framework plugins do not work when all script tags are at the end of <body>.
That's why I moved them inside <head>

>  FAIL   t/db_dependent/Koha/UI/Form/Builder/Biblio.t
>    FAIL   forbidden patterns
>                 forbidden pattern: Use of DateTime->now should certainly be
> replaced with dt_from_string (bug 24840) (line 36)
Fixed by the last patch
Comment 7 Marcel de Rooy 2023-03-03 08:01:06 UTC
 Koha/UI/Form/Builder/Biblio.pm                | 372 +++++++++++++++
 acqui/neworderempty.pl                        |  60 ++-
 cataloguing/addbiblio.pl                      | 350 +++-----------
 .../prog/en/modules/acqui/neworderempty.tt    | 444 +++++++++---------
 koha-tmpl/intranet-tmpl/prog/js/cataloging.js |   3 +-
 t/db_dependent/Koha/UI/Form/Builder/Biblio.t  | 182 +++++++

Could you explain why we need to move so much code here? Hard to catch bugs in such refactoring ;)
Comment 8 Julian Maurice 2023-03-03 08:18:03 UTC
Trust me, I don't move code for fun :)

I needed to reuse the code from addbiblio.pl in neworderempty.pl so I moved it into a Koha:: module, that's it.
Comment 9 Marcel de Rooy 2023-04-18 12:14:50 UTC
Looking here
Comment 10 Marcel de Rooy 2023-04-18 13:04:21 UTC
This test
        ( $applydefaults =~ /changed/ && $changed_framework ) ||
wont work for default fw but currently we focus here on ACQ?
Comment 11 Marcel de Rooy 2023-04-18 13:05:22 UTC
Created attachment 149798 [details] [review]
Bug 32484: Enable plugins when UseACQFrameworkForBiblioRecords is set

Test plan:
1. Do not apply the patch yet
2. Make sure you have an ACQ biblio framework with some framework
   plugins enabled. Create it if you don't.
3. Enable UseACQFrameworkForBiblioRecords system preference
4. Create a new acquisition basket.
5. On this new basket, click on "+ Add to basket", then "From a new
   (empty) record"
6. You should see a simplified MARC editor based on ACQ framework.
   Confirm that the plugins are not enabled (no visible buttons, nothing
   happening on focus/blur)
7. Apply patch
8. Repeat step 5
9. You should now see the plugins' buttons (only if you enabled plugins
   that use the 'click' event)
10. Confirm that the enabled plugins work correctly

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 12 Marcel de Rooy 2023-04-18 13:05:24 UTC
Created attachment 149799 [details] [review]
Bug 32484: Replace DateTime->now by Koha::DateUtils::dt_from_string

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 13 Marcel de Rooy 2023-04-18 13:05:27 UTC
Created attachment 149800 [details] [review]
Bug 32484: (QA follow-up) Fixes to POD

Resolve:
 FAIL   Koha/UI/Form/Builder/Biblio.pm
   FAIL   pod coverage
                POD is missing for build_authorized_values_list
                POD is missing for create_key

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 14 Marcel de Rooy 2023-04-18 13:05:29 UTC
Created attachment 149801 [details] [review]
Bug 32484: (QA follow-up) Polishing unit test

Test plan, removed done_testing.
License.
Since we are in t/db, good to add transaction, although we currently
do not change anything here. But who knows?

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 15 Marcel de Rooy 2023-04-18 13:05:31 UTC
Created attachment 149802 [details] [review]
Bug 32484: (QA follow-up) Few typos

Test plan:
Run git grep "specifique key"
Run git grep "the fonction"

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 16 Julian Maurice 2023-04-18 13:16:06 UTC
Thank you Marcel for the QA and the follow-ups :)
Comment 17 Tomás Cohen Arazi 2023-04-21 13:37:17 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 18 Katrin Fischer 2023-04-23 21:23:45 UTC
*** Bug 31582 has been marked as a duplicate of this bug. ***
Comment 19 Katrin Fischer 2023-04-23 21:25:28 UTC
It appears that this fixed bug 31582. Any chance we could reasonably consider this for backporting maybe? This will also help immensely in not creating false record with missing LDR etc.
Comment 20 Martin Renvoize 2023-05-11 10:38:45 UTC
Many hands makes light work, thankyou everyone!

Pushed to 22.11.x for the next release
Comment 21 Martin Renvoize 2023-05-11 10:39:34 UTC
Pushed as requested.
Comment 22 Lucas Gass 2023-05-15 21:39:15 UTC
Enhancement will not be backported to the 22.05.x series