Bug 13735 - Item form in serials module doesn't respect max length set in the frameworks
Summary: Item form in serials module doesn't respect max length set in the frameworks
Status: Pushed to Master
Alias: None
Product: Koha
Classification: Unclassified
Component: Serials (show other bugs)
Version: master
Hardware: All All
: P5 - low trivial (vote)
Target Milestone: ---
Assignee: Katrin Fischer
QA Contact: Marcel de Rooy
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-02-20 01:45 UTC by James
Modified: 2019-03-22 20:35 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 13735: Update PrepareItemrecordDisplay to use maxlength (5.26 KB, patch)
2019-02-10 10:08 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 13735: Update PrepareItemrecordDisplay to use maxlength (5.33 KB, patch)
2019-02-12 14:02 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 13735: Update PrepareItemrecordDisplay to use maxlength (5.42 KB, patch)
2019-03-22 10:03 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 James 2015-02-20 01:45:21 UTC
Although the public note subfield is set to have a character limitation of 9999, it only allows 255 characters when adding an item through receive function in the serials module.
It is possible to add more characters to this field for items created this way by editing them in the biblio record.
Comment 1 Katrin Fischer 2019-02-10 09:35:33 UTC
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.
Comment 2 Katrin Fischer 2019-02-10 10:08:22 UTC
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
Comment 3 Katrin Fischer 2019-02-10 10:10:42 UTC
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.
Comment 4 Martin Renvoize 2019-02-12 14:02:12 UTC
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>
Comment 5 Martin Renvoize 2019-02-12 14:02:50 UTC
Good catch, patch works for me.. signing off.
Comment 6 Martin Renvoize 2019-02-12 15:01:41 UTC
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.
Comment 7 Marcel de Rooy 2019-03-22 10:01:08 UTC
(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.
Comment 8 Marcel de Rooy 2019-03-22 10:03:36 UTC
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>
Comment 9 Katrin Fischer 2019-03-22 14:31:12 UTC
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 :)
Comment 10 Katrin Fischer 2019-03-22 14:31:23 UTC
Thx for QA!
Comment 11 Nick Clemens 2019-03-22 20:35:29 UTC
Awesome work all!

Pushed to master for 19.05