News items in Koha's OPAC and Staff client are anonymous. This is the first in a set of patches to add authorship tracking to the news items, with optional display preferences maintaining the current functionality. This first patch changes the database; it adds a column called 'borrowernumber' to the 'koha_news' table, adding a foreign key relationship to the 'borrowers' table. This is how tracking authorship is done. A LEFT JOIN statement is then used to get the title, firstname and surname columns available in the news data set. That is a separate patch.
Created attachment 39361 [details] [review] Adds koha_news column borrowernumber Should merge with f52084df0e395fdf89d80ca2fd77844273a8cf7c
Comment on attachment 39361 [details] [review] Adds koha_news column borrowernumber Missing key functionality.
Created attachment 39413 [details] [review] Adds borrowerid column to news, stores current user id. Final tested version.
Created attachment 39417 [details] SQL command file to revert changes to database Usage: $ mysql -u kohaadmin -p -D koha < remove_news_author.sql
Test plan: * Check out clean master, reference sha-1 for this was: f52084df0e395fdf89d80ca2fd77844273a8cf7c * You need at least one news item, log in and add one from 'Administration' -> 'Tools' -> 'News'. * Create a branch for the testing: git checkout -b test_news_author * Apply the patch. * Execute the database update: $ perl installer/data/mysql/atomicupdate/add_news_author.pl * Go back to Staff interface and post a new news item. * Use the mysql command line client to connect to the Koha installation's database: mysql -u kohaadmin -p -D koha (change username and database to values appropiate for your system). * Execute the following query: SELECT title, borrowernumber FROM opac_news; * Inspect the results, the 'borrowernumber' column should be NULL for all news items created before the patch was applied, and non-NULL for the new item created AFTER the patch was applied. To remove all the changes (there are dependant patches that you can test before you do this!): * Execute the supplied SQL script: $ mysql -u kohaadmin -p -D koha < remove_news_author.sql * Drop the testing branch: $ git branch -d test_news_author
Created attachment 39439 [details] [review] [Signed-off] Bug 14246: Add koha_news.borrowernumber and store This patch adds a new column to koha_news that links a borrowernumber to each item. This allows Koha to display the author for each entry which makes it suitable as a simple CMS. Sponsored-by: Halland County Library Signed-off-by: Marc Véron <veron@veron.ch>
This is missing a corresponding kohastructure.sql changes, or schema file changes as well. http://wiki.koha-community.org/wiki/Database_updates
Created attachment 39521 [details] [review] Updated version in accordance wih Wiki rules.
Comment on attachment 39521 [details] [review] Updated version in accordance wih Wiki rules. Review of attachment 39521 [details] [review]: ----------------------------------------------------------------- ::: installer/data/mysql/atomicupdate/bug_14246-add_news_author.sql @@ +1,1 @@ > +ALTER TABLE `opac_news` ADD `borrowernumber` INT(11) default NULL; Please add an AFTER clause, to ensure that an upgrade and a fresh install will have the same field order. https://dev.mysql.com/doc/refman/5.7/en/alter-table.html
Okay, will do. The tests probably also needs to be updated as they seem to compare the database content, which I changed. Thanks for the feedback.
Created attachment 39745 [details] [review] Updated SQL, AFTER qualifier and changed UPDATE action This is a differential patchset against the earlier one.
Created attachment 39747 [details] [review] Updated tests in NewsChannels.t This patch updates the existing tests in NewsChannel.t to make them pass with the new patches. However, they are pretty basic and could probably stand to be updated. I have some more complex tests written using the new DBIx::Class framework and TestBuilder, where could I possibly include them?
Comment on attachment 39745 [details] [review] Updated SQL, AFTER qualifier and changed UPDATE action Review of attachment 39745 [details] [review]: ----------------------------------------------------------------- ::: installer/data/mysql/atomicupdate/bug_14246-add_news_author.sql @@ +1,1 @@ > +ALTER TABLE `opac_news` ADD `borrowernumber` int(11) AFTER `number` default NULL; The AFTER clause should follow the definition of the column, which would include 'default NULL'. This is a MySQL syntax error.
Setting back to ASSIGNED, so Martin can correct issue raised in comment #13.
Created attachment 40125 [details] [review] (fix) correct ALTER TABLE mysql syntax Address bug 14246 comment 13 remark - correcting MySQL ALTER TABLE syntax.
I guess this feature should be also be used by OPAC/Staff side. Now the user itself cannot use the feature (only people with access to the db).
(In reply to Joonas Kylmälä from comment #16) > I guess this feature should be also be used by OPAC/Staff side. Now the user > itself cannot use the feature (only people with access to the db). This patch simply adds a new column as a base for further development. It does what is advertised in comment #1: "it adds a column called 'borrowernumber' to the 'koha_news' table, adding a foreign key relationship to the 'borrowers' table." With patch, creating a news item fills in the borrowernumber. This information can later be used, it opens up a lot of possibilities. My opinion is that the patch should be switched back to "Needs sign-off".
This patch is needed to continue with 14247. Please sign-off asap!
(In reply to Katrin Fischer from comment #18) > This patch is needed to continue with 14247. Please sign-off asap! Koha Dev folks... can we sign off without Koha::Schema::Result::Opacnews being updated?
Including a Schema changes is still optional I'd say, see: http://wiki.koha-community.org/wiki/Database_updates ...if you _want_ to...
(In reply to Katrin Fischer from comment #20) > Including a Schema changes is still optional I'd say, see: > http://wiki.koha-community.org/wiki/Database_updates > ...if you _want_ to... I asked, because I discovered issues with AuthorisedValuesBranch and MarcModificationTemplateAction and isnullable, when I tried the koha schema building thing. Totally unrelated to this. Back to testing.
Created attachment 43714 [details] [review] Bug 14246: Add borrowernumber to koha_news This patch adds a new column to koha_news that links a borrowernumber to each item. This allows Koha to display the author for each entry which makes it suitable as a simple CMS. Changes (from rejected patch): * Added missing kohastructure.sql changes. * Turned the atomic update file into a SQL file and changed name in accordance with wiki guidelines. * Changed SQL syntax and naming to be consistent with existing code. * Attached test plan to commit message. Test plan: * You need at least one news item, log in and add one from 'Administration' -> 'Tools' -> 'News'. * Apply the patch. * Apply database upgrade (directly or indirectly). * Go back to Staff interface and post a new news item. * Use the mysql command line client to connect to the Koha installation's database: mysql -u kohaadmin -p -D koha (change username and database to values appropiate for your system). * Execute the following query: SELECT title, borrowernumber FROM opac_news; * Inspect the results, the 'borrowernumber' column should be NULL for all news items created before the patch was applied, and non-NULL for the new item created AFTER the patch was applied. Optional: Remove database changes via: ALTER TABLE opac_news DROP FOREIGN KEY borrowernumber_fk; ALTER TABLE opac_news DROP COLUMN borrowernumber; Sponsored-By: Halland County Library Signed-off-by: Nick Clemens <nick@quecheelibrary.org>
Created attachment 43715 [details] [review] Bug 14246: Add borrowernumber, altered SQL As suggested by kind reviewers, an AFTER statement was added to the atomic upgrade script to ensure that the final field order is consistent is both fresh and upgraded databases. Also: * UPDATE action changed to CASCADE to improve robustness. Test plan: * Apply first patch in this set. * Apply this patch. * Perform a database upgrade. * Use mysql client to inspect field order. It should be identical to the kohastructure.sql order. Sponsored-by: Halland County Library Signed-off-by: Nick Clemens <nick@quecheelibrary.org>
Created attachment 43716 [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: Nick Clemens <nick@quecheelibrary.org>
Created attachment 43717 [details] [review] (fix) correct ALTER TABLE mysql syntax Address bug 14246 comment 13 remark - correcting MySQL ALTER TABLE syntax. Signed-off-by: Nick Clemens <nick@quecheelibrary.org>
Created attachment 43718 [details] [review] Bug 14246: Adding Koha::Schema::Result differences Ran misc/devel/update_dbix_class_files.pl as shown on wiki/Database_updates#updatedatabase.pl However, only git added Borrower and Opacnews, since those were the two affected by this patch.
Created attachment 43719 [details] [review] Bug 14246: Add borrowernumber to koha_news This patch adds a new column to koha_news that links a borrowernumber to each item. This allows Koha to display the author for each entry which makes it suitable as a simple CMS. Changes (from rejected patch): * Added missing kohastructure.sql changes. * Turned the atomic update file into a SQL file and changed name in accordance with wiki guidelines. * Changed SQL syntax and naming to be consistent with existing code. * Attached test plan to commit message. Test plan: * You need at least one news item, log in and add one from 'Administration' -> 'Tools' -> 'News'. * Apply the patch. * Apply database upgrade (directly or indirectly). * Go back to Staff interface and post a new news item. * Use the mysql command line client to connect to the Koha installation's database: mysql -u kohaadmin -p -D koha (change username and database to values appropiate for your system). * Execute the following query: SELECT title, borrowernumber FROM opac_news; * Inspect the results, the 'borrowernumber' column should be NULL for all news items created before the patch was applied, and non-NULL for the new item created AFTER the patch was applied. Optional: Remove database changes via: ALTER TABLE opac_news DROP FOREIGN KEY borrowernumber_fk; ALTER TABLE opac_news DROP COLUMN borrowernumber; Sponsored-By: Halland County Library Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Created attachment 43720 [details] [review] Bug 14246: Add borrowernumber, altered SQL As suggested by kind reviewers, an AFTER statement was added to the atomic upgrade script to ensure that the final field order is consistent is both fresh and upgraded databases. Also: * UPDATE action changed to CASCADE to improve robustness. Test plan: * Apply first patch in this set. * Apply this patch. * Perform a database upgrade. * Use mysql client to inspect field order. It should be identical to the kohastructure.sql order. Sponsored-by: Halland County Library Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Created attachment 43721 [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: Nick Clemens <nick@quecheelibrary.org> Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Created attachment 43722 [details] [review] (fix) correct ALTER TABLE mysql syntax Address bug 14246 comment 13 remark - correcting MySQL ALTER TABLE syntax. Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Created attachment 43723 [details] [review] Bug 14246: Adding Koha::Schema::Result differences Ran misc/devel/update_dbix_class_files.pl as shown on wiki/Database_updates#updatedatabase.pl However, only git added Borrower and Opacnews, since those were the two affected by this patch. Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
I tested: - Cascading when a borrower is deleted, the borrowernumber is set to NULL. - Adding is the only time a borrowernumber is set. - t/db_dependent/NewsChannels.t A couple remarks: - I don't like diags added in the tests. - I would have gone further and allowed setting and changing the borrower number, but I think that is beyond scope.
(In reply to M. Tompsett from comment #32) (...) > - I would have gone further and allowed > setting and changing the borrower number, but I think that is beyond scope. I look forward to see that as well - in a new Bug on top of and as enhancement to this one.
Waiting for bug 14248.
Created attachment 43909 [details] [review] [PASSED QA] Bug 14246: Add borrowernumber to koha_news This patch adds a new column to koha_news that links a borrowernumber to each item. This allows Koha to display the author for each entry which makes it suitable as a simple CMS. Changes (from rejected patch): * Added missing kohastructure.sql changes. * Turned the atomic update file into a SQL file and changed name in accordance with wiki guidelines. * Changed SQL syntax and naming to be consistent with existing code. * Attached test plan to commit message. Test plan: * You need at least one news item, log in and add one from 'Administration' -> 'Tools' -> 'News'. * Apply the patch. * Apply database upgrade (directly or indirectly). * Go back to Staff interface and post a new news item. * Use the mysql command line client to connect to the Koha installation's database: mysql -u kohaadmin -p -D koha (change username and database to values appropiate for your system). * Execute the following query: SELECT title, borrowernumber FROM opac_news; * Inspect the results, the 'borrowernumber' column should be NULL for all news items created before the patch was applied, and non-NULL for the new item created AFTER the patch was applied. Optional: Remove database changes via: ALTER TABLE opac_news DROP FOREIGN KEY borrowernumber_fk; ALTER TABLE opac_news DROP COLUMN borrowernumber; Sponsored-By: Halland County Library Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 43910 [details] [review] [PASSED QA] Bug 14246: Add borrowernumber, altered SQL As suggested by kind reviewers, an AFTER statement was added to the atomic upgrade script to ensure that the final field order is consistent is both fresh and upgraded databases. Also: * UPDATE action changed to CASCADE to improve robustness. Test plan: * Apply first patch in this set. * Apply this patch. * Perform a database upgrade. * Use mysql client to inspect field order. It should be identical to the kohastructure.sql order. Sponsored-by: Halland County Library Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 43911 [details] [review] [PASSED QA] 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: Nick Clemens <nick@quecheelibrary.org> Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 43912 [details] [review] [PASSED QA] (fix) correct ALTER TABLE mysql syntax Address bug 14246 comment 13 remark - correcting MySQL ALTER TABLE syntax. Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 43913 [details] [review] [PASSED QA] Bug 14246: Adding Koha::Schema::Result differences Ran misc/devel/update_dbix_class_files.pl as shown on wiki/Database_updates#updatedatabase.pl However, only git added Borrower and Opacnews, since those were the two affected by this patch. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Patches pushed to master. Thanks Martin!