|
Description
Nick Clemens (kidclamp)
2022-10-07 18:59:40 UTC
Created attachment 141520 [details] [review] Bug 31716: Enlagre the authorised_value column in authorised_values To test: 1 - Apply patch 2 - Update database 3 - Confirm you can store a longer value in authorised_values Created attachment 141521 [details] [review] Bug 31716: DO NOT PUSH - Schema updates I can now update an authorized value code using the database, but I can't add or edit using the staff interface as the form entry's maximum length is 80: <input type="text" id="authorised_value" name="authorised_value" value="" maxlength="80" class="focus" />. When you do try to edit an entry, you get this message on the edit form "Please enter no more than 80 characters." To update a value in the database: 1. koha-mysql kohadev 2. After applying the patch (I also ran dbic - not sure whether that is required) you can see that the change to the database structure has been enabled: show columns from authorised_values; 3. SQL to update an entry - where XXXX = the id of an existing entry: update authorised_values set authorised_value="This is a really really long authorized value although I am not sure why you would want such a code" where id = XXXX; ALTER TABLE authorised_values MODIFY COLUMN authorised_value varchar(255) You go from 80 to 255. At some point that is too low again. We should imo go to a TEXT type: A TEXT column with a maximum length of 65,535 (216 - 1) characters. One point: VARCHAR columns can be fully indexed. TEXT columns can only be indexed over a specified length. Created attachment 141722 [details] [review] Bug 31716: Enlarge the authorised_value column in authorised_values This patch changes authorised_value from VARCHAR(80) to VARCHAR(255) To test: 1 - Apply patch 2 - Update database 3 - Confirm you can store a longer value in authorised_values Created attachment 141723 [details] [review] Bug 31716: Prevent issues with foreign key VARCHAR is now allowed to be 65535 bytes max, as opposed to 255 - when testing this, however, I received errors about latering a column with a foreign key. Adding this to the update to avoid problems like bug 31673. Created attachment 141724 [details] [review] Bug 31716: DO NOT PUSH - Schema updates (In reply to David Nind from comment #3) > I can now update an authorized value code using the database, but I can't > add or edit using the staff interface as the form entry's maximum length is > 80: <input type="text" id="authorised_value" name="authorised_value" > value="" maxlength="80" class="focus" />. > Fixed (In reply to Marcel de Rooy from comment #4) > You go from 80 to 255. At some point that is too low again. We should imo go > to a TEXT type: A TEXT column with a maximum length of 65,535 (216 - 1) > characters. > One point: VARCHAR columns can be fully indexed. TEXT columns can only be > indexed over a specified length. Some versions of MySQL/Maria allow varchar up to 65535 - so we can increase in future as needed - this was the largest push without more work and sufficient for current needs (In reply to Nick Clemens from comment #8) > (In reply to Marcel de Rooy from comment #4) > > You go from 80 to 255. At some point that is too low again. We should imo go > > to a TEXT type: A TEXT column with a maximum length of 65,535 (216 - 1) > > characters. > > One point: VARCHAR columns can be fully indexed. TEXT columns can only be > > indexed over a specified length. > > Some versions of MySQL/Maria allow varchar up to 65535 - so we can increase > in future as needed - this was the largest push without more work and > sufficient for current needs URLs could be up to 2048 chars? Note that addressing the authorized values table is only half of the work. The receiving column should be able to store it too. But you know :) (In reply to Marcel de Rooy from comment #9) > (In reply to Nick Clemens from comment #8) > > (In reply to Marcel de Rooy from comment #4) > URLs could be up to 2048 chars? Note that addressing the authorized values > table is only half of the work. The receiving column should be able to store > it too. But you know :) Indeed, flexibility comes with responsibility sometimes :-) Created attachment 141904 [details] [review] Bug 31716: Enlarge the authorised_value column in authorised_values This patch changes authorised_value from VARCHAR(80) to VARCHAR(255) To test: 1 - Apply patch 2 - Update database 3 - Confirm you can store a longer value in authorised_values Signed-off-by: David Nind <david@davidnind.com> Created attachment 141905 [details] [review] Bug 31716: Prevent issues with foreign key VARCHAR is now allowed to be 65535 bytes max, as opposed to 255 - when testing this, however, I received errors about latering a column with a foreign key. Adding this to the update to avoid problems like bug 31673. Signed-off-by: David Nind <david@davidnind.com> Created attachment 141906 [details] [review] Bug 31716: DO NOT PUSH - Schema updates Signed-off-by: David Nind <david@davidnind.com> And you make them mismatch again. All columns must be modified IMO (and be FK ideally). (In reply to Jonathan Druart from comment #14) > And you make them mismatch again. All columns must be modified IMO (and be > FK ideally). What is mismatching? Can you provide some detail (In reply to Nick Clemens from comment #15) > (In reply to Jonathan Druart from comment #14) > > And you make them mismatch again. All columns must be modified IMO (and be > > FK ideally). > > What is mismatching? Can you provide some detail Actually this concerns me too. When you allow to have longer values for the code, we will have the situation, that people are likely to create longer descriptions that won't in most places. Think: items.ccode, items.location, asort1/asort2 in aqorders etc. May I make a different suggestion? If these are for cataloguing, maybe we could make it an option in the frameworks so that not the code but the description will be stored. That might prove nice in other situations as well. longer descriptions should be 'longer codes'! I don't mind the long descriptions... I feel it would be worth to learn more about the use case here. Resolved using hardcoded JS, not going to keep working on this one (In reply to Nick Clemens from comment #19) > Resolved using hardcoded JS, not going to keep working on this one |