| 
 
        
          Description
        
        
          Peter Vashchuk
        
        
        
        
          2023-01-31 11:45:26 UTC
        
       
    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 I tested this in sandbox and it does what it claims to do: trims whitespaces from the beginning and the end of the written text when patron is saved. But if you write for example "Little lamb" it will not trim those extra spaces between the words. The patron brief info on the left trims them when showing the attribute but spaces are there when you modify patron. Should we trim extra spaces also from between words? We should probably create a separate bug for that. I think what you propose might be beyond the scope of this fix. The purpose of this patch was to avoid unintentional duplicates caused by trailing whitespace that occasionally gets added by the barcode scanners that our librarians use. I tried to test this patch but it doesn't apply on main Created attachment 181006 [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> Rebased on the latest main Created attachment 181007 [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> Signed-off-by: Marion Durand <marion.durand@biblibre.com> This isn't tidy, can you check your tools? FAIL Koha/Patron/Attribute/Type.pm FAIL tidiness File is not tidy, please run `perl misc/devel/tidy.pl Koha/Patron/Attribute/Type.pm` OK Koha/Schema/Result/BorrowerAttributeType.pm FAIL admin/patron-attr-types.pl FAIL tidiness File is not tidy, please run `perl misc/devel/tidy.pl admin/patron-attr-types.pl` FAIL installer/data/mysql/atomicupdate/bug_32767-trim-attr-feature.pl FAIL tidiness File is not tidy, please run `perl misc/devel/tidy.pl installer/data/mysql/atomicupdate/bug_32767-trim-attr-feature.pl` OK installer/data/mysql/kohastructure.sql FAIL koha-tmpl/intranet-tmpl/prog/en/modules/admin/patron-attr-types.tt FAIL tidiness File is not tidy, please run `perl misc/devel/tidy.pl koha-tmpl/intranet-tmpl/prog/en/modules/admin/patron-attr-types.tt` OK members/memberentry.pl OK opac/opac-memberentry.pl FAIL t/db_dependent/Koha/Patron/Attribute.t FAIL tidiness File is not tidy, please run `perl misc/devel/tidy.pl t/db_dependent/Koha/Patron/Attribute.t` Unit tests also seem to be failing wit this applied. Also, not a blocker, but it would be nice if the unit tests and DB updates were their own commits. Created attachment 181589 [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. Created attachment 181590 [details] [review] Bug 32767: Add tests for the trims attributes feature  |