Bug 30358 - Strip leading/trailing whitespace characters from input fields when cataloguing
Summary: Strip leading/trailing whitespace characters from input fields when cataloguing
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Aleisha Amohia
QA Contact: Testopia
URL:
Keywords:
: 30443 (view as bug list)
Depends on:
Blocks:
 
Reported: 2022-03-25 01:41 UTC by Aleisha Amohia
Modified: 2022-11-22 17:31 UTC (History)
6 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Medium patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 30358: Strip leading/trailing whitespace characters from input fields when cataloguing (4.59 KB, patch)
2022-03-25 03:13 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30358: Strip leading/trailing whitespace characters from input fields when cataloguing (8.18 KB, patch)
2022-03-28 03:10 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30358: (follow-up) Also strip inner newlines (5.42 KB, patch)
2022-04-04 02:36 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30358: (follow-up) Inner newlines should be replaced with a space (1.48 KB, patch)
2022-04-07 06:04 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30358: (follow-up) Fixing tests and including for inner newlines (2.11 KB, patch)
2022-04-07 06:04 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30358: (follow-up) Fixing tests and including for inner newlines (2.73 KB, patch)
2022-04-11 03:11 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30358: (follow-up) Fixing tests and including for inner newlines (3.39 KB, patch)
2022-04-12 22:53 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30358: (follow-up) Clarify syspref wording about fields affected (1.39 KB, patch)
2022-04-12 22:53 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30358: (follow-up) Consider field has multiple subfields of same key (1.11 KB, patch)
2022-04-27 22:56 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30358: (follow-up) Put multiple subfields fix on auth side (962 bytes, patch)
2022-05-02 04:58 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30358: Strip leading/trailing whitespace characters from input fields when cataloguing (8.18 KB, patch)
2022-07-09 22:29 UTC, David Nind
Details | Diff | Splinter Review
Bug 30358: (follow-up) Also strip inner newlines (5.42 KB, patch)
2022-07-09 22:29 UTC, David Nind
Details | Diff | Splinter Review
Bug 30358: (follow-up) Inner newlines should be replaced with a space (1.53 KB, patch)
2022-07-09 22:29 UTC, David Nind
Details | Diff | Splinter Review
Bug 30358: (follow-up) Fixing tests and including for inner newlines (3.44 KB, patch)
2022-07-09 22:29 UTC, David Nind
Details | Diff | Splinter Review
Bug 30358: (follow-up) Clarify syspref wording about fields affected (1.43 KB, patch)
2022-07-09 22:29 UTC, David Nind
Details | Diff | Splinter Review
Bug 30358: (follow-up) Consider field has multiple subfields of same key (1.16 KB, patch)
2022-07-09 22:29 UTC, David Nind
Details | Diff | Splinter Review
Bug 30358: (follow-up) Put multiple subfields fix on auth side (1008 bytes, patch)
2022-07-09 22:29 UTC, David Nind
Details | Diff | Splinter Review
Bug 30358: (follow-up) stripWhitespaceChars subroutine and tests (4.47 KB, patch)
2022-08-15 04:50 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30358: Strip leading/trailing whitespace characters from input fields when cataloguing (8.33 KB, patch)
2022-11-02 02:08 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30358: (follow-up) Also strip inner newlines (5.46 KB, patch)
2022-11-02 02:08 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30358: (follow-up) Inner newlines should be replaced with a space (1.53 KB, patch)
2022-11-02 02:08 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30358: (follow-up) Fixing tests and including for inner newlines (3.44 KB, patch)
2022-11-02 02:08 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30358: (follow-up) Clarify syspref wording about fields affected (1.44 KB, patch)
2022-11-02 02:08 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30358: (follow-up) Consider field has multiple subfields of same key (1.16 KB, patch)
2022-11-02 02:09 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30358: (follow-up) Put multiple subfields fix on auth side (1008 bytes, patch)
2022-11-02 02:09 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30358: (follow-up) stripWhitespaceChars subroutine and tests (4.47 KB, patch)
2022-11-02 02:09 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30358: (follow-up) Fixing ModBiblioMarc.t tests (1.91 KB, patch)
2022-11-03 01:13 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30358: Strip leading/trailing whitespace characters from input fields when cataloguing (8.29 KB, patch)
2022-11-03 06:34 UTC, David Nind
Details | Diff | Splinter Review
Bug 30358: (follow-up) Also strip inner newlines (5.42 KB, patch)
2022-11-03 06:34 UTC, David Nind
Details | Diff | Splinter Review
Bug 30358: (follow-up) Inner newlines should be replaced with a space (1.53 KB, patch)
2022-11-03 06:34 UTC, David Nind
Details | Diff | Splinter Review
Bug 30358: (follow-up) Fixing tests and including for inner newlines (3.44 KB, patch)
2022-11-03 06:34 UTC, David Nind
Details | Diff | Splinter Review
Bug 30358: (follow-up) Clarify syspref wording about fields affected (1.43 KB, patch)
2022-11-03 06:34 UTC, David Nind
Details | Diff | Splinter Review
Bug 30358: (follow-up) Consider field has multiple subfields of same key (1.16 KB, patch)
2022-11-03 06:34 UTC, David Nind
Details | Diff | Splinter Review
Bug 30358: (follow-up) Put multiple subfields fix on auth side (1008 bytes, patch)
2022-11-03 06:35 UTC, David Nind
Details | Diff | Splinter Review
Bug 30358: (follow-up) stripWhitespaceChars subroutine and tests (4.52 KB, patch)
2022-11-03 06:35 UTC, David Nind
Details | Diff | Splinter Review
Bug 30358: (follow-up) Fixing ModBiblioMarc.t tests (1.96 KB, patch)
2022-11-03 06:35 UTC, David Nind
Details | Diff | Splinter Review
Bug 30358: Strip leading/trailing whitespace characters from input fields when cataloguing (8.36 KB, patch)
2022-11-21 21:39 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30358: (follow-up) Also strip inner newlines (5.43 KB, patch)
2022-11-21 21:39 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30358: (follow-up) Inner newlines should be replaced with a space (1.53 KB, patch)
2022-11-21 21:39 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30358: (follow-up) Fixing tests and including for inner newlines (3.45 KB, patch)
2022-11-21 21:40 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30358: (follow-up) Clarify syspref wording about fields affected (1.43 KB, patch)
2022-11-21 21:40 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30358: (follow-up) Consider field has multiple subfields of same key (1.16 KB, patch)
2022-11-21 21:40 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30358: (follow-up) Put multiple subfields fix on auth side (1010 bytes, patch)
2022-11-21 21:40 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30358: (follow-up) stripWhitespaceChars subroutine and tests (4.52 KB, patch)
2022-11-21 21:40 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30358: (follow-up) Fixing ModBiblioMarc.t tests (1.96 KB, patch)
2022-11-21 21:40 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30358 (follow-up): Do not strip whitespace from control fields (2.89 KB, patch)
2022-11-21 21:40 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30358 (follow-up) Simplify regex (1.09 KB, patch)
2022-11-21 21:40 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 30358 (follow-up) Simplify regex (1.14 KB, patch)
2022-11-22 17:31 UTC, Hammat wele
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Aleisha Amohia 2022-03-25 01:41:03 UTC
Some libraries do a lot of copy pasting when cataloguing, which means occasionally copying over whitespace characters including line breaks and carriage breaks.

