32484 – Enable framework plugins when UseACQFrameworkForBiblioRecords is set

Bug 32484 - Enable framework plugins when UseACQFrameworkForBiblioRecords is set

Summary: Enable framework plugins when UseACQFrameworkForBiblioRecords is set

Status:	CLOSED FIXED

Alias:	None

Product:	Koha
Classification:	Unclassified
Component:	Acquisitions (show other bugs)
Version:	Main
Hardware:	All All

Importance:	P5 - low normal
Assignee:	Julian Maurice
QA Contact:	Marcel de Rooy

URL:
Keywords:

Duplicates (1):	31582 (view as bug list)
Depends on:
Blocks:	34993
	Show dependency tree / graph

Reported:	2022-12-16 15:31 UTC by Julian Maurice
Modified:	2024-07-04 20:37 UTC (History)
CC List:	5 users (show)

See Also:	31582 31536
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
Circulation function:

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
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

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)

Created attachment 144690 [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

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 (tcohen) 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 (ashimema) 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 (ashimema) 2023-05-11 10:39:34 UTC

Pushed as requested.

Comment 22 Lucas Gass (lukeg) 2023-05-15 21:39:15 UTC

Enhancement will not be backported to the 22.05.x series