Summary: | Select a department: empty field if no description set in authorized value | ||
---|---|---|---|
Product: | Koha | Reporter: | Mirko Tietgen <mirko> |
Component: | Course reserves | Assignee: | Bugs List <koha-bugs> |
Status: | CLOSED WONTFIX | QA Contact: | Testopia <testopia> |
Severity: | minor | ||
Priority: | P5 - low | CC: | jonathan.druart, mtompset, tmisilo, veron |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | Small patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Attachments: |
Proposed Patch
Add check to determine if d.lib exists Bug 13399: Missing validation on authorised value descriptions Bug 13399: Missing validation on authorised value descriptions Bug 13399: Missing validation on authorised value descriptions |
Description
Mirko Tietgen
2014-12-05 09:27:58 UTC
Created attachment 36242 [details] [review] Proposed Patch I don't have a system to test this on yet, however I believe this should do what you want. Sorry, patch does not apply: git bz apply 13399 36242 - Proposed Patch Applying: Add check to determine if d.lib exists fatal: corrupt patch at line 20 Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 Add check to determine if d.lib exists Created attachment 37532 [details] [review] Add check to determine if d.lib exists Add a check to see if d.lib exists, if it does display that value. Otherwise display the authorised value Rescued Tom's patch. It looks good. Thanks for fixing the patch! This does not work, it's not filled with the value everywhere it is displayed. It should be fixed on a higher level, for all authorised values I think (C4::Koha::GetAuthorisedValues and some others). Or maybe the best way would be to make the description mandatory on the AV administration page. (In reply to Jonathan Druart from comment #6) > Or maybe the best way would be to make the description mandatory on the AV > administration page. I think this is a better solution, yes. Created attachment 37783 [details] [review] Bug 13399: Missing validation on authorised value descriptions This adds validations which force both the OPAC and Staff Client authorised value descriptions to be filled in. Counter Patch in the works. Created attachment 37784 [details] [review] Bug 13399: Missing validation on authorised value descriptions This adds validations which force both the OPAC and Staff Client authorized value descriptions to be filled in. TEST PLAN --------- 1) Login to staff client 2) Koha administration -> Authorized values 3) Change drop-down to 'DEPARTMENT' -- Assuming you have default values installed, and DEPARTMENT exists. 4) Click '+ New authorized value for DEPARTMENT' 5) Enter a 'Authorized value:' and click 'Save' -- This will create it without OPAC and Staff Client descriptions! This is wrong. 6) Apply patch 7) $ ./installer/data/mysql/updatedatabase.pl -- expecting atomic update to run. 8) Refresh staff client page -- the descriptions should now be filled in. 9) Repeat steps 4 and 5 -- This time, you will be given two error messages -- Nothing will be created. 10) Attempt to create a new authorized value with descriptions. -- should save without issue. 11) Run koha qa test tools -- SQL files are unchecked, but steps 7 & 8 confirm validity. -- Should be no issues. Created attachment 37785 [details] [review] Bug 13399: Missing validation on authorised value descriptions This adds validations which force both the OPAC and Staff Client authorized value descriptions to be filled in. TEST PLAN --------- 1) Login to staff client 2) Koha administration -> Authorized values 3) Change drop-down to 'DEPARTMENT' -- Assuming you have default values installed, and DEPARTMENT exists. 4) Click '+ New authorized value for DEPARTMENT' 5) Enter a 'Authorized value:' and click 'Save' -- This will create it without OPAC and Staff Client descriptions! This is wrong. 6) Apply patch 7) $ ./installer/data/mysql/updatedatabase.pl -- expecting atomic update to run. 8) Refresh staff client page -- the descriptions should now be filled in. 9) Repeat steps 4 and 5 -- This time, you will be given two error messages -- Nothing will be created. 10) Attempt to create a new authorized value with descriptions. -- should save without issue. 11) Run koha qa test tools -- SQL files are unchecked, but steps 7 & 8 confirm validity. -- Should be no issues. I don't think this is the correct solution. There are reasons why it makes sometimes sense to leave one or both descriptions empty. In the past Koha even came with 0 values that had empty descriptions to offer an empty entry in some pull down lists. Also, if you leave the OPAC description empty it will fall back to the library description as a feature. And is this really a bug? What if a library just doesn't want to display anything in department and term? (In reply to Katrin Fischer from comment #12) > And is this really a bug? What if a library just doesn't want to display > anything in department and term? Then they wouldn't have entered any authorized values, right? Blanks in dropdowns shouldn't be forced data-entries. They should be added to the dropdowns. (In reply to Katrin Fischer from comment #11) > In the past Koha > even came with 0 values that had empty descriptions to offer an empty entry > in some pull down lists. Can you provide a suggestion where to look in the user interface for these types of drop-downs that may require blanks? Mark, Term and Department are mandatory in the course reserves module - the library is forced to enter an authorized value. Asked the other way around: what negative side effect does it have to not enforce entering a subscription? 7 years later, I still believe this not to be a bug. Please reopen to restart discussion if you disagree. |