Bug 19289 - Allow configuration of the fields on the 'Catalog details' form in the acquisition baskets
Summary: Allow configuration of the fields on the 'Catalog details' form in the acquis...
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low new feature (vote)
Assignee: Jonathan Druart
QA Contact: Katrin Fischer
URL:
Whiteboard:
Keywords:
Depends on: 3841 20540
Blocks: 21316 20087 21537
  Show dependency treegraph
 
Reported: 2017-09-11 15:11 UTC by Séverine Queune
Modified: 2018-10-10 15:04 UTC (History)
4 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Medium patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
When creating a new order on an acquisition basket, bibliographic fields displayed on 'Catalog details' can now be customized. The system preference 'UseACQFrameworkForBiblioRecords' must be enabled. Fields are set on the 'ACQ' MARC framework.


Attachments
Bug 19289: Add existing fields to the ACQ framework (6.01 KB, patch)
2017-10-26 20:38 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19289: Use the ACQ framework to display bibliographic details (28.44 KB, patch)
2017-10-26 20:39 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19289: Add existing fields to the ACQ framework (6.05 KB, patch)
2017-10-29 20:01 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 19289: Use the ACQ framework to display bibliographic details (28.45 KB, patch)
2017-10-29 20:01 UTC, Josef Moravec
Details | Diff | Splinter Review
Add to basket from new empty recvord (36.80 KB, image/jpeg)
2017-11-08 09:18 UTC, Séverine Queune
Details
ACQ framework - Display only used tags/subfields (93.17 KB, image/png)
2017-11-08 09:21 UTC, Séverine Queune
Details
Bug 19289: Add existing fields to the ACQ framework (6.11 KB, patch)
2017-11-13 10:28 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 19289: Use the ACQ framework to display bibliographic details (28.51 KB, patch)
2017-11-13 10:28 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 19289: Remove reference to a non-existent .js file (1.19 KB, patch)
2018-01-18 18:14 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19289: Use jQuery validate plugin (4.44 KB, patch)
2018-01-19 17:14 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19289: Add existing fields to the ACQ framework (6.12 KB, patch)
2018-02-27 13:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19289: Use the ACQ framework to display bibliographic details (28.01 KB, patch)
2018-02-27 13:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19289: Use jQuery validate plugin (4.15 KB, patch)
2018-02-27 13:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19289: Use copyrightdate instead of publicationyear for MARC21 (1.91 KB, patch)
2018-02-27 13:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19289: Extend size of inputs to 50 (1.18 KB, patch)
2018-04-06 20:16 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19289: Add existing fields to the ACQ framework (6.18 KB, patch)
2018-04-07 09:08 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 19289: Use the ACQ framework to display bibliographic details (28.07 KB, patch)
2018-04-07 09:08 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 19289: Use jQuery validate plugin (4.21 KB, patch)
2018-04-07 09:08 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 19289: Use copyrightdate instead of publicationyear for MARC21 (1.97 KB, patch)
2018-04-07 09:09 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 19289: Extend size of inputs to 50 (1.23 KB, patch)
2018-04-07 09:09 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 19289: (follow-up) Add existing fields to the ACQ framework (24.44 KB, patch)
2018-04-09 15:53 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19289: (follow-up) Add existing fields to the ACQ framework (26.85 KB, patch)
2018-04-10 15:12 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19289: (follow-up) Add existing fields to the ACQ framework (26.91 KB, patch)
2018-04-11 05:56 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Séverine Queune 2017-09-11 15:11:26 UTC
The BULAC (http://www.bulac.fr/) would like to modify the existing form of the bibliographic details when adding an order ‘From a new (empty) record’ in a basket.

The ACQ MARC framework already customize the ‘Item’ block, it should be the same for the ‘Catalog details’ part, for important informations (for our staff) are missing when ordering a new book from a basket, such as language, classification, ISSN for serials, etc.

As long as “Change sponsored?” is set on “Seeking developer”, anyone wanting to quote us for the development of such a feature or just ask for more information is welcome to write to us: <sigb@bulac.fr>.
Comment 1 Jonathan Druart 2017-10-26 20:38:58 UTC
Created attachment 68640 [details] [review]
Bug 19289: Add existing fields to the ACQ framework

When ordering, the bibliographic details contain the title, author,
publisher code, publication year, isbn and series info.
To avoid regression to provide the same behaviour, we will add these
fields to the ACQ framework

NOTE: This patch makes the change only for
  - en
  - fr / unimarc complet

Other frameworks will be updated before to be pushed, to avoid
unecessary rebases
Comment 2 Jonathan Druart 2017-10-26 20:39:02 UTC
Created attachment 68641 [details] [review]
Bug 19289: Use the ACQ framework to display bibliographic details

The ACQ MARC framework is only used for the ‘Item’ block.
This patch add the ability to define biblio fields (!= 995 or 952) to
customize the display of the bibliographic details when ordering.

This new feature is controlled by a new pref:
UseACQFrameworkForBiblioRecords

Test plan:
- Create a new installation to populate the ACQ framework correctly
- Set the pref UseACQFrameworkForBiblioRecords to "Use"
- Create a new order
=> You will see the lib from the ACQ framework
- Add/remove/update biblio subfields in the ACQ framework
- Create a new order
=> You should see the new subfields displayed

Note for QA: I though I would be able to refactor existing code to make
it more flexible, but it is a bit messy and lost a lot of time. I
finally decided to copy/paste the existing code. I simplified it as, I
think, we do not want the plugin, etc. like in the full biblio editor.
Comment 3 Josef Moravec 2017-10-29 20:01:49 UTC
Created attachment 68810 [details] [review]
Bug 19289: Add existing fields to the ACQ framework

When ordering, the bibliographic details contain the title, author,
publisher code, publication year, isbn and series info.
To avoid regression to provide the same behaviour, we will add these
fields to the ACQ framework

NOTE: This patch makes the change only for
  - en
  - fr / unimarc complet

Other frameworks will be updated before to be pushed, to avoid
unecessary rebases

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 4 Josef Moravec 2017-10-29 20:01:54 UTC
Created attachment 68811 [details] [review]
Bug 19289: Use the ACQ framework to display bibliographic details

The ACQ MARC framework is only used for the ‘Item’ block.
This patch add the ability to define biblio fields (!= 995 or 952) to
customize the display of the bibliographic details when ordering.

This new feature is controlled by a new pref:
UseACQFrameworkForBiblioRecords

Test plan:
- Create a new installation to populate the ACQ framework correctly
- Set the pref UseACQFrameworkForBiblioRecords to "Use"
- Create a new order
=> You will see the lib from the ACQ framework
- Add/remove/update biblio subfields in the ACQ framework
- Create a new order
=> You should see the new subfields displayed

Note for QA: I though I would be able to refactor existing code to make
it more flexible, but it is a bit messy and lost a lot of time. I
finally decided to copy/paste the existing code. I simplified it as, I
think, we do not want the plugin, etc. like in the full biblio editor.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 5 Séverine Queune 2017-11-06 17:29:14 UTC
I wanted to test this patch even thought it's signed off but I couldn't apply it on Biblibre's sandbox.
Comment 6 Jonathan Druart 2017-11-06 18:47:36 UTC
(In reply to Séverine Queune from comment #5)
> I wanted to test this patch even thought it's signed off but I couldn't
> apply it on Biblibre's sandbox.

It applies correctly for me on top of bug 3841, did you apply the dependency patches before?
Comment 7 Séverine Queune 2017-11-07 15:43:29 UTC
(In reply to Jonathan Druart from comment #6)
> (In reply to Séverine Queune from comment #5)
> > I wanted to test this patch even thought it's signed off but I couldn't
> > apply it on Biblibre's sandbox.
> 
> It applies correctly for me on top of bug 3841, did you apply the dependency
> patches before?
My bad, I didn't see the dependency...

It seems to work quite good.
I tested with new empty records and existing records and all fields are diplayed as expected.

I just wonder about these fields in the 'Catalog details' framework that I didn't found on the MARC framework structure :
9 (RLIN) (6489)
9 (RLIN) (6549)
9 (RLIN) (6559)
9 (RLIN) (6569)
9 (RLIN) (6579)
9 (RLIN) (6589)
9 (RLIN) (6629)
I don't know where they come from and i can't mask them.
Comment 8 Jonathan Druart 2017-11-07 17:21:42 UTC
Can you detail please?
This is what I see when I install using "en"/MARC21: https://screenshots.firefox.com/cJdFOY6Lk1Ves2KK/pro.kohadev.org
Comment 9 Séverine Queune 2017-11-08 09:18:55 UTC
Created attachment 69015 [details]
Add to basket from new empty recvord

I worked on Biblibre's sandbox #2 and modified the ACQ framework to get the structure we want to use, but I couldn’t found where the "9 (RLIN)" fields that I see when I add an order from a new empty record come from.
Comment 10 Séverine Queune 2017-11-08 09:21:54 UTC
Created attachment 69016 [details]
ACQ framework -  Display only used tags/subfields

Here is the ACQ framework structure with only the used subfields and as you can see, there is no informations about "9 (RLIN)" fields.
Comment 11 Jonathan Druart 2017-11-08 12:32:31 UTC
Hi Séverine, I have created the correct ACQ framework only for:

en/marcflavour/marc21/mandatory/marc21_framework_DEFAULT.sql
fr-FR/marcflavour/unimarc_complet/Obligatoire/framework_DEFAULT.sql

When you setup the sandbox, tick "No database (run webinstaller)" to use the files used from the Koha codebase and not the database created for the sandbox.

Otherwise you can remove the subfield (@) from 009.
Comment 12 Séverine Queune 2017-11-08 15:26:56 UTC
(In reply to Jonathan Druart from comment #11)
> Hi Séverine, I have created the correct ACQ framework only for:
> 
> en/marcflavour/marc21/mandatory/marc21_framework_DEFAULT.sql
> fr-FR/marcflavour/unimarc_complet/Obligatoire/framework_DEFAULT.sql
> 
> When you setup the sandbox, tick "No database (run webinstaller)" to use the
> files used from the Koha codebase and not the database created for the
> sandbox.
> 
> Otherwise you can remove the subfield (@) from 009.

Thanks Jonathan !
Comment 13 Séverine Queune 2017-11-08 15:31:40 UTC
I can't sign it from Biblibre's sandbox because of the dependency, but ok for me !
Comment 14 Nicolas Legrand 2017-11-13 10:28:53 UTC
Created attachment 69114 [details] [review]
Bug 19289: Add existing fields to the ACQ framework

When ordering, the bibliographic details contain the title, author,
publisher code, publication year, isbn and series info.
To avoid regression to provide the same behaviour, we will add these
fields to the ACQ framework

NOTE: This patch makes the change only for
  - en
  - fr / unimarc complet

Other frameworks will be updated before to be pushed, to avoid
unecessary rebases

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>
Comment 15 Nicolas Legrand 2017-11-13 10:28:58 UTC
Created attachment 69115 [details] [review]
Bug 19289: Use the ACQ framework to display bibliographic details

The ACQ MARC framework is only used for the ‘Item’ block.
This patch add the ability to define biblio fields (!= 995 or 952) to
customize the display of the bibliographic details when ordering.

This new feature is controlled by a new pref:
UseACQFrameworkForBiblioRecords

Test plan:
- Create a new installation to populate the ACQ framework correctly
- Set the pref UseACQFrameworkForBiblioRecords to "Use"
- Create a new order
=> You will see the lib from the ACQ framework
- Add/remove/update biblio subfields in the ACQ framework
- Create a new order
=> You should see the new subfields displayed

Note for QA: I though I would be able to refactor existing code to make
it more flexible, but it is a bit messy and lost a lot of time. I
finally decided to copy/paste the existing code. I simplified it as, I
think, we do not want the plugin, etc. like in the full biblio editor.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>
Comment 16 Nicolas Legrand 2018-01-18 14:52:50 UTC
is it me or there is a catalog_details.js that should be there and is not?
Comment 17 Jonathan Druart 2018-01-18 18:14:53 UTC
Created attachment 70706 [details] [review]
Bug 19289: Remove reference to a non-existent .js file
Comment 18 Jonathan Druart 2018-01-18 18:15:43 UTC
(In reply to Nicolas Legrand from comment #16)
> is it me or there is a catalog_details.js that should be there and is not?

It was a first try, but I did not create this file finally. This line should be removed.
Comment 19 Séverine Queune 2018-01-19 14:27:51 UTC
Hi Jonathan, 

Nicolas applied this morning the patch in our local instance, and everything seems to work as expected, except that we don't have any more warnings if required fields are empty, and the order is saved.

I just tried once again in Biblibre's sandbox#2 and behaviour is the same.
I'm really sorry I didn't notice it during my tests, it's not logical for me to try to save a form without the required fields so I didn't think about testing it.

Do you need a new bug report ?
Comment 20 Jonathan Druart 2018-01-19 17:14:36 UTC
Created attachment 70765 [details] [review]
Bug 19289: Use jQuery validate plugin

This is quite dirty but rewrite the JS checks is out of the scope of
this bug report.
Comment 21 Séverine Queune 2018-01-24 16:15:13 UTC
(In reply to Jonathan Druart from comment #20)
> Created attachment 70765 [details] [review] [review]
> Bug 19289: Use jQuery validate plugin
> 
> This is quite dirty but rewrite the JS checks is out of the scope of
> this bug report.

Hi Jonathan, 
I just create a new bud report (#20087) for this problem.
Comment 22 Katrin Fischer 2018-02-18 20:47:40 UTC
QA tools pass.

Some first notes from reading the code:

1) installer/data/mysql/en/marcflavour/marc21/mandatory/marc21_framework_DEFAULT.sql

It looks like the code makes some assumptions about biblioitems.publicationyear. There is something you will not like about this field: It's only used by UNIMARC by default. MARC21 installations safe that information to biblio.copyrightdate. Both use cases need to be supported.

2) Only en and fr-FR installers are changed?

3) Atomicupdate doesn't include changes for creating the ACQ framework.

3) koha-tmpl/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt
A lot of FIXMEs in this code?

Jonathan, can you please take a look?
Comment 23 Jonathan Druart 2018-02-18 21:16:32 UTC
(In reply to Katrin Fischer from comment #22)
> QA tools pass.
> 
> Some first notes from reading the code:
> 
> 1)
> installer/data/mysql/en/marcflavour/marc21/mandatory/
> marc21_framework_DEFAULT.sql
> 
> It looks like the code makes some assumptions about
> biblioitems.publicationyear. There is something you will not like about this
> field: It's only used by UNIMARC by default. MARC21 installations safe that
> information to biblio.copyrightdate. Both use cases need to be supported.

It is the current behaviour, neworderempty.tt displays biblioitems.publicationyear.
If it is an existing bug it needs to be fixed on a separate bug report.

> 2) Only en and fr-FR installers are changed?

Yes, what I wrote in the first commit message "Other frameworks will be updated before to be pushed, to avoid unecessary rebases"
Same changes will be copied/pasted to other installer files before push.


> 3) Atomicupdate doesn't include changes for creating the ACQ framework.

ACQ framework was added by bug 3841 for new installation. I do not think we should deal with existing installation (what about those having already the framework?)

> 3) koha-tmpl/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt
> A lot of FIXMEs in this code?

It is to highlight the fact that the default values defined in ACQ will not be taken into account. I can remove it if you think it is confusing.
Comment 24 Katrin Fischer 2018-02-27 07:07:51 UTC
Hi Jonathan,

continuing here. Could you please rebase?

(In reply to Jonathan Druart from comment #23)
> (In reply to Katrin Fischer from comment #22)
> > QA tools pass.
> > 
> > Some first notes from reading the code:
> > 
> > 1)
> > installer/data/mysql/en/marcflavour/marc21/mandatory/
> > marc21_framework_DEFAULT.sql
> > 
> > It looks like the code makes some assumptions about
> > biblioitems.publicationyear. There is something you will not like about this
> > field: It's only used by UNIMARC by default. MARC21 installations safe that
> > information to biblio.copyrightdate. Both use cases need to be supported.
> 
> It is the current behaviour, neworderempty.tt displays
> biblioitems.publicationyear.
> If it is an existing bug it needs to be fixed on a separate bug report.

I have thought about this some and I think for MARC21 just replace publicationyear with copyrightdate. The problem is: it will create on mapping. There is no field mapped to publicationyear in a MARC21 installation so the SQL will do nothing as it is on a MARC21 installation most likely. If you change it to copyrightyear the field will be filled as it's in the other frameworks and we will only have to deal with a display problem. Ideal would probably be check what exists in the frameworks you are pulling from and use that.

> > 2) Only en and fr-FR installers are changed?
> 
> Yes, what I wrote in the first commit message "Other frameworks will be
> updated before to be pushed, to avoid unecessary rebases"
> Same changes will be copied/pasted to other installer files before push.

OK

> > 3) Atomicupdate doesn't include changes for creating the ACQ framework.
> 
> ACQ framework was added by bug 3841 for new installation. I do not think we
> should deal with existing installation (what about those having already the
> framework?)

