Bugzilla – Attachment 64241 Details for
Bug 15326
Add CMS feature
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 15326: [FOLLOW-UP] Many fixes
Bug-15326-FOLLOW-UP-Many-fixes.patch (text/plain), 15.53 KB, created by
Aleisha Amohia
on 2017-06-12 23:54:03 UTC
(
hide
)
Description:
Bug 15326: [FOLLOW-UP] Many fixes
Filename:
MIME Type:
Creator:
Aleisha Amohia
Created:
2017-06-12 23:54:03 UTC
Size:
15.53 KB
patch
obsolete
>From 80408e665e7245eab6d6d2ddbf7392c5615ca2a7 Mon Sep 17 00:00:00 2001 >From: Aleisha Amohia <aleishaamohia@hotmail.com> >Date: Mon, 12 Jun 2017 23:51:25 +0000 >Subject: [PATCH] Bug 15326: [FOLLOW-UP] Many fixes > >This patch: >- moves action buttons into a dropdown, adds a delete button to dropdown >- fixes the tests (they should now all pass) >- fixes qa tool problems >- removes the Koha::CmsPages->remove() method (unnecessary) >- makes changes to the Koha::CmsPages->add() method (now simpler, easier >to test) > >This is now ready to test. > >Sponsored-by: Catalyst IT >--- > Koha/CmsPages.pm | 67 ++++------- > .../prog/en/modules/tools/cmspages.tt | 43 ++++--- > t/db_dependent/Pages.t | 123 ++++++++++++--------- > tools/cmspages.pl | 22 +++- > 4 files changed, 145 insertions(+), 110 deletions(-) > >diff --git a/Koha/CmsPages.pm b/Koha/CmsPages.pm >index 85a612f..a5ed1ea 100644 >--- a/Koha/CmsPages.pm >+++ b/Koha/CmsPages.pm >@@ -49,49 +49,45 @@ sub object_class { > } > > =head3 add >+ > Adds a new page to the CMS database based on the input data. >+ > =cut > > sub add { > my ( $self, $input ) = @_; > carp( 'Koha::CmsPages::add() undefined parameter: input' ) unless defined $input; >+ > my $data = { >- parent => scalar $input->param( 'parent' ) eq '' ? undef : scalar $input->param( 'parent' ), >- location => scalar $input->param( 'disp' ) eq '' ? undef : scalar $input->param( 'disp'), >- branchcode => scalar $input->param( 'branch' ) eq '' ? undef : scalar $input->param( 'branch' ), >- lang => scalar $input->param( 'lang' ), >- sortorder => scalar $input->param( 'number' ), >- title => scalar $input->param( 'title' ), >- title_link => scalar $input->param( 'title_link' ), >- publish => defined (scalar $input->param( 'publish' )), >- content => scalar $input->param( 'content' ) >+ parent => $input->{parent}, >+ location => $input->{location}, >+ branchcode => $input->{branchcode}, >+ lang => $input->{lang}, >+ sortorder => $input->{sortorder}, >+ title => $input->{title}, >+ title_link => $input->{title_link}, >+ publish => $input->{publish}, >+ content => $input->{content} > }; > >- if (defined $input->param( 'id' )) { >- $data->{'id'} = scalar $input->param( 'id' ); >+ if (defined $input->{id}){ >+ $data->{id} = $input->{id}; > } > > # Ensuring link titles are unique >- my $title_link = $input->param('title_link'); >- if ( Koha::CmsPages->find({ title_link => $title_link }) ) { >- my $existing_link = Koha::CmsPages->find({ title_link => $title_link }); >- my $link = $existing_link->unblessed; >- unless ( $link->{'id'} eq $input->param('id') ) { >- return undef; >+ my $title_link = $input->{title_link}; >+ if ( Koha::CmsPages->find({ title_link => $title_link }) ){ >+ my $link = Koha::CmsPages->find({ title_link => $title_link }); >+ unless ( $link->id eq $input->{id} ){ >+ return; > } > } > > # Ensuring that, if page has parent, provided location suits parent location >- my $parent = $input->param('parent'); >- my $parent_obj = Koha::CmsPages->find($parent); >- if ( $parent_obj ) { >- my $parent_unblessed = $parent_obj->unblessed; >- if ( $parent_unblessed->{'location'} ne "" ) { >- # if parent location is not all interfaces >- if ( $parent_unblessed->{'location'} != $input->param('disp') ) { >- # if parent location is not the same as provided location >- return undef; >- } >+ my $parent = Koha::CmsPages->find($input->{parent}); >+ if ( $parent && $parent->location ne "" ){ >+ if ( $parent->location ne $input->{location} ){ >+ return; > } > } > >@@ -102,7 +98,7 @@ sub add { > > =head3 list > >-Lists pages based input filtering paramters for location, branch and language. >+Lists pages based input filtering parameters for location, branch and language. > > =cut > >@@ -199,21 +195,6 @@ sub child_links { > return $rs; > } > >-=head3 remove >- >-Removes a page identified by id from the CMS database. >- >-=cut >- >-sub remove { >- my ( $self, @ids ) = @_; >- >- carp( 'Koha::CmsPages::remove() undefined parameter ids' ) unless @ids; >- >- my $pages = $self->_resultset()->search({ id => \@ids }); >- $pages->delete; >-} >- > =head1 AUTHOR > > Martin Persson <xarragon@gmail.com> >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/cmspages.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/cmspages.tt >index 901c94d..43dac1a 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/cmspages.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/cmspages.tt >@@ -9,8 +9,6 @@ > [% INCLUDE 'datatables.inc' %] > <script type="text/javascript" src="[% interface %]/lib/tiny_mce/tiny_mce.js"></script> > <script type="text/javascript">//<![CDATA[ >-var MSG_CONFIRM_DELETE_PAGE = _("Are you sure you want to delete the selected page(s)?"); >- > $(document).ready(function() { > $("#pages_table").dataTable($.extend(true, {}, dataTablesDefaults, { > "aoColumnDefs": [ >@@ -20,7 +18,16 @@ $(document).ready(function() { > })); > > $("#del_button").on("click",function(){ >- return confirmDelete(MSG_CONFIRM_DELETE_PAGE); >+ if ($("input[name='ids'][type='checkbox']:checked").length){ >+ return confirmDelete(_("Are you sure you want to delete the selected page(s)?")); >+ } else { >+ alert(_("No pages have been selected.")); >+ return false; >+ } >+ }); >+ >+ $(".del_single").on("click", function(){ >+ return confirmDelete(_("Are you sure you want to delete this page?")); > }); > > $(".SelectAll").on("click", function(){ >@@ -89,7 +96,7 @@ tinyMCE.init({ > [% END %] > [% UNLESS ( form || view ) %] > <div id="toolbar" class="btn-toolbar"> >- <a class="btn btn-small" id="newentry" href="[% link_self %]?op=form"><i class="fa fa-plus"></i> New page</a> >+ <a class="btn btn-default btn-sm" id="newentry" href="[% link_self %]?op=form"><i class="fa fa-plus"></i> New page</a> > </div> > [% END %] > [% IF ( failed_add ) %] >@@ -252,7 +259,7 @@ tinyMCE.init({ > <th>Number</th> > <th>Link title</th> > <th>Page title</th> >- <th>Actions</th> >+ <th> </th> > </tr></thead> > <tbody>[% FOREACH page IN page_list %] > <tr> >@@ -278,15 +285,23 @@ tinyMCE.init({ > <td><a href="[% link_self %]?op=form&id=[% page.id %]">[% page.title_link %]</a></td> > <td>[% page.title %]</td> > <td class="actions"> >- <a href="[% link_self %]?op=form&id=[% page.id %]" class="btn btn-mini"><i class="fa fa-pencil"></i> Edit</a> >- [% IF ( page.location == "" ) %] >- <a href="[% link_opac %]?id=[% page.id %]" class="btn btn-mini"><i class="fa fa-eye"></i> OPAC</a> >- <a href="[% link_self %]?op=view&id=[% page.id %]" class="btn btn-mini"><i class="fa fa-eye"></i> Intranet</a> >- [% ELSIF ( page.location == "1" ) %] >- <a href="[% link_self %]?op=view&id=[% page.id %]" class="btn btn-mini"><i class="fa fa-eye"></i> Intranet</a> >- [% ELSIF ( page.location == "2" ) %] >- <a href="[% link_opac %]?id=[% page.id %]" class="btn btn-mini"><i class="fa fa-eye"></i> OPAC</a> >- [% END %] >+ <div class="btn-group"> >+ <a class="btn btn-default btn-xs dropdown-toggle" id="opencmspageactions[% page.id %]" role="button" data-toggle="dropdown" href="#"> >+ Actions <b class="caret"></b></a> >+ <ul class="dropdown-menu pull-right" role="menu" aria-labelledby="opencmspageactions[% page.id %]"> >+ <li><a href="[% link_self %]?op=form&id=[% page.id %]"><i class="fa fa-pencil"></i> Edit</a></li> >+ <li><a href="[% link_self %]?op=delSingle&id=[% page.id %]" class="del_single"><i class="fa fa-trash"></i> Delete</a></li> >+ [% IF ( page.location == "" ) %] >+ <li><a href="[% link_opac %]?id=[% page.id %]"><i class="fa fa-eye"></i> OPAC</a></li> >+ <li><a href="[% link_self %]?op=view&id=[% page.id %]"><i class="fa fa-eye"></i> Intranet</a></li> >+ [% ELSIF ( page.location == "1" ) %] >+ <li><a href="[% link_self %]?op=view&id=[% page.id %]"><i class="fa fa-eye"></i> Intranet</a></li> >+ [% ELSIF ( page.location == "2" ) %] >+ <li><a href="[% link_opac %]?id=[% page.id %]"><i class="fa fa-eye"></i> OPAC</a></li> >+ [% END %] >+ </ul> >+ </div> >+ </td> > </tr> > [% END %]</tbody> > </table> >diff --git a/t/db_dependent/Pages.t b/t/db_dependent/Pages.t >index 2c98c38..bad7477 100644 >--- a/t/db_dependent/Pages.t >+++ b/t/db_dependent/Pages.t >@@ -18,75 +18,96 @@ > > use Modern::Perl; > >-use Test::More tests => 2; >+use Test::More tests => 10; > use Test::Warn; >-use Data::Dumper::Concise; > > use C4::Context; > use Koha::Database; > use t::lib::TestBuilder; > > BEGIN { >- use_ok('Koha::Objects'); > use_ok('Koha::CmsPages'); >+ use_ok('Koha::CmsPage'); > } > >-my $builder = t::lib::TestBuilder->new(); >+my $database = Koha::Database->new(); >+my $schema = $database->schema(); >+my $dbh = C4::Context->dbh; >+my $builder = t::lib::TestBuilder->new(); > >-# Start transaction >-my $dbh = C4::Context->dbh; >-$dbh->{ AutoCommit } = 0; >-$dbh->{ RaiseError } = 1; >-$dbh->do( 'DELETE FROM items' ); >+$schema->storage->txn_begin(); >+$dbh->do( 'DELETE FROM cms_pages' ); >+$dbh->do( 'DELETE FROM issues' ); > $dbh->do( 'DELETE FROM borrowers' ); >+$dbh->do( 'DELETE FROM items' ); > $dbh->do( 'DELETE FROM branches' ); >-$dbh->do( 'DELETE FROM cms_pages' ); > >-my $branchcode = $builder->build({ source => 'Branch' }); >- >-my $page10 = $builder->build({ >- source => 'CmsPage', >- value => { >- parent => undef, >- title_link => 'page 10', >- title => 'full page 10', >- published => 1, >- sortorder => 1, >- } >-}); >-my $page11 = $builder->build({ >- source => 'CmsPage', >- value => { >- parent => $page10->{'id'}, >- title_link => 'subpage 11', >- title => 'full subpage 11', >- published => 1, >- sortorder => 0, >- } >+my $branchcode = $builder->build({ source => 'Branch' })->{'branchcode'}; >+my $borrowernumber = $builder->build({ source => 'Borrower' })->{'borrowernumber'}; >+ >+# new parent page >+my $parent_page = Koha::CmsPages->add({ >+ id => 1, >+ branchcode => $branchcode, >+ location => 2, >+ lang => '', >+ parent => undef, >+ title_link => 'page1', >+ title => 'Page 1', >+ publish => 0, >+ sortorder => 1, >+ content => "This is the first page.", > }); >-my $page12 = $builder->build({ >- source => 'CmsPage', >- value => { >- parent => $page10->{'id'}, >- title_link => 'subpage 12', >- title => 'full subpage 12', >- published => 1, >- sortorder => 1, >- } >+ >+is($parent_page->title, 'Page 1', "First page created"); >+ >+# new child page >+my $child_page = Koha::CmsPages->add({ >+ id => 2, >+ branchcode => $branchcode, >+ location => 2, >+ lang => '', >+ parent => $parent_page->id, >+ title_link => 'page2', >+ title => 'Page 2', >+ publish => 1, >+ sortorder => 2, >+ content => "This is the second page.", > }); >-my $page20 = $builder->build({ >- source => 'CmsPage', >- value => { >- parent => undef, >- title_link => 'page 20', >- title => 'full page 20', >- published => 0, >- sortorder => 2, >- } >+ >+# $page->parent specifies who this page is the parent of >+is(Koha::CmsPages->find($child_page->parent)->title, 'Page 1', 'Child and parent page connected'); >+ >+# Koha::CmsPages->list() with no specified ID should return all pages >+my ( $page_list, $parent_list, $data ) = Koha::CmsPages->list( '' ); >+is(scalar @{ $page_list }, 2, "Two pages have been created"); >+ >+# Koha::CmsPages->list() is used for filtering (filter display location) >+( $page_list, $parent_list, $data ) = Koha::CmsPages->list( '', 2, '', '' ); >+is(scalar @{ $page_list }, 2, "Both pages can be seen on staff interface"); >+ >+is($parent_list->[0]->{id}, $parent_page->id, "Correct page is returned as a parent"); >+is($data, undef, "Data is undefined because no ID was given"); >+ >+# Koha::CmsPages->list() is used to show data about a page when given an ID >+( $page_list, $parent_list, $data ) = Koha::CmsPages->list( 2 ); >+is($data->{content}, "This is the second page.", "Data is defined when given an ID"); >+ >+# Try to add a child page under a page where their locations do not match up >+my $test_page = Koha::CmsPages->add({ >+ id => 3, >+ branchcode => $branchcode, >+ location => 1, >+ lang => '', >+ parent => $child_page->id, >+ title_link => 'failedadd', >+ title => 'Failed add', >+ publish => 0, >+ sortorder => 3, >+ content => "This page should not be added", > }); > >-my $result = Koha::CmsPages->child_links( undef ); >-is_deeply( $result, [ $page10, $page20 ]); >+is($test_page, undef, "Child page cannot be added under a page if their locations do not match"); > > $dbh->rollback(); > >diff --git a/tools/cmspages.pl b/tools/cmspages.pl >index 846f5e1..ccacc3a 100755 >--- a/tools/cmspages.pl >+++ b/tools/cmspages.pl >@@ -25,6 +25,7 @@ use C4::Output; > use C4::Languages qw( getTranslatedLanguages ); > use Koha::CmsPages; > use Koha::Libraries; >+use Koha::CmsPage; > > my $link_self = '/cgi-bin/koha/tools/cmspages.pl'; > >@@ -72,16 +73,33 @@ if ( $op eq 'form' ) { > op => 'add' > ); > } elsif ( $op eq 'add' ) { >- my $success = Koha::CmsPages->add( $cgi ); >+ my $success = Koha::CmsPages->add({ >+ id => scalar $cgi->param('id'), >+ parent => scalar $cgi->param('parent'), >+ location => scalar $cgi->param('disp'), >+ branchcode => scalar $cgi->param('branch'), >+ lang => scalar $cgi->param('lang'), >+ sortorder => scalar $cgi->param('number'), >+ title => scalar $cgi->param('title'), >+ title_link => scalar $cgi->param('title_link'), >+ publish => scalar $cgi->param('publish'), >+ content => scalar $cgi->param('content'), >+ }); > unless ($success) { > $template->param( failed_add => 1 ); > } > if ($success) { > print $cgi->redirect( $link_self ); > } >+} elsif ( $op eq 'delSingle' ) { >+ Koha::CmsPages->find( $id )->delete(); >+ print $cgi->redirect( $link_self ); > } elsif ( $op eq 'del' ) { > # param => multi_param; no list-ctx warnings >- Koha::CmsPages->remove( $cgi->multi_param( 'ids' ) ); >+ my @ids = $cgi->multi_param('ids'); >+ foreach (@ids){ >+ Koha::CmsPages->find($_)->delete; >+ } > print $cgi->redirect( $link_self ); > } elsif ( $op eq 'view' ) { > my $cms_data = Koha::CmsPages->page( $id ); >-- >2.1.4
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 15326
:
45621
|
58272
|
58607
|
58608
|
58609
|
58787
|
63904
|
63905
|
64241
|
64941
|
64942
|
66255
|
70866
|
70867
|
70868
|
70869
|
70870
|
70871
|
76387
|
76388
|
76389
|
76390
|
76391
|
76392
|
78232
|
78233
|
78234
|
78235
|
78236
|
78237
|
78238
|
78240
|
79269
|
83318
|
83319
|
83320
|
83321
|
83322
|
83323
|
83324
|
83325
|
83326
|
83404
|
83405
|
83406
|
83407
|
83408
|
83409
|
83410
|
83411
|
83412
|
84682
|
84683
|
84684
|
84685
|
84686
|
84687
|
84688
|
84689
|
84690
|
84691
|
87494
|
87495
|
87496
|
87497
|
87498
|
87499
|
87500
|
87501
|
87502
|
87503
|
87608
|
87609
|
87610
|
87617
|
96084
|
96085
|
96086
|
96087
|
96088
|
96089
|
96090
|
96091
|
96092
|
96093
|
96094
|
96095
|
96096
|
96097
|
96098
|
97237
|
101565
|
101566
|
101567
|
101568
|
101569
|
101570
|
101571
|
101572
|
101573
|
101574
|
101575
|
101576
|
101577
|
101578
|
101579
|
101580
|
128341
|
128342
|
128343
|
128344
|
128345
|
128346
|
128347
|
128348
|
128349
|
128350
|
128351
|
128352
|
128353
|
128354
|
128355
|
128356
|
128357
|
128585
|
128642
|
136432
|
136757
|
136768
|
138489
|
138493
|
139028
|
139029
|
139030
|
139031
|
139111
|
139123
|
139124
|
139125
|
139172
|
139173
|
139174