Description
Viktor Sarge
2015-06-01 09:41:09 UTC
Created attachment 41638 [details] [review] Bug : View arbitrary branch's news This patch adds an input named 'branch' to opac-main.pl that allows overriding the home library to view news from any branch. This is part of the 'Use Koha as a CMS' development. To reviewers: Does this create any security implications? The column is text; it is possbile to push SQL statemens into the DB via the opac-membership.pl page, but all the code there seems to use DBIx::Class and prepared statements. When attemped, accepting the changes were rejected because of the foreign key constraint placed on the main borrowers table. It is recommended to use this patch in conjunction with 11584 which adds a WYSIWYG editor to System Preferences. Test plan: * Make sure your Koha installation has multiple branches: * Log in to the Staff interface and select 'Koha administration' * Select 'Libraries and groups', the first item in the list * Add an extra branch named 'BRANCH1': * Press the 'New library' button * Set 'Library code' to 'BRANCH1' * Enter an arbitrary name * Press 'Submit' * Add at least two news items with different branches: * Log in to the Staff interface and select 'Tools' * Under 'Additional tools' select 'News' * On the 'Koha news' page, create a news item for all libraries: * Press the 'New entry' button * Set 'Library' to 'All libraries' and enter a title. * Press 'Submit' * On the 'Koha news' page, create a news item for one library: * Press the 'New entry' button * Set 'Library' to 'BRANCH1' and enter a title * Press 'Submit' * Add links to OPAC to select branch: * Select 'System preferences' and then the 'OPAC' tab * Scroll down to the 'OpacNav' preference and click 'Click to Edit' * Create two hyperlinks with a parameter named 'branch': * '<a href="?branch=BRANCH1">BRANCH1</a>' * '<a href="?branch=">none</a>' * Click 'Save all OPAC preferences' * Test OPAC: * By default, global news and news for home branch should show. * When clicking the 'BRANCH1' link, you should see global news plus news for the BRANCH1 branch. * When the parameter 'branch' is set but has no value, news from the home branch should be shown. Issues: Tricky to create a link to the same page which removes existing paramters. The user normally has a home branch set. We can override it with the parameter. If undefined, the home branch should be shown. Sponsored-by: Halland County Library Created attachment 41639 [details] [review] Bug 14305: View arbitrary branch's news, RSS fix This fix changes the RSS link to reflect the URL paramter override. This is less elegant than the existing solution which uses the Branches TT module, perhaps there is a better way? Sponsored-By: Halland County Library Test plan: * Follow instructions in the original patch but also check the URLs and contents of the RSS link at the bottom the OPAC page. *** Bug 14764 has been marked as a duplicate of this bug. *** Created attachment 45650 [details] [review] Bug 14305: View arbitrary branch's news This patch adds an input named 'branch' to opac-main.pl that allows overriding the home library to view news from any branch. This is part of the 'Use Koha as a CMS' development. To reviewers: Does this create any security implications? The column is text; it is possbile to push SQL statemens into the DB via the opac-membership.pl page, but all the code there seems to use DBIx::Class and prepared statements. When attemped, accepting the changes were rejected because of the foreign key constraint placed on the main borrowers table. It is recommended to use this patch in conjunction with 11584 which adds a WYSIWYG editor to System Preferences. Test plan: * Make sure your Koha installation has multiple branches: * Log in to the Staff interface and select 'Koha administration' * Select 'Libraries and groups', the first item in the list * Add an extra branch named 'BRANCH1': * Press the 'New library' button * Set 'Library code' to 'BRANCH1' * Enter an arbitrary name * Press 'Submit' * Add at least two news items with different branches: * Log in to the Staff interface and select 'Tools' * Under 'Additional tools' select 'News' * On the 'Koha news' page, create a news item for all libraries: * Press the 'New entry' button * Set 'Library' to 'All libraries' and enter a title. * Press 'Submit' * On the 'Koha news' page, create a news item for one library: * Press the 'New entry' button * Set 'Library' to 'BRANCH1' and enter a title * Press 'Submit' * Add links to OPAC to select branch: * Select 'System preferences' and then the 'OPAC' tab * Scroll down to the 'OpacNav' preference and click 'Click to Edit' * Create two hyperlinks with a parameter named 'branch': * '<a href="?branch=BRANCH1">BRANCH1</a>' * '<a href="?branch=">none</a>' * Click 'Save all OPAC preferences' * Test OPAC: * By default, global news and news for home branch should show. * When clicking the 'BRANCH1' link, you should see global news plus news for the BRANCH1 branch. * When the parameter 'branch' is set but has no value, news from the home branch should be shown. Issues: Tricky to create a link to the same page which removes existing paramters. The user normally has a home branch set. We can override it with the parameter. If undefined, the home branch should be shown. Sponsored-by: Halland County Library Current status: Needs Signoff This ticket overlapped a bit in functionality and actual code with 14764. I am cleaning up both now and marking them properly. This patch only reads the branch from the URL; the user has to create the URLs themselves. This separation is a bit stupid, but the tickets were created before I was familiar with what was practical. Created attachment 45673 [details] [review] Bug 14305: View arbitrary branch's news This patch adds an input named 'branch' to opac-main.pl that allows overriding the home library to view news from any branch. This is part of the 'Use Koha as a CMS' development. To reviewers: Does this create any security implications? The column is text; it is possbile to push SQL statemens into the DB via the opac-membership.pl page, but all the code there seems to use DBIx::Class and prepared statements. When attemped, accepting the changes were rejected because of the foreign key constraint placed on the main borrowers table. It is recommended to use this patch in conjunction with 11584 which adds a WYSIWYG editor to System Preferences. Test plan: * Make sure your Koha installation has multiple branches: * Log in to the Staff interface and select 'Koha administration' * Select 'Libraries and groups', the first item in the list * Add an extra branch named 'BRANCH1': * Press the 'New library' button * Set 'Library code' to 'BRANCH1' * Enter an arbitrary name * Press 'Submit' * Add at least two news items with different branches: * Log in to the Staff interface and select 'Tools' * Under 'Additional tools' select 'News' * On the 'Koha news' page, create a news item for all libraries: * Press the 'New entry' button * Set 'Library' to 'All libraries' and enter a title. * Press 'Submit' * On the 'Koha news' page, create a news item for one library: * Press the 'New entry' button * Set 'Library' to 'BRANCH1' and enter a title * Press 'Submit' * Add links to OPAC to select branch: * Select 'System preferences' and then the 'OPAC' tab * Scroll down to the 'OpacNav' preference and click 'Click to Edit' * Create two hyperlinks with a parameter named 'branch': * '<a href="?branch=BRANCH1">BRANCH1</a>' * '<a href="?branch=">none</a>' * Click 'Save all OPAC preferences' * Test OPAC: * By default, global news and news for home branch should show. * When clicking the 'BRANCH1' link, you should see global news plus news for the BRANCH1 branch. * When the parameter 'branch' is set but has no value, news from the home branch should be shown. Issues: Tricky to create a link to the same page which removes existing paramters. The user normally has a home branch set. We can override it with the parameter. If undefined, the home branch should be shown. Sponsored-by: Halland County Library http://bugs.koha-community.org/show_bug.cgi?id=14305 Changed 2015-12-14: Whitespace fix Comment on attachment 41639 [details] [review] Bug 14305: View arbitrary branch's news, RSS fix Review of attachment 41639 [details] [review]: ----------------------------------------------------------------- ::: koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-main.tt @@ -53,2 @@ > <a href="[% OPACBaseURL %]/cgi-bin/koha/opac-news-rss.pl?branchcode=[% branchcode %]"><img src="[% interface %]/[% theme %]/images/feed-icon-16x16.png"></a> > RSS feed for [% IF ( loggedinusername ) %][% Branches.GetName( branchcode ) %] <i>and</i> [% END %] system-wide library news. If a user is not logged in and passes branchcode=BranchA in the URL, line 54 isn't going to display correctly. Add '?branch={some branchcode that has specific branch news}' to your opac-main url. Notice the RSS message is wrong. Created attachment 49917 [details] [review] Bug 14305: RSS message correction follow-up If a user is not logged in but requests a specific branch, the RSS feed message fails to mention the branch. TEST PLAN --------- 1) Apply first two patches 2) go to OPAC's opac-main.pl?branch={some branch with specific news} -- notice bad RSS message 3) Apply this patch 4) Repeat step 2 -- notice branch is properly shown. 5) koha qa test tools Created attachment 49918 [details] [review] Bug 14305: View arbitrary branch's news, RSS fix This fix changes the RSS link to reflect the URL paramter override. This is less elegant than the existing solution which uses the Branches TT module, perhaps there is a better way? Sponsored-By: Halland County Library Test plan: * Follow instructions in the original patch but also check the URLs and contents of the RSS link at the bottom the OPAC page. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Created attachment 49919 [details] [review] Bug 14305: View arbitrary branch's news This patch adds an input named 'branch' to opac-main.pl that allows overriding the home library to view news from any branch. This is part of the 'Use Koha as a CMS' development. To reviewers: Does this create any security implications? The column is text; it is possbile to push SQL statemens into the DB via the opac-membership.pl page, but all the code there seems to use DBIx::Class and prepared statements. When attemped, accepting the changes were rejected because of the foreign key constraint placed on the main borrowers table. It is recommended to use this patch in conjunction with 11584 which adds a WYSIWYG editor to System Preferences. Test plan: * Make sure your Koha installation has multiple branches: * Log in to the Staff interface and select 'Koha administration' * Select 'Libraries and groups', the first item in the list * Add an extra branch named 'BRANCH1': * Press the 'New library' button * Set 'Library code' to 'BRANCH1' * Enter an arbitrary name * Press 'Submit' * Add at least two news items with different branches: * Log in to the Staff interface and select 'Tools' * Under 'Additional tools' select 'News' * On the 'Koha news' page, create a news item for all libraries: * Press the 'New entry' button * Set 'Library' to 'All libraries' and enter a title. * Press 'Submit' * On the 'Koha news' page, create a news item for one library: * Press the 'New entry' button * Set 'Library' to 'BRANCH1' and enter a title * Press 'Submit' * Add links to OPAC to select branch: * Select 'System preferences' and then the 'OPAC' tab * Scroll down to the 'OpacNav' preference and click 'Click to Edit' * Create two hyperlinks with a parameter named 'branch': * '<a href="?branch=BRANCH1">BRANCH1</a>' * '<a href="?branch=">none</a>' * Click 'Save all OPAC preferences' * Test OPAC: * By default, global news and news for home branch should show. * When clicking the 'BRANCH1' link, you should see global news plus news for the BRANCH1 branch. * When the parameter 'branch' is set but has no value, news from the home branch should be shown. Issues: Tricky to create a link to the same page which removes existing paramters. The user normally has a home branch set. We can override it with the parameter. If undefined, the home branch should be shown. Sponsored-by: Halland County Library http://bugs.koha-community.org/show_bug.cgi?id=14305 Changed 2015-12-14: Whitespace fix Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Created attachment 49920 [details] [review] Bug 14305: RSS message correction follow-up If a user is not logged in but requests a specific branch, the RSS feed message fails to mention the branch. TEST PLAN --------- 1) Apply first patch 2) go to OPAC's opac-main.pl?branch={some branch with specific news} -- notice bad RSS message 3) Apply this patch 4) Repeat step 2 -- notice branch is properly shown. 5) koha qa test tools Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Left as Needs Signoff, because I can't sign off my own patch. Please test and sign off. (In reply to M. Tompsett from comment #11) > TEST PLAN > --------- > 1) Apply first patch -- actually the first two. > 2) go to OPAC's opac-main.pl?branch={some branch with specific news} > -- notice bad RSS message > 3) Apply this patch > 4) Repeat step 2 > -- notice branch is properly shown. > 5) koha qa test tools > > Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Created attachment 50152 [details] [review] [SIGNED-OFF] Bug 14305: RSS message correction follow-up If a user is not logged in but requests a specific branch, the RSS feed message fails to mention the branch. TEST PLAN --------- 1) Apply first patch 2) go to OPAC's opac-main.pl?branch={some branch with specific news} -- notice bad RSS message 3) Apply this patch 4) Repeat step 2 -- notice branch is properly shown. 5) koha qa test tools Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 50186 [details] [review] Bug 14305: View arbitrary branch's news, RSS fix This fix changes the RSS link to reflect the URL paramter override. This is less elegant than the existing solution which uses the Branches TT module, perhaps there is a better way? Sponsored-By: Halland County Library Test plan: * Follow instructions in the original patch but also check the URLs and contents of the RSS link at the bottom the OPAC page. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 50187 [details] [review] Bug 14305: View arbitrary branch's news This patch adds an input named 'branch' to opac-main.pl that allows overriding the home library to view news from any branch. This is part of the 'Use Koha as a CMS' development. To reviewers: Does this create any security implications? The column is text; it is possbile to push SQL statemens into the DB via the opac-membership.pl page, but all the code there seems to use DBIx::Class and prepared statements. When attemped, accepting the changes were rejected because of the foreign key constraint placed on the main borrowers table. It is recommended to use this patch in conjunction with 11584 which adds a WYSIWYG editor to System Preferences. Test plan: * Make sure your Koha installation has multiple branches: * Log in to the Staff interface and select 'Koha administration' * Select 'Libraries and groups', the first item in the list * Add an extra branch named 'BRANCH1': * Press the 'New library' button * Set 'Library code' to 'BRANCH1' * Enter an arbitrary name * Press 'Submit' * Add at least two news items with different branches: * Log in to the Staff interface and select 'Tools' * Under 'Additional tools' select 'News' * On the 'Koha news' page, create a news item for all libraries: * Press the 'New entry' button * Set 'Library' to 'All libraries' and enter a title. * Press 'Submit' * On the 'Koha news' page, create a news item for one library: * Press the 'New entry' button * Set 'Library' to 'BRANCH1' and enter a title * Press 'Submit' * Add links to OPAC to select branch: * Select 'System preferences' and then the 'OPAC' tab * Scroll down to the 'OpacNav' preference and click 'Click to Edit' * Create two hyperlinks with a parameter named 'branch': * '<a href="?branch=BRANCH1">BRANCH1</a>' * '<a href="?branch=">none</a>' * Click 'Save all OPAC preferences' * Test OPAC: * By default, global news and news for home branch should show. * When clicking the 'BRANCH1' link, you should see global news plus news for the BRANCH1 branch. * When the parameter 'branch' is set but has no value, news from the home branch should be shown. Issues: Tricky to create a link to the same page which removes existing paramters. The user normally has a home branch set. We can override it with the parameter. If undefined, the home branch should be shown. Sponsored-by: Halland County Library http://bugs.koha-community.org/show_bug.cgi?id=14305 Changed 2015-12-14: Whitespace fix Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 50188 [details] [review] Bug 14305: RSS message correction follow-up If a user is not logged in but requests a specific branch, the RSS feed message fails to mention the branch. TEST PLAN --------- 1) Apply first patch 2) go to OPAC's opac-main.pl?branch={some branch with specific news} -- notice bad RSS message 3) Apply this patch 4) Repeat step 2 -- notice branch is properly shown. 5) koha qa test tools Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Pushed to master for the May 2016 release! Obrigado Martin! |