On my installation that meant after activating the rule I had an empty framework. The ACQ framework was only created with 952, so no fields the record itself. Giving a more complete sample with the most important fields would be good as entering this data is very annoying. At the moment an acq record is created with the default framework - will it be ACQ now? Couldn't test because of the conflicts. I am a bit worried people will forget to add important fields like the leader (causes problems with encoding), 999 (causing problems at indexing) and others.

> 
> > 3) koha-tmpl/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt
> > A lot of FIXMEs in this code?
> 
> It is to highlight the fact that the default values defined in ACQ will not
> be taken into account. I can remove it if you think it is confusing

As some code editors let you highlight strings like FIXME I think we should not use if it's not a real FIXME maybe. Why will the default not be used? Are there other limits (plugins?)
Comment 25 Jonathan Druart 2018-02-27 13:45:56 UTC
(In reply to Katrin Fischer from comment #24)
> Hi Jonathan,
> 
> continuing here. Could you please rebase?

Will do.

> (In reply to Jonathan Druart from comment #23)
> > (In reply to Katrin Fischer from comment #22)
> > > QA tools pass.
> > > 
> > > Some first notes from reading the code:
> > > 
> > > 1)
> > > installer/data/mysql/en/marcflavour/marc21/mandatory/
> > > marc21_framework_DEFAULT.sql
> > > 
> > > It looks like the code makes some assumptions about
> > > biblioitems.publicationyear. There is something you will not like about this
> > > field: It's only used by UNIMARC by default. MARC21 installations safe that
> > > information to biblio.copyrightdate. Both use cases need to be supported.
> > 
> > It is the current behaviour, neworderempty.tt displays
> > biblioitems.publicationyear.
> > If it is an existing bug it needs to be fixed on a separate bug report.
> 
> I have thought about this some and I think for MARC21 just replace
> publicationyear with copyrightdate. The problem is: it will create on
> mapping. There is no field mapped to publicationyear in a MARC21
> installation so the SQL will do nothing as it is on a MARC21 installation
> most likely. If you change it to copyrightyear the field will be filled as
> it's in the other frameworks and we will only have to deal with a display
> problem. Ideal would probably be check what exists in the frameworks you are
> pulling from and use that.

I am not sure I understand the problem neither the solution, but I have tried and provided a patch.
Let me know if it is what you had in mind.

> > > 3) Atomicupdate doesn't include changes for creating the ACQ framework.
> > 
> > ACQ framework was added by bug 3841 for new installation. I do not think we
> > should deal with existing installation (what about those having already the
> > framework?)
> 
> On my installation that meant after activating the rule I had an empty
> framework. The ACQ framework was only created with 952, so no fields the
> record itself. Giving a more complete sample with the most important fields
> would be good as entering this data is very annoying. At the moment an acq
> record is created with the default framework - will it be ACQ now? Couldn't
> test because of the conflicts. I am a bit worried people will forget to add
> important fields like the leader (causes problems with encoding), 999
> (causing problems at indexing) and others.

ACQ is only use for display purpose here, so no need to add fields we do not want to see.

> > > 3) koha-tmpl/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt
> > > A lot of FIXMEs in this code?
> > 
> > It is to highlight the fact that the default values defined in ACQ will not
> > be taken into account. I can remove it if you think it is confusing
> 
> As some code editors let you highlight strings like FIXME I think we should
> not use if it's not a real FIXME maybe. Why will the default not be used?
> Are there other limits (plugins?)

No, nothing else is used, no default, no plugin, etc.
We display a dropdown list if linked with a authorised value list and make the field mandatory if marked as it in the framework.
Comment 26 Jonathan Druart 2018-02-27 13:47:18 UTC
Created attachment 72256 [details] [review]
Bug 19289: Add existing fields to the ACQ framework

When ordering, the bibliographic details contain the title, author,
publisher code, publication year, isbn and series info.
To avoid regression to provide the same behaviour, we will add these
fields to the ACQ framework

NOTE: This patch makes the change only for
  - en
  - fr / unimarc complet

Other frameworks will be updated before to be pushed, to avoid
unecessary rebases

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>
Comment 27 Jonathan Druart 2018-02-27 13:47:24 UTC
Created attachment 72257 [details] [review]
Bug 19289: Use the ACQ framework to display bibliographic details

The ACQ MARC framework is only used for the ‘Item’ block.
This patch add the ability to define biblio fields (!= 995 or 952) to
customize the display of the bibliographic details when ordering.

This new feature is controlled by a new pref:
UseACQFrameworkForBiblioRecords

Test plan:
- Create a new installation to populate the ACQ framework correctly
- Set the pref UseACQFrameworkForBiblioRecords to "Use"
- Create a new order
=> You will see the lib from the ACQ framework
- Add/remove/update biblio subfields in the ACQ framework
- Create a new order
=> You should see the new subfields displayed

Note for QA: I though I would be able to refactor existing code to make
it more flexible, but it is a bit messy and lost a lot of time. I
finally decided to copy/paste the existing code. I simplified it as, I
think, we do not want the plugin, etc. like in the full biblio editor.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>
Comment 28 Jonathan Druart 2018-02-27 13:47:28 UTC
Created attachment 72258 [details] [review]
Bug 19289: Use jQuery validate plugin

This is quite dirty but rewrite the JS checks is out of the scope of
this bug report.
Comment 29 Jonathan Druart 2018-02-27 13:47:32 UTC
Created attachment 72259 [details] [review]
Bug 19289: Use copyrightdate instead of publicationyear for MARC21

See comment 24
Comment 30 Katrin Fischer 2018-04-03 05:42:13 UTC
Hi Jonathan, 

Continuing testing:

I've used the SQL from the installer file to create my ACQ framework.

1) Looking at the code, why this change on the ISBD view?
     filters => 'ViewPolicy',
     options => {
         interface => 'intranet',
-        frameworkcode => $framework
+        frameworkcode => 'ACQ'
     },

2) The input fields are much shorter with the ACQ framework active. Can we extend them a bit to be a similar size than before? Especially title and series fields are too short.

3) The descriptions of the fields coming from the default framework are not very nice because they are out of context without the field description. For example 440 says title, but it's not clear it's the series title. The MARC helps a bit.

4) I filled in title and series title and got an error on saving:
Indicator in 440 is empty at /home/vagrant/kohaclone/C4/Biblio.pm line 2294.
:13: parser error : Opening and ending tag mismatch: record line 6 and datafield
</datafield>
            ^
:14: parser error : Opening and ending tag mismatch: collection line 2 and record
</record>
         ^
:15: parser error : Extra content at the end of the document
</collection>
Comment 31 Jonathan Druart 2018-04-06 20:16:00 UTC
Created attachment 73800 [details] [review]
Bug 19289: Extend size of inputs to 50
Comment 32 Jonathan Druart 2018-04-06 20:19:14 UTC
(In reply to Katrin Fischer from comment #30)
> Hi Jonathan, 
> 
> Continuing testing:
> 
> I've used the SQL from the installer file to create my ACQ framework.
> 
> 1) Looking at the code, why this change on the ISBD view?
>      filters => 'ViewPolicy',
>      options => {
>          interface => 'intranet',
> -        frameworkcode => $framework
> +        frameworkcode => 'ACQ'
>      },

No idea! Sounds like a mistake.
Done.

> 2) The input fields are much shorter with the ACQ framework active. Can we
> extend them a bit to be a similar size than before? Especially title and
> series fields are too short.

Done.

> 3) The descriptions of the fields coming from the default framework are not
> very nice because they are out of context without the field description. For
> example 440 says title, but it's not clear it's the series title. The MARC
> helps a bit.

We could show the description of the tag too, but it will display bad as well, "tag / subfield" would be: "SERIES STATEMENT/ADDED ENTRY--TITLE / Title".

Do you have a suggestion to improve that?

> 4) I filled in title and series title and got an error on saving:
> Indicator in 440 is empty at /home/vagrant/kohaclone/C4/Biblio.pm line 2294.
> :13: parser error : Opening and ending tag mismatch: record line 6 and
> datafield
> </datafield>
>             ^
> :14: parser error : Opening and ending tag mismatch: collection line 2 and
> record
> </record>
>          ^
> :15: parser error : Extra content at the end of the document
> </collection>

It's a bug in TransformHtmlToXml, I have opened and attached a patch on bug 20540.
Comment 33 Katrin Fischer 2018-04-07 09:08:38 UTC
Created attachment 73819 [details] [review]
Bug 19289: Add existing fields to the ACQ framework

When ordering, the bibliographic details contain the title, author,
publisher code, publication year, isbn and series info.
To avoid regression to provide the same behaviour, we will add these
fields to the ACQ framework

NOTE: This patch makes the change only for
  - en
  - fr / unimarc complet

Other frameworks will be updated before to be pushed, to avoid
unecessary rebases

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 34 Katrin Fischer 2018-04-07 09:08:54 UTC
Created attachment 73820 [details] [review]
Bug 19289: Use the ACQ framework to display bibliographic details

The ACQ MARC framework is only used for the ‘Item’ block.
This patch add the ability to define biblio fields (!= 995 or 952) to
customize the display of the bibliographic details when ordering.

This new feature is controlled by a new pref:
UseACQFrameworkForBiblioRecords

Test plan:
- Create a new installation to populate the ACQ framework correctly
- Set the pref UseACQFrameworkForBiblioRecords to "Use"
- Create a new order
=> You will see the lib from the ACQ framework
- Add/remove/update biblio subfields in the ACQ framework
- Create a new order
=> You should see the new subfields displayed

Note for QA: I though I would be able to refactor existing code to make
it more flexible, but it is a bit messy and lost a lot of time. I
finally decided to copy/paste the existing code. I simplified it as, I
think, we do not want the plugin, etc. like in the full biblio editor.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 35 Katrin Fischer 2018-04-07 09:08:58 UTC
Created attachment 73821 [details] [review]
Bug 19289: Use jQuery validate plugin

This is quite dirty but rewrite the JS checks is out of the scope of
this bug report.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 36 Katrin Fischer 2018-04-07 09:09:03 UTC
Created attachment 73822 [details] [review]
Bug 19289: Use copyrightdate instead of publicationyear for MARC21

See comment 24

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 37 Katrin Fischer 2018-04-07 09:09:07 UTC
Created attachment 73823 [details] [review]
Bug 19289: Extend size of inputs to 50

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 38 Katrin Fischer 2018-04-07 09:10:30 UTC
I will sign off on the dependent bug 20540.

Blocking for another reason: We still need a follow up for the other MARC21 and UNIMARC sample frameworks.
Comment 39 Jonathan Druart 2018-04-09 15:53:07 UTC
Created attachment 73878 [details] [review]
Bug 19289: (follow-up) Add existing fields to the ACQ framework

Changes for other frameworks
Comment 40 Katrin Fischer 2018-04-10 05:34:52 UTC
Looks ok, but I think you missed pl-PL in the last follow-up?
Comment 41 Jonathan Druart 2018-04-10 15:11:17 UTC
(In reply to Katrin Fischer from comment #40)
> Looks ok, but I think you missed pl-PL in the last follow-up?

Hum, I missed it because bug 3841 missed it already :-/
I have attached and pushed a follow-up for 3841.
Comment 42 Jonathan Druart 2018-04-10 15:12:48 UTC
Created attachment 73948 [details] [review]
Bug 19289: (follow-up) Add existing fields to the ACQ framework

Changes for other frameworks
Comment 43 Katrin Fischer 2018-04-11 05:56:04 UTC
Created attachment 73994 [details] [review]
Bug 19289: (follow-up) Add existing fields to the ACQ framework

Changes for other frameworks

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 44 Jonathan Druart 2018-04-11 19:54:16 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 45 Séverine Queune 2018-04-21 07:27:36 UTC
This my first release note (and it's in english, which is not a natural language for me) : I won't be offended if you think something/anything/everything must de rewrite !
Comment 46 Katrin Fischer 2018-04-21 07:42:23 UTC
Happy to see you around on bugzilla Severine :)
Comment 47 Nick Clemens 2018-05-14 19:34:04 UTC
Enhancement, not backported for 17.11.x