Created attachment 145846 [details] [review] Bug 32767: New feature that trims attributes (if enabled) to avoid duplicates Added a new "Trim identifier:" checkbox for attribute editor page, that if enabled, will trim the value from any trailing whitespaces. The use case for this feature is to avoid unintentionally creating duplicate attribute values with trailing whitespaces around of it. If this checkbox is grayed for you, that means that you have some values surrounded with whitespaces, and Koha can't decide for you which you want to remove and when (because that as well will lead to creating duplicated entries). So in order to use this feature you first need to clean up your database from already existing values with trailing whitespaces. This SQL query might help you find all untrimmed attributes & borrowers: ```SELECT * FROM borrower_attributes WHERE code=%Attribute Name% AND attribute REGEXP "^\\s+|\\s+$";``` Steps to reproduce: 1. Go to the attribute editor page. Check if the "Trim identifier:" checkbox is not grayed. (if it is grayed, it means you have some values surrounded with whitespaces, in that case you need to fix them manually) 2. Check the checkbox and save settings. 3. Add that attribute for a preferred patron, enter some value with a trailing whitespaces around of it. 4. Check that the trailing whitespaces were trimmed correctly.
Please provide tests.
Created attachment 151540 [details] [review] Bug 32767: New feature that trims attributes (if enabled) to avoid duplicates Added a new "Trim identifier:" checkbox for attribute editor page, that if enabled, will trim the value from any trailing whitespaces. The use case for this feature is to avoid unintentionally creating duplicate attribute values with trailing whitespaces around of it. If this checkbox is grayed for you, that means that you have some values surrounded with whitespaces, and Koha can't decide for you which you want to remove and when (because that as well will lead to creating duplicated entries). So in order to use this feature you first need to clean up your database from already existing values with trailing whitespaces. This SQL query might help you find all untrimmed attributes & borrowers: ```SELECT * FROM borrower_attributes WHERE code=%Attribute Name% AND attribute REGEXP "^\\s+|\\s+$";``` Steps to reproduce: 1. Go to the attribute editor page. Check if the "Trim identifier:" checkbox is not grayed. (if it is grayed, it means you have some values surrounded with whitespaces, in that case you need to fix them manually) 2. Check the checkbox and save settings. 3. Add that attribute for a preferred patron, enter some value with a trailing whitespaces around of it. 4. Check that the trailing whitespaces were trimmed correctly. Signed-off-by: Sam Lau <samalau@gmail.com>
I am not sure if this needs to be optional. I can't come up with a use case for whitespace before or after any entered values. If this is not a problem for maybe multi-line entries, why not make it the default behaviour? Or maybe default for anything marked unique at least?
As far as I remember this feature is optional to prevent any complications it will cause for libraries that already have duplicate attributes with spaces that are supposed to be unique, that's why it's disabled if your database contains any untrimmed unique attributes.
Created attachment 153651 [details] [review] Bug 32767: New feature that trims attributes (if enabled) to avoid duplicates Added a new "Trim identifier:" checkbox for attribute editor page, that if enabled, will trim the value from any trailing whitespaces. The use case for this feature is to avoid unintentionally creating duplicate attribute values with trailing whitespaces around of it. If this checkbox is grayed for you, that means that you have some values surrounded with whitespaces, and Koha can't decide for you which you want to remove and when (because that as well will lead to creating duplicated entries). So in order to use this feature you first need to clean up your database from already existing values with trailing whitespaces. This SQL query might help you find all untrimmed attributes & borrowers: ```SELECT * FROM borrower_attributes WHERE code=%Attribute Name% AND attribute REGEXP "^\\s+|\\s+$";``` Steps to reproduce: 1. Go to the attribute editor page. Check if the "Trim identifier:" checkbox is not grayed. (if it is grayed, it means you have some values surrounded with whitespaces, in that case you need to fix them manually) 2. Check the checkbox and save settings. 3. Add that attribute for a preferred patron, enter some value with a trailing whitespaces around of it. 4. Check that the trailing whitespaces were trimmed correctly. Signed-off-by: Sam Lau <samalau@gmail.com>
Added tests
Created attachment 153678 [details] [review] Bug 32767: New feature that trims attributes (if enabled) to avoid duplicates Added a new "Trim identifier:" checkbox for attribute editor page, that if enabled, will trim the value from any trailing whitespaces. The use case for this feature is to avoid unintentionally creating duplicate attribute values with trailing whitespaces around of it. If this checkbox is grayed for you, that means that you have some values surrounded with whitespaces, and Koha can't decide for you which you want to remove and when (because that as well will lead to creating duplicated entries). So in order to use this feature you first need to clean up your database from already existing values with trailing whitespaces. This SQL query might help you find all untrimmed attributes & borrowers: ```SELECT * FROM borrower_attributes WHERE code=%Attribute Name% AND attribute REGEXP "^\\s+|\\s+$";``` Steps to reproduce: 1. Go to the attribute editor page. Check if the "Trim identifier:" checkbox is not grayed. (if it is grayed, it means you have some values surrounded with whitespaces, in that case you need to fix them manually) 2. Check the checkbox and save settings. 3. Add that attribute for a preferred patron, enter some value with a trailing whitespaces around of it. 4. Check that the trailing whitespaces were trimmed correctly. Signed-off-by: Sam Lau <samalau@gmail.com>
Oops. Please run QA tools. (You are in QA?) Failing test and more trouble.
(In reply to Marcel de Rooy from comment #9) > Oops. Please run QA tools. (You are in QA?) > Failing test and more trouble. Tested on the latest master, QA tools are OK, and tests also seem fine: $> prove t/db_dependent/Koha/Patron/Attribute.t t/db_dependent/Koha/Patron/Attribute.t .. ok All tests successful. Files=1, Tests=3 Result: PASS can you please help me find out what the problem is?
(In reply to Peter Vashchuk from comment #10) > (In reply to Marcel de Rooy from comment #9) > > Oops. Please run QA tools. (You are in QA?) > > Failing test and more trouble. > > Tested on the latest master, QA tools are OK, and tests also seem fine: > > $> prove t/db_dependent/Koha/Patron/Attribute.t > t/db_dependent/Koha/Patron/Attribute.t .. ok > All tests successful. > Files=1, Tests=3 > Result: PASS > > can you please help me find out what the problem is? Sure. Did you merge latest changes on qa tools? I will copy my results in the next comment. Note that the test does pass. I probably did not run the atomicupdate. Sorry about that.. But there are still some issues raised.
Processing files before patches |========================>| 10 / 10 (100.00%) Processing files after patches |========================>| 10 / 10 (100.00%) WARN Koha/Patron/Attribute.pm OK critic OK forbidden patterns OK git manipulation OK pod OK pod coverage SKIP spelling WARN tidiness The file is less tidy than before (bad/messy lines before: 8, now: 10) OK valid WARN Koha/Patron/Attribute/Type.pm OK critic OK forbidden patterns OK git manipulation OK pod OK pod coverage SKIP spelling WARN tidiness The file is less tidy than before (bad/messy lines before: 6, now: 14) OK valid WARN Koha/Schema/Result/BorrowerAttributeType.pm OK critic OK forbidden patterns OK git manipulation OK pod OK pod coverage SKIP spelling WARN tidiness The file is less tidy than before (bad/messy lines before: 47, now: 49) OK valid WARN admin/patron-attr-types.pl OK critic OK forbidden patterns OK git manipulation OK pod SKIP spelling WARN tidiness The file is less tidy than before (bad/messy lines before: 62, now: 65) OK valid WARN installer/data/mysql/atomicupdate/bug_32767-trim-attr-feature.pl OK critic OK file permissions OK forbidden patterns OK git manipulation OK pod SKIP spelling WARN tidiness The file is less tidy than before (bad/messy lines before: 0, now: 4) OK valid FAIL installer/data/mysql/kohastructure.sql OK boolean_vs_tinyint OK charset_collate OK git manipulation FAIL tinyint_has_boolean_flag The new column borrower_attribute_types.trim_value is using TINYINT(1) as type but is not defined as boolean in the schema file (Koha/Schema/Result/BorrowerAttributeType.pm), see the SQL12 coding guideline OK koha-tmpl/intranet-tmpl/prog/en/modules/admin/patron-attr-types.tt OK filters OK forbidden patterns OK git manipulation OK js_in_body SKIP spelling OK tt_valid OK valid_template WARN members/memberentry.pl OK critic OK forbidden patterns OK git manipulation OK pod SKIP spelling WARN tidiness The file is less tidy than before (bad/messy lines before: 318, now: 319) OK valid OK opac/opac-memberentry.pl OK critic OK forbidden patterns OK git manipulation OK pod SKIP spelling OK tidiness OK valid WARN t/db_dependent/Koha/Patron/Attribute.t OK critic OK forbidden patterns OK git manipulation OK pod SKIP spelling WARN tidiness The file is less tidy than before (bad/messy lines before: 84, now: 104) OK valid Processing additional checks OK! Running tests (1) * Proving /usr/share/koha/t/db_dependent/Koha/Patron/Attribute.t OK!
Created attachment 156985 [details] [review] Bug 32767: New feature that trims attributes (if enabled) to avoid duplicates Added a new "Trim identifier:" checkbox for attribute editor page, that if enabled, will trim the value from any trailing whitespaces. The use case for this feature is to avoid unintentionally creating duplicate attribute values with trailing whitespaces around of it. If this checkbox is grayed for you, that means that you have some values surrounded with whitespaces, and Koha can't decide for you which you want to remove and when (because that as well will lead to creating duplicated entries). So in order to use this feature you first need to clean up your database from already existing values with trailing whitespaces. This SQL query might help you find all untrimmed attributes & borrowers: ```SELECT * FROM borrower_attributes WHERE code=%Attribute Name% AND attribute REGEXP "^\\s+|\\s+$";``` Steps to reproduce: 1. Go to the attribute editor page. Check if the "Trim identifier:" checkbox is not grayed. (if it is grayed, it means you have some values surrounded with whitespaces, in that case you need to fix them manually) 2. Check the checkbox and save settings. 3. Add that attribute for a preferred patron, enter some value with a trailing whitespaces around of it. 4. Check that the trailing whitespaces were trimmed correctly. Signed-off-by: Sam Lau <samalau@gmail.com>
Created attachment 159891 [details] [review] Bug 32767: New feature that trims attributes (if enabled) to avoid duplicates Added a new "Trim identifier:" checkbox for attribute editor page, that if enabled, will trim the value from any trailing whitespaces. The use case for this feature is to avoid unintentionally creating duplicate attribute values with trailing whitespaces around of it. If this checkbox is grayed for you, that means that you have some values surrounded with whitespaces, and Koha can't decide for you which you want to remove and when (because that as well will lead to creating duplicated entries). So in order to use this feature you first need to clean up your database from already existing values with trailing whitespaces. This SQL query might help you find all untrimmed attributes & borrowers: ```SELECT * FROM borrower_attributes WHERE code=%Attribute Name% AND attribute REGEXP "^\\s+|\\s+$";``` Steps to reproduce: 1. Go to the attribute editor page. Check if the "Trim identifier:" checkbox is not grayed. (if it is grayed, it means you have some values surrounded with whitespaces, in that case you need to fix them manually) 2. Check the checkbox and save settings. 3. Add that attribute for a preferred patron, enter some value with a trailing whitespaces around of it. 4. Check that the trailing whitespaces were trimmed correctly. Signed-off-by: Sam Lau <samalau@gmail.com> Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Please retest rebase after CSRF changes.
*** Bug 37445 has been marked as a duplicate of this bug. ***
Created attachment 170815 [details] [review] Bug 32767: New feature that trims attributes (if enabled) to avoid duplicates Added a new "Trim identifier:" checkbox for attribute editor page, that if enabled, will trim the value from any trailing whitespaces. The use case for this feature is to avoid unintentionally creating duplicate attribute values with trailing whitespaces around of it. If this checkbox is grayed for you, that means that you have some values surrounded with whitespaces, and Koha can't decide for you which you want to remove and when (because that as well will lead to creating duplicated entries). So in order to use this feature you first need to clean up your database from already existing values with trailing whitespaces. This SQL query might help you find all untrimmed attributes & borrowers: ```SELECT * FROM borrower_attributes WHERE code=%Attribute Name% AND attribute REGEXP "^\\s+|\\s+$";``` Steps to reproduce: 1. Go to the attribute editor page. Check if the "Trim identifier:" checkbox is not grayed. (if it is grayed, it means you have some values surrounded with whitespaces, in that case you need to fix them manually) 2. Check the checkbox and save settings. 3. Add that attribute for a preferred patron, enter some value with a trailing whitespaces around of it. 4. Check that the trailing whitespaces were trimmed correctly. Signed-off-by: Sam Lau <samalau@gmail.com> Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Tested and rebased on the latest master