|
Description
Martin Persson
2015-05-21 16:00:58 UTC
Created attachment 39362 [details] [review] DB update script and sysprefs file Should merge with f52084df0e395fdf89d80ca2fd77844273a8cf7c Comment on attachment 39362 [details] [review] DB update script and sysprefs file Missing key functionality. Needs ability to switch on/off display for OPAC/Staff separately. Created attachment 39414 [details] [review] Adds preferences for news authorship display Depends on bug 14246. Created attachment 39418 [details]
SQL command file to remove news author preferences
To remove the changes from the database:
mysql -u kohaadmin -p -D koha < remove_news_prefs.sql
Test plan: * Apply dependant patch 14246 (if not done already). * Apply current patch. * Execute the database update file (requires PERL5LIB and KOHA_CONF to be set up): $ perl perl installer/data/mysql/atomicupdate/add_news_prefs.pl * Log into the Staff client, go to 'Administration' -> 'Global Preferences' -> 'Tools', a new section titled 'News' should appear with a single option named 'NewsAuthorDisplay'. Available values should be 'Not at all', 'OPAC only', 'Staff client only' and 'Both OPAC and staff client'. By default the value should be 'Not at all' (to mimic existing behaviour, important!). * Change the value to something different and save the changes. * Use the mysql command line client to connect to the Koha database: $ mysql -u kohaadmin -p -D koha * Enter the following query: SELECT * FROM systempreferences WHERE variable = 'NewsAuthorDisplay'; * Check to make sure that the 'value' column has changed to reflect the choice you made in the above step. Created attachment 39440 [details] [review] [Signed-off] Bug 14247: Add system preference for news display This patch adds a new system preference named 'NewsAuthorDisplay' under the 'Tools' category. This allows global control of whether the name and title of the user authoring a news posting is shown in the OPAC, staff interface, both or none. Depends on patch for bug 14246. Sponsored-by: Halland County Library Preference appears and behaves as expected. Signed-off-by: Marc Véron <veron@veron.ch> Martin,
Actually, the atomicupdate file should have been created like:
== installer/data/mysql/atomicupdate/add_news_prefs.sql
-- your comment
INSERT IGNORE INTO systempreferences (variable,type,options,value,explanation) VALUES('NewsAuthorDisplay','Choice','none|opac|staff|both','none','Display the author name for news items.');
And that's all.
On this way, it will be executed by the updatedatabase.pl script.
ATM it does not deal with pl scripts.
This is not blocker, the RM will manage to deal with it.
However you have to add the new syspref in the sysprefs file (installer/data/mysql/sysprefs.sql).
Marked as Failed QA.
Created attachment 39488 [details] [review] Updated version of patch, integrates syspref SQL, atomic update is pure SQL and test plan is in commit message. Thanks for the informative comments. The order of the fields used for insertion in the atomic update has been changed to match the order in the main sysprefs.sql insert statement. The clean install was tested against a new database that had only had the kohastructure.sql applied. No errors and data looked correct. cait @ IRC informed me that in situations with minor changes, I can simply add another patch on top of the existing one rather than obsoleting it. That might have been more appropiate here. Pardon any inconvenience. Created attachment 39527 [details] [review] Optional patch to rename the atomic update to match guidelines To be pedantic I added an (optional) patch to rename the atomic update SQL file to the proper bug_X-prefix. Comment on attachment 39488 [details] [review] Updated version of patch, integrates syspref SQL, atomic update is pure SQL and test plan is in commit message. Review of attachment 39488 [details] [review]: ----------------------------------------------------------------- Martin, ::: koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/tools.pref @@ +13,5 @@ > - images. > + News: > + - > + - "Show the author for news items:" > + - pref: NewsAuthorDisplay since you are seeding the sql with "none" as the default (which btw is the correct thing to do), do consider adding after 'pref: NewsAuthorDisplay' - default: none setting it back to assigned Created attachment 43093 [details] [review] Bug 14247: Add system preference for news display Added default value for system preference. Test plan: * Apply dependant patch 14246 (if not done already). * Apply current patch. * Log into the Staff client, go to 'Administration' -> 'Global Preferences' -> 'Tools', a new section titled 'News' should appear with a single option named 'NewsAuthorDisplay'. Available values should be 'Not at all', 'OPAC only', 'Staff client only' and 'Both OPAC and staff client'. By default the value should be 'Not at all' (to mimic existing behaviour, important!). * Change the value to something different and save the changes. * Use the mysql command line client to connect to the Koha database: $ mysql -u kohaadmin -p -D koha * Enter the following query: SELECT * FROM systempreferences WHERE variable = 'NewsAuthorDisplay'; * Check to make sure that the 'value' column has changed to reflect the choice you made in the above step. Separate SQL testing (clean installation of Koha): * Koha$ mysql -u root < create_koha_test_db.sql * Koha$ mysql -u koha_test -D koha_test < installer/data/mysql/kohastructure.sql * Koha$ mysql -u koha_test -D koha_test < installer/data/mysql/sysprefs.sql Testing the update script works the same way: * Koha$ echo "DELETE FROM systempreferences WHERE variable = 'NewsAuthorDisplay';" | mysql -u koha_test -D koha_test * Koha$ mysql -u koha_test -D koha_test < installer/data/mysql/atomicupdate/add_news_prefs.sql Updated as suggested, new patch has default value set. Created attachment 43538 [details] [review] Bug 14247: Add system preference for news display Added default value for system preference. Test plan: * Apply dependant patch 14246 (if not done already). * Apply current patch. * Log into the Staff client, go to 'Administration' -> 'Global Preferences' -> 'Tools', a new section titled 'News' should appear with a single option named 'NewsAuthorDisplay'. Available values should be 'Not at all', 'OPAC only', 'Staff client only' and 'Both OPAC and staff client'. By default the value should be 'Not at all' (to mimic existing behaviour, important!). * Change the value to something different and save the changes. * Use the mysql command line client to connect to the Koha database: $ mysql -u kohaadmin -p -D koha * Enter the following query: SELECT * FROM systempreferences WHERE variable = 'NewsAuthorDisplay'; * Check to make sure that the 'value' column has changed to reflect the choice you made in the above step. Separate SQL testing (clean installation of Koha): * Koha$ mysql -u root < create_koha_test_db.sql * Koha$ mysql -u koha_test -D koha_test < installer/data/mysql/kohastructure.sql * Koha$ mysql -u koha_test -D koha_test < installer/data/mysql/sysprefs.sql Testing the update script works the same way: * Koha$ echo "DELETE FROM systempreferences WHERE variable = 'NewsAuthorDisplay';" | mysql -u koha_test -D koha_test * Koha$ mysql -u koha_test -D koha_test < installer/data/mysql/atomicupdate/add_news_prefs.sql Signed-off-by: Marc Véron <veron@veron.ch> Created attachment 43539 [details] [review] Bug 14246: Update NewsChannel tests This commit adds the new 'borrowernumber' field to the existing test framework to make it pass. It does not include new tests. Sponsored-by: Halland County Library Signed-off-by: Marc Véron <veron@veron.ch> Created attachment 43540 [details] [review] Bug 14247: Add system preference for news display Test plan: * Apply dependant patch 14246 (if not done already). * Apply current patch. * Log into the Staff client, go to 'Administration' -> 'Global Preferences' -> 'Tools', a new section titled 'News' should appear with a single option named 'NewsAuthorDisplay'. Available values should be 'Not at all', 'OPAC only', 'Staff client only' and 'Both OPAC and staff client'. By default the value should be 'Not at all' (to mimic existing behaviour, important!). * Change the value to something different and save the changes. * Use the mysql command line client to connect to the Koha database: $ mysql -u kohaadmin -p -D koha * Enter the following query: SELECT * FROM systempreferences WHERE variable = 'NewsAuthorDisplay'; * Check to make sure that the 'value' column has changed to reflect the choice you made in the above step. Separate SQL testing (clean installation of Koha): * Koha$ mysql -u root < create_koha_test_db.sql * Koha$ mysql -u koha_test -D koha_test < installer/data/mysql/kohastructure.sql * Koha$ mysql -u koha_test -D koha_test < installer/data/mysql/sysprefs.sql Testing the update script works the same way: * Koha$ echo "DELETE FROM systempreferences WHERE variable = 'NewsAuthorDisplay';" | mysql -u koha_test -D koha_test * Koha$ mysql -u koha_test -D koha_test < installer/data/mysql/atomicupdate/add_news_prefs.sql Signed-off-by: Marc Véron <veron@veron.ch> Created attachment 43541 [details] [review] Bug 14247: Add system preference for news display Test plan: * Apply dependant patch 14246 (if not done already). * Apply current patch. * Log into the Staff client, go to 'Administration' -> 'Global Preferences' -> 'Tools', a new section titled 'News' should appear with a single option named 'NewsAuthorDisplay'. Available values should be 'Not at all', 'OPAC only', 'Staff client only' and 'Both OPAC and staff client'. By default the value should be 'Not at all' (to mimic existing behaviour, important!). * Change the value to something different and save the changes. * Use the mysql command line client to connect to the Koha database: $ mysql -u kohaadmin -p -D koha * Enter the following query: SELECT * FROM systempreferences WHERE variable = 'NewsAuthorDisplay'; * Check to make sure that the 'value' column has changed to reflect the choice you made in the above step. Separate SQL testing (clean installation of Koha): * Koha$ mysql -u root < create_koha_test_db.sql * Koha$ mysql -u koha_test -D koha_test < installer/data/mysql/kohastructure.sql * Koha$ mysql -u koha_test -D koha_test < installer/data/mysql/sysprefs.sql Testing the update script works the same way: * Koha$ echo "DELETE FROM systempreferences WHERE variable = 'NewsAuthorDisplay';" | mysql -u koha_test -D koha_test * Koha$ mysql -u koha_test -D koha_test < installer/data/mysql/atomicupdate/add_news_prefs.sql Signed-off-by: Marc Véron <veron@veron.ch> Created attachment 43542 [details] [review] Bug 14247: Add system preference for news display Test plan: * Apply dependant patch 14246 (if not done already). * Apply current patch. * Log into the Staff client, go to 'Administration' -> 'Global Preferences' -> 'Tools', a new section titled 'News' should appear with a single option named 'NewsAuthorDisplay'. Available values should be 'Not at all', 'OPAC only', 'Staff client only' and 'Both OPAC and staff client'. By default the value should be 'Not at all' (to mimic existing behaviour, important!). * Change the value to something different and save the changes. * Use the mysql command line client to connect to the Koha database: $ mysql -u kohaadmin -p -D koha * Enter the following query: SELECT * FROM systempreferences WHERE variable = 'NewsAuthorDisplay'; * Check to make sure that the 'value' column has changed to reflect the choice you made in the above step. Separate SQL testing (clean installation of Koha): * Koha$ mysql -u root < create_koha_test_db.sql * Koha$ mysql -u koha_test -D koha_test < installer/data/mysql/kohastructure.sql * Koha$ mysql -u koha_test -D koha_test < installer/data/mysql/sysprefs.sql Testing the update script works the same way: * Koha$ echo "DELETE FROM systempreferences WHERE variable = 'NewsAuthorDisplay';" | mysql -u koha_test -D koha_test * Koha$ mysql -u koha_test -D koha_test < installer/data/mysql/atomicupdate/add_news_prefs.sql Signed-off-by: Marc Véron <veron@veron.ch> Created attachment 43543 [details] [review] Bug 14247: Rename sysprefs atomic update file This trivial patch changes the name of the SQL update file to match the guidelines on the wiki. Sponsored-by: Halland County Library Signed-off-by: Marc Véron <veron@veron.ch> Comment on attachment 43541 [details] [review] Bug 14247: Add system preference for news display Obsoleted because the same content is in second patch. @ Martin, can you please verify / confirm that the signed-off patches are the correct ones? There is something wrong here - the 3 patches, even with the one I put back lack the logic in the template/pl files for the display. bumblebee:~/kohaclone (14247-newsauthor) $ git diff --stat HEAD~3 installer/data/mysql/atomicupdate/bug_14247-add_news_prefs.sql | 2 ++ installer/data/mysql/sysprefs.sql | 1 + koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/tools.pref | 10 ++++++++++ t/db_dependent/NewsChannels.t | 45 ++++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 57 insertions(+), 1 deletion(-) Also, the tests are not passing - I think because we are missing a dependency - "newsauthor" or bug 14246. We need this to be signed off please asap and this to be put in order. bumblebee:~/kohaclone (14247-newsauthor) $ perl t/db_dependent/NewsChannels.t 1..11 ok 1 - use C4::NewsChannels; # Category CAT1 not found, inserting # Borrower BRWR1 not found, inserting ok 2 ok 3 - Correctly failed on no parameter! ok 4 - Successfully added the first dummy news item! DBD::mysql::st execute failed: Unknown column 'borrowernumber' in 'field list' at /home/katrin/kohaclone/C4/NewsChannels.pm line 69. DBD::mysql::st execute failed: Unknown column 'borrowernumber' in 'field list' at /home/katrin/kohaclone/C4/NewsChannels.pm line 69. # Looks like you planned 11 tests but ran 4. # Looks like your test exited with 255 just after 4. Please check asap - feature slush is close!) Comment on attachment 43539 [details] [review] Bug 14246: Update NewsChannel tests This patch is identical to the one on bug 14246. Since we know that this bug depends on it, there is no need to include it here. Created attachment 43724 [details] [review] Bug 14247: Add system preference for news display Test plan: * Apply dependant patch 14246 (if not done already). * Apply current patch. * Log into the Staff client, go to 'Administration' -> 'Global Preferences' -> 'Tools', a new section titled 'News' should appear with a single option named 'NewsAuthorDisplay'. Available values should be 'Not at all', 'OPAC only', 'Staff client only' and 'Both OPAC and staff client'. By default the value should be 'Not at all' (to mimic existing behaviour, important!). * Change the value to something different and save the changes. * Use the mysql command line client to connect to the Koha database: $ mysql -u kohaadmin -p -D koha * Enter the following query: SELECT * FROM systempreferences WHERE variable = 'NewsAuthorDisplay'; * Check to make sure that the 'value' column has changed to reflect the choice you made in the above step. Separate SQL testing (clean installation of Koha): * Koha$ mysql -u root < create_koha_test_db.sql * Koha$ mysql -u koha_test -D koha_test < installer/data/mysql/kohastructure.sql * Koha$ mysql -u koha_test -D koha_test < installer/data/mysql/sysprefs.sql Testing the update script works the same way: * Koha$ echo "DELETE FROM systempreferences WHERE variable = 'NewsAuthorDisplay';" | mysql -u koha_test -D koha_test * Koha$ mysql -u koha_test -D koha_test < installer/data/mysql/atomicupdate/add_news_prefs.sql NOTE: Adds and allows changes, but no apparent change in OPAC. Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Created attachment 43725 [details] [review] Bug 14247: Rename sysprefs atomic update file This trivial patch changes the name of the SQL update file to match the guidelines on the wiki. Sponsored-by: Halland County Library Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Waiting for bug 14248. Created attachment 43914 [details] [review] [PASSED QA] Bug 14247: Add system preference for news display Test plan: * Apply dependant patch 14246 (if not done already). * Apply current patch. * Log into the Staff client, go to 'Administration' -> 'Global Preferences' -> 'Tools', a new section titled 'News' should appear with a single option named 'NewsAuthorDisplay'. Available values should be 'Not at all', 'OPAC only', 'Staff client only' and 'Both OPAC and staff client'. By default the value should be 'Not at all' (to mimic existing behaviour, important!). * Change the value to something different and save the changes. * Use the mysql command line client to connect to the Koha database: $ mysql -u kohaadmin -p -D koha * Enter the following query: SELECT * FROM systempreferences WHERE variable = 'NewsAuthorDisplay'; * Check to make sure that the 'value' column has changed to reflect the choice you made in the above step. Separate SQL testing (clean installation of Koha): * Koha$ mysql -u root < create_koha_test_db.sql * Koha$ mysql -u koha_test -D koha_test < installer/data/mysql/kohastructure.sql * Koha$ mysql -u koha_test -D koha_test < installer/data/mysql/sysprefs.sql Testing the update script works the same way: * Koha$ echo "DELETE FROM systempreferences WHERE variable = 'NewsAuthorDisplay';" | mysql -u koha_test -D koha_test * Koha$ mysql -u koha_test -D koha_test < installer/data/mysql/atomicupdate/add_news_prefs.sql Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 43915 [details] [review] [PASSED QA] Bug 14247: Rename sysprefs atomic update file This trivial patch changes the name of the SQL update file to match the guidelines on the wiki. Sponsored-by: Halland County Library Signed-off-by: Marc Véron <veron@veron.ch> Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Patches pushed to master. Thanks Martin! |