It would be good if Koha stripped these.
Comment 1 Aleisha Amohia 2022-03-25 03:13:30 UTC
Created attachment 132202 [details] [review]
Bug 30358: Strip leading/trailing whitespace characters from input fields when cataloguing

WIP
Comment 2 Aleisha Amohia 2022-03-28 03:10:27 UTC
Created attachment 132275 [details] [review]
Bug 30358: Strip leading/trailing whitespace characters from input fields when cataloguing

This enhancement adds a system preference StripWhitespaceChars which,
when enabled, will strip leading and trailing whitespace characters from
all fields when cataloguing both bibliographic records and authority
records. Whitespace characters that will be stripped are:
- spaces
- newlines
- carriage returns
- tabs

To test:
1. Apply patch and install database updates
2. Go to Administration, system preferences, find the new
StripWhitespaceChars preference. It should be "Don't strip" by default.
Change it to "Strip".
3. Search for a biblio record and edit it. Put some leading or trailing
whitespace characters in input fields and textarea fields and save.
4. Confirm these characters are removed when you save the record.
5. Repeat steps 3 and 4 for authority records.
6. Confirm tests pass t/db_dependent/Biblio/ModBiblioMarc.t

Sponsored-by: Educational Services Australia SCIS
Comment 3 Aleisha Amohia 2022-04-04 01:45:07 UTC
*** Bug 30443 has been marked as a duplicate of this bug. ***
Comment 4 Aleisha Amohia 2022-04-04 01:46:00 UTC
Line breaks and carriage returns are known to break legacy library system record imports and, text area fields in MARC records are not designed to carry formatting, therefore it is implied that the data would be saved as a plain text string with no added formatting.

