Bug 13735

It's true on master still. While the maxlength on the edit items for is set to 9999 it's 255 in the serials module.

Created attachment 84945 [details] [review]
Bug 13735: Update PrepareItemrecordDisplay to use maxlength

The PrepareItemrecordDisplay method used to generate the item
display on receiving serials was using a hardcoded maxlength
of 255 on input fields. This patch updates it to use the
maxlength defined in the frameworks instead as the normal
item form does.

To test:
- Create or reuse an existing subscription
- Make note of the framework the record uses
- Add an item on receive
- Check the maxlength on the various fields is set to 255
  (check source code or use developer tools)
- You can also verify that by adding a long note to
  one of the note fields
- Apply patch
- Repeat tests, the maxlength should now be 9999
- In your framework settings, change the length of one
  of the subfields to another value
- Verify that the item form reflects the change

I feel this was an oversight when maxlength was added to the frameworks (it was forever ago, but not there from the beginning). 

As this is a change to a routine in C4 it cries for unit tests, but as the method currently appears completely untested I am not sure where to start with this. Putting this out for sign-off and feedback for now.

Created attachment 85019 [details] [review]
Bug 13735: Update PrepareItemrecordDisplay to use maxlength

The PrepareItemrecordDisplay method used to generate the item
display on receiving serials was using a hardcoded maxlength
of 255 on input fields. This patch updates it to use the
maxlength defined in the frameworks instead as the normal
item form does.

To test:
- Create or reuse an existing subscription
- Make note of the framework the record uses
- Add an item on receive
- Check the maxlength on the various fields is set to 255
  (check source code or use developer tools)
- You can also verify that by adding a long note to
  one of the note fields
- Apply patch
- Repeat tests, the maxlength should now be 9999
- In your framework settings, change the length of one
  of the subfields to another value
- Verify that the item form reflects the change

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Good catch, patch works for me.. signing off.

With my QA hat on.. I don't think we should require a test here.. the code is hideously integrated and a test would end up being more complicated than the fix itself..  When it comes to refactoring this into Koha:: we will need lots of tests to cover these routines, spreading from integrations ui tests down to module unit tests.

Just my opinion for the next QAer to come along.

(In reply to Martin Renvoize from comment #6)
> With my QA hat on.. I don't think we should require a test here.. the code
> is hideously integrated and a test would end up being more complicated than
> the fix itself..  When it comes to refactoring this into Koha:: we will need
> lots of tests to cover these routines, spreading from integrations ui tests
> down to module unit tests.
> 
> Just my opinion for the next QAer to come along.

Yeah, no problem.

Created attachment 86881 [details] [review]
Bug 13735: Update PrepareItemrecordDisplay to use maxlength

The PrepareItemrecordDisplay method used to generate the item
display on receiving serials was using a hardcoded maxlength
of 255 on input fields. This patch updates it to use the
maxlength defined in the frameworks instead as the normal
item form does.

To test:
- Create or reuse an existing subscription
- Make note of the framework the record uses
- Add an item on receive
- Check the maxlength on the various fields is set to 255
  (check source code or use developer tools)
- You can also verify that by adding a long note to
  one of the note fields
- Apply patch
- Repeat tests, the maxlength should now be 9999
- In your framework settings, change the length of one
  of the subfields to another value
- Verify that the item form reflects the change

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Ultimately... we should use the same code for generating the form here that we use in catalouging as there are other issues (missing select2 functionality is an example) - but that will go way beyond this bug fix :)

Thx for QA!

Awesome work all!

Pushed to master for 19.05

Pushed to 18.11.x for 18.11.04

backported to 18.05.x for 18.05.12