For users who follow these cataloguing standards, there should be a setting to ask Koha to strip these.

Planning to amend StripWhitespaceChars to also strip inner line breaks and carriage returns.
Comment 5 Aleisha Amohia 2022-04-04 02:36:24 UTC
Created attachment 132919 [details] [review]
Bug 30358: (follow-up) Also strip inner newlines

This patch amends the StripWhitespaceChars system preference to also
strip inner newlines (line breaks and carriage returns) when enabled.
Comment 6 Owen Leonard 2022-04-04 14:02:45 UTC
This seems to work but the test does pass:

t/db_dependent/Biblio/ModBiblioMarc.t .. 1/2
    #   Failed test 'Title still has whitespace characters because StripWhitespaceChars is disabled'
    #   at t/db_dependent/Biblio/ModBiblioMarc.t line 67.
    #          got: 'My title'
    #     expected: '  My title
    # '
    # Looks like you failed 1 test of 3.

#   Failed test 'StripWhitespaceChars tests'
#   at t/db_dependent/Biblio/ModBiblioMarc.t line 75.
# Looks like you failed 1 test of 2.
t/db_dependent/Biblio/ModBiblioMarc.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/2 subtests
Comment 7 Aleisha Amohia 2022-04-07 06:04:08 UTC
Created attachment 133055 [details] [review]
Bug 30358: (follow-up) Inner newlines should be replaced with a space
Comment 8 Aleisha Amohia 2022-04-07 06:04:12 UTC
Created attachment 133056 [details] [review]
Bug 30358: (follow-up) Fixing tests and including for inner newlines
Comment 9 Aleisha Amohia 2022-04-11 03:11:35 UTC
Created attachment 133140 [details] [review]
Bug 30358: (follow-up) Fixing tests and including for inner newlines
Comment 10 David Cook 2022-04-11 05:27:32 UTC
(In reply to Aleisha Amohia from comment #4)
> text area fields in MARC records are not designed to
> carry formatting, therefore it is implied that the data would be saved as a
> plain text string with no added formatting.

As far as I know, there is no prohibition against using carriage returns and line feeds in MARC records. The MARCXML schema seems to indicate that all whitespace is allowed: https://www.loc.gov/standards/marcxml/schema/MARC21slim.xsd. 

That said, I haven't ever seen an example that includes them either. 

> For users who follow these cataloguing standards, there should be a setting
> to ask Koha to strip these.
> 
> Planning to amend StripWhitespaceChars to also strip inner line breaks and
> carriage returns.

It would probably be a good idea to indicate that it's data fields and not the leader and control fields that are affected. 

--

I'd also suggest adding an additional option for inner newlines. While I can see the utility of trimming whitespace off the beginning and end, I imagine that many librarians would be frustrated by inner whitespace being trimmed (at least if there wasn't a clear warning about it).
Comment 11 Katrin Fischer 2022-04-11 06:10:11 UTC
As this is going to be tied to a new pref StripWhitespaceChars I think it will be ok. We had the same discussion I feel over adding HTML tags to records before. Ok, maybe that was a bit more extreme. I feel why some libraries won't want this, others will be happy as it helps when you copy&paste a lot of content from other sources.
Comment 12 David Cook 2022-04-11 07:09:38 UTC
(In reply to Katrin Fischer from comment #11)
> As this is going to be tied to a new pref StripWhitespaceChars I think it
> will be ok. We had the same discussion I feel over adding HTML tags to
> records before. Ok, maybe that was a bit more extreme. I feel why some
> libraries won't want this, others will be happy as it helps when you
> copy&paste a lot of content from other sources.

Yeah, the system preference default looks fine. I think adding clear description in the system preference is a good idea though. The current wording is misleading.
Comment 13 Aleisha Amohia 2022-04-12 22:53:21 UTC
Created attachment 133224 [details] [review]
Bug 30358: (follow-up) Fixing tests and including for inner newlines
Comment 14 Aleisha Amohia 2022-04-12 22:53:25 UTC
Created attachment 133225 [details] [review]
Bug 30358: (follow-up) Clarify syspref wording about fields affected
Comment 15 Aleisha Amohia 2022-04-27 22:56:44 UTC
Created attachment 134195 [details] [review]
Bug 30358: (follow-up) Consider field has multiple subfields of same key

To test:

1) Click the clone subfield button to make multiple subfields with the
same key, i.e. 500$a$a$a
2) Save the record and confirm that the fields contain the correct data
after whitespaces are stripped.
Comment 16 Aleisha Amohia 2022-05-02 04:58:05 UTC
Created attachment 134453 [details] [review]
Bug 30358: (follow-up) Put multiple subfields fix on auth side
Comment 17 David Nind 2022-07-09 22:29:13 UTC
Created attachment 137451 [details] [review]
Bug 30358: Strip leading/trailing whitespace characters from input fields when cataloguing

This enhancement adds a system preference StripWhitespaceChars which,
when enabled, will strip leading and trailing whitespace characters from
all fields when cataloguing both bibliographic records and authority
records. Whitespace characters that will be stripped are:
- spaces
- newlines
- carriage returns
- tabs

To test:
1. Apply patch and install database updates
2. Go to Administration, system preferences, find the new
StripWhitespaceChars preference. It should be "Don't strip" by default.
Change it to "Strip".
3. Search for a biblio record and edit it. Put some leading or trailing
whitespace characters in input fields and textarea fields and save.
4. Confirm these characters are removed when you save the record.
5. Repeat steps 3 and 4 for authority records.
6. Confirm tests pass t/db_dependent/Biblio/ModBiblioMarc.t

Sponsored-by: Educational Services Australia SCIS

Signed-off-by: David Nind <david@davidnind.com>
Comment 18 David Nind 2022-07-09 22:29:18 UTC
Created attachment 137452 [details] [review]
Bug 30358: (follow-up) Also strip inner newlines

This patch amends the StripWhitespaceChars system preference to also
strip inner newlines (line breaks and carriage returns) when enabled.

Signed-off-by: David Nind <david@davidnind.com>
Comment 19 David Nind 2022-07-09 22:29:23 UTC
Created attachment 137453 [details] [review]
Bug 30358: (follow-up) Inner newlines should be replaced with a space

Signed-off-by: David Nind <david@davidnind.com>
Comment 20 David Nind 2022-07-09 22:29:27 UTC
Created attachment 137454 [details] [review]
Bug 30358: (follow-up) Fixing tests and including for inner newlines

Signed-off-by: David Nind <david@davidnind.com>
Comment 21 David Nind 2022-07-09 22:29:31 UTC
Created attachment 137455 [details] [review]
Bug 30358: (follow-up) Clarify syspref wording about fields affected

Signed-off-by: David Nind <david@davidnind.com>
Comment 22 David Nind 2022-07-09 22:29:36 UTC
Created attachment 137456 [details] [review]
Bug 30358: (follow-up) Consider field has multiple subfields of same key

To test:

1) Click the clone subfield button to make multiple subfields with the
same key, i.e. 500$a$a$a
2) Save the record and confirm that the fields contain the correct data
after whitespaces are stripped.

Signed-off-by: David Nind <david@davidnind.com>
Comment 23 David Nind 2022-07-09 22:29:40 UTC
Created attachment 137457 [details] [review]
Bug 30358: (follow-up) Put multiple subfields fix on auth side

Signed-off-by: David Nind <david@davidnind.com>
Comment 24 Fridolin Somers 2022-08-13 07:54:49 UTC
Looking for QA.

Code added in C4/AuthoritiesMarc.pm and C4/Biblio.pm is exactly the same.
Please move to a common method, maybe in Koha::Util::MARC.
And add a unit test on this method.
It will make a solid code ;)
Comment 25 Aleisha Amohia 2022-08-15 04:50:36 UTC
Created attachment 139110 [details] [review]
Bug 30358: (follow-up) stripWhitespaceChars subroutine and tests

To test:

Confirm test plan above still works as expected and tests pass in
t/Koha_MetadataRecord.t

Sponsored-by: Catalyst IT
Comment 26 David Nind 2022-10-28 20:58:46 UTC
Patch no longer applies...

git bz apply 30358

Bug 30358 - Strip leading/trailing whitespace characters from input fields when cataloguing

137451 - Bug 30358: Strip leading/trailing whitespace characters from input fields when cataloguing
137452 - Bug 30358: (follow-up) Also strip inner newlines
137453 - Bug 30358: (follow-up) Inner newlines should be replaced with a space
137454 - Bug 30358: (follow-up) Fixing tests and including for inner newlines
137455 - Bug 30358: (follow-up) Clarify syspref wording about fields affected
137456 - Bug 30358: (follow-up) Consider field has multiple subfields of same key
137457 - Bug 30358: (follow-up) Put multiple subfields fix on auth side
139110 - Bug 30358: (follow-up) stripWhitespaceChars subroutine and tests

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 30358: Strip leading/trailing whitespace characters from input fields when cataloguing
.git/rebase-apply/patch:16: trailing whitespace.
    $record->add_fields($auth_type_tag,'','', $auth_type_subfield=>$authtypecode); 
warning: 1 line adds whitespace errors.
Using index info to reconstruct a base tree...
M	C4/AuthoritiesMarc.pm
M	C4/Biblio.pm
M	installer/data/mysql/mandatory/sysprefs.sql
M	koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/cataloguing.pref
M	t/db_dependent/Biblio/ModBiblioMarc.t
.git/rebase-apply/patch:16: trailing whitespace.
    $record->add_fields($auth_type_tag,'','', $auth_type_subfield=>$authtypecode); 
warning: 1 line adds whitespace errors.
Falling back to patching base and 3-way merge...
Auto-merging t/db_dependent/Biblio/ModBiblioMarc.t
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/cataloguing.pref
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/cataloguing.pref
Auto-merging installer/data/mysql/mandatory/sysprefs.sql
Auto-merging C4/Biblio.pm
Auto-merging C4/AuthoritiesMarc.pm
error: Failed to merge in the changes.
Patch failed at 0001 Bug 30358: Strip leading/trailing whitespace characters from input fields when cataloguing
Comment 27 Aleisha Amohia 2022-11-02 02:08:33 UTC Comment hidden (obsolete)
Comment 28 Aleisha Amohia 2022-11-02 02:08:38 UTC Comment hidden (obsolete)
Comment 29 Aleisha Amohia 2022-11-02 02:08:44 UTC Comment hidden (obsolete)
Comment 30 Aleisha Amohia 2022-11-02 02:08:49 UTC Comment hidden (obsolete)
Comment 31 Aleisha Amohia 2022-11-02 02:08:55 UTC Comment hidden (obsolete)
Comment 32 Aleisha Amohia 2022-11-02 02:09:02 UTC Comment hidden (obsolete)
Comment 33 Aleisha Amohia 2022-11-02 02:09:07 UTC Comment hidden (obsolete)
Comment 34 Aleisha Amohia 2022-11-02 02:09:14 UTC Comment hidden (obsolete)
Comment 35 Fridolin Somers 2022-11-02 07:33:01 UTC
Note that line feed in title or author breaks items search because of Ajax via JSON
Comment 36 David Nind 2022-11-02 13:36:56 UTC
Everything works as per the test plan, except for tests failing for t/db_dependent/Biblio/ModBiblioMarc.t
Comment 37 Aleisha Amohia 2022-11-03 01:13:02 UTC Comment hidden (obsolete)
Comment 38 David Nind 2022-11-03 06:34:31 UTC
Created attachment 143057 [details] [review]
Bug 30358: Strip leading/trailing whitespace characters from input fields when cataloguing

This enhancement adds a system preference StripWhitespaceChars which,
when enabled, will strip leading and trailing whitespace characters from
all fields when cataloguing both bibliographic records and authority
records. Whitespace characters that will be stripped are:
- spaces
- newlines
- carriage returns
- tabs

To test:
1. Apply patch and install database updates
2. Go to Administration, system preferences, find the new
StripWhitespaceChars preference. It should be "Don't strip" by default.
Change it to "Strip".
3. Search for a biblio record and edit it. Put some leading or trailing
whitespace characters in input fields and textarea fields and save.
4. Confirm these characters are removed when you save the record.
5. Repeat steps 3 and 4 for authority records.
6. Confirm tests pass t/db_dependent/Biblio/ModBiblioMarc.t

Sponsored-by: Educational Services Australia SCIS

Signed-off-by: David Nind <david@davidnind.com>
Comment 39 David Nind 2022-11-03 06:34:37 UTC
Created attachment 143058 [details] [review]
Bug 30358: (follow-up) Also strip inner newlines

This patch amends the StripWhitespaceChars system preference to also
strip inner newlines (line breaks and carriage returns) when enabled.

Signed-off-by: David Nind <david@davidnind.com>
Comment 40 David Nind 2022-11-03 06:34:42 UTC
Created attachment 143059 [details] [review]
Bug 30358: (follow-up) Inner newlines should be replaced with a space

Signed-off-by: David Nind <david@davidnind.com>
Comment 41 David Nind 2022-11-03 06:34:47 UTC
Created attachment 143060 [details] [review]
Bug 30358: (follow-up) Fixing tests and including for inner newlines

Signed-off-by: David Nind <david@davidnind.com>
Comment 42 David Nind 2022-11-03 06:34:52 UTC
Created attachment 143061 [details] [review]
Bug 30358: (follow-up) Clarify syspref wording about fields affected

Signed-off-by: David Nind <david@davidnind.com>
Comment 43 David Nind 2022-11-03 06:34:58 UTC
Created attachment 143062 [details] [review]
Bug 30358: (follow-up) Consider field has multiple subfields of same key

To test:

1) Click the clone subfield button to make multiple subfields with the
same key, i.e. 500$a$a$a
2) Save the record and confirm that the fields contain the correct data
after whitespaces are stripped.

Signed-off-by: David Nind <david@davidnind.com>
Comment 44 David Nind 2022-11-03 06:35:04 UTC
Created attachment 143063 [details] [review]
Bug 30358: (follow-up) Put multiple subfields fix on auth side

Signed-off-by: David Nind <david@davidnind.com>
Comment 45 David Nind 2022-11-03 06:35:09 UTC
Created attachment 143064 [details] [review]
Bug 30358: (follow-up) stripWhitespaceChars subroutine and tests

To test:

Confirm test plan above still works as expected and tests pass in
t/Koha_MetadataRecord.t

Sponsored-by: Catalyst IT

Signed-off-by: David Nind <david@davidnind.com>
Comment 46 David Nind 2022-11-03 06:35:15 UTC
Created attachment 143065 [details] [review]
Bug 30358: (follow-up) Fixing ModBiblioMarc.t tests

Signed-off-by: David Nind <david@davidnind.com>
Comment 47 Marcel de Rooy 2022-11-03 12:54:04 UTC
Wondering if this is a bit too aggressively cleaning up?
E.g. field 008 (and maybe more control fields) could end with a few spaces. You will remove them, which is not correct.
Comment 48 Marcel de Rooy 2022-11-03 12:59:11 UTC
About the regex:

$value =~ s/^\s+|\s+$|^\n+|\n+$|^\r+|\r+$|^\t+|\t+$//g;

Is the following not the same:

$value =~ s/^\s+|\s+$//g;

?
Comment 49 Aleisha Amohia 2022-11-21 03:25:20 UTC
(In reply to Marcel de Rooy from comment #47)
> Wondering if this is a bit too aggressively cleaning up?
> E.g. field 008 (and maybe more control fields) could end with a few spaces.
> You will remove them, which is not correct.

Should I write a follow-up to exclude fields < 010?
Comment 50 Marcel de Rooy 2022-11-21 07:49:13 UTC
(In reply to Aleisha Amohia from comment #49)
> (In reply to Marcel de Rooy from comment #47)
> > Wondering if this is a bit too aggressively cleaning up?
> > E.g. field 008 (and maybe more control fields) could end with a few spaces.
> > You will remove them, which is not correct.
> 
> Should I write a follow-up to exclude fields < 010?

Sounds reasonable to me.
Comment 51 Aleisha Amohia 2022-11-21 21:39:43 UTC
Created attachment 144120 [details] [review]
Bug 30358: Strip leading/trailing whitespace characters from input fields when cataloguing

This enhancement adds a system preference StripWhitespaceChars which,
when enabled, will strip leading and trailing whitespace characters from
all fields when cataloguing both bibliographic records and authority
records. Whitespace characters that will be stripped are:
- spaces
- newlines
- carriage returns
- tabs

To test:
1. Apply patch and install database updates
2. Go to Administration, system preferences, find the new
StripWhitespaceChars preference. It should be "Don't strip" by default.
Change it to "Strip".
3. Search for a biblio record and edit it. Put some leading or trailing
whitespace characters in input fields and textarea fields and save.
4. Confirm these characters are removed when you save the record.
5. Repeat steps 3 and 4 for authority records.
6. Confirm tests pass t/db_dependent/Biblio/ModBiblioMarc.t

Sponsored-by: Educational Services Australia SCIS

Signed-off-by: David Nind <david@davidnind.com>
Comment 52 Aleisha Amohia 2022-11-21 21:39:51 UTC
Created attachment 144121 [details] [review]
Bug 30358: (follow-up) Also strip inner newlines

This patch amends the StripWhitespaceChars system preference to also
strip inner newlines (line breaks and carriage returns) when enabled.

Signed-off-by: David Nind <david@davidnind.com>
Comment 53 Aleisha Amohia 2022-11-21 21:39:58 UTC
Created attachment 144122 [details] [review]
Bug 30358: (follow-up) Inner newlines should be replaced with a space

Signed-off-by: David Nind <david@davidnind.com>
Comment 54 Aleisha Amohia 2022-11-21 21:40:06 UTC
Created attachment 144123 [details] [review]
Bug 30358: (follow-up) Fixing tests and including for inner newlines

Signed-off-by: David Nind <david@davidnind.com>
Comment 55 Aleisha Amohia 2022-11-21 21:40:13 UTC
Created attachment 144124 [details] [review]
Bug 30358: (follow-up) Clarify syspref wording about fields affected

Signed-off-by: David Nind <david@davidnind.com>
Comment 56 Aleisha Amohia 2022-11-21 21:40:21 UTC
Created attachment 144125 [details] [review]
Bug 30358: (follow-up) Consider field has multiple subfields of same key

To test:

1) Click the clone subfield button to make multiple subfields with the
same key, i.e. 500$a$a$a
2) Save the record and confirm that the fields contain the correct data
after whitespaces are stripped.

Signed-off-by: David Nind <david@davidnind.com>
Comment 57 Aleisha Amohia 2022-11-21 21:40:29 UTC
Created attachment 144126 [details] [review]
Bug 30358: (follow-up) Put multiple subfields fix on auth side

Signed-off-by: David Nind <david@davidnind.com>
Comment 58 Aleisha Amohia 2022-11-21 21:40:36 UTC
Created attachment 144127 [details] [review]
Bug 30358: (follow-up) stripWhitespaceChars subroutine and tests

To test:

Confirm test plan above still works as expected and tests pass in
t/Koha_MetadataRecord.t

Sponsored-by: Catalyst IT

Signed-off-by: David Nind <david@davidnind.com>
Comment 59 Aleisha Amohia 2022-11-21 21:40:43 UTC
Created attachment 144128 [details] [review]
Bug 30358: (follow-up) Fixing ModBiblioMarc.t tests

Signed-off-by: David Nind <david@davidnind.com>
Comment 60 Aleisha Amohia 2022-11-21 21:40:51 UTC
Created attachment 144129 [details] [review]
Bug 30358 (follow-up): Do not strip whitespace from control fields
Comment 61 Aleisha Amohia 2022-11-21 21:40:59 UTC
Created attachment 144130 [details] [review]
Bug 30358 (follow-up) Simplify regex

The regex does the following:
1. Replace newlines and carriage returns with a space
2. Replace leading and trailing whitespace with nothing (strip)
Comment 62 Hammat wele 2022-11-22 17:31:17 UTC
Created attachment 144171 [details] [review]
Bug 30358 (follow-up) Simplify regex

The regex does the following:
1. Replace newlines and carriage returns with a space
2. Replace leading and trailing whitespace with nothing (strip)

Signed-off-by: Hammat Wele <hammat.wele@inlibro.com>