Koha-community/Koha
13
7
Fork 0
Code Issues Releases Wiki Activity

Bug 31383: (QA follow-up) Tidy

Browse source 
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
This commit is contained in:
Martin Renvoize 2023-09-21 17:40:47 +01:00 committed by Tomas Cohen Arazi
parent 9c5d09172d
commit 5bab527644
Signed by: tomascohen
GPG key ID: 0A272EA1B2F3C15F
5 changed files with 141 additions and 76 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
Koha/AdditionalContentsLocalization.pm
View file

@ -41,7 +41,7 @@ Return the Koha::AdditionalContent for this translated content.
=cut
sub additional_content {
my ( $self ) = @_;
my ($self) = @_;
my $rs = $self->_result->additional_content;
return Koha::AdditionalContent->_new_from_dbic($rs);
}

13
Koha/Schema/Result/AdditionalContentsLocalization.pm
View file

@ -159,9 +159,22 @@ __PACKAGE__->belongs_to(
# Created by DBIx::Class::Schema::Loader v0.07049 @ 2023-03-08 09:59:00
# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:7N/mHMqyPJJsYaA8V968wQ
=head2 koha_object_class
Koha Object class
=cut
sub koha_object_class {
'Koha::AdditionalContentsLocalization';
}
=head2 koha_objects_class
Koha Objects class
=cut
sub koha_objects_class {
'Koha::AdditionalContentsLocalizations';
}

87
installer/data/mysql/atomicupdate/bug_31383.pl
View file

@ -1,19 +1,22 @@
use Modern::Perl;
return {
bug_number => "31383",
bug_number => "31383",
description => "Split the additional_contents table",
up => sub {
up => sub {
my ($args) = @_;
my ($dbh, $out) = @$args{qw(dbh out)};
my ( $dbh, $out ) = @$args{qw(dbh out)};
unless ( TableExists('additional_contents_localizations') ) {
$dbh->do(q{
$dbh->do(
q{
ALTER TABLE additional_contents
CHANGE COLUMN idnew `id` int(10) unsigned NOT NULL AUTO_INCREMENT COMMENT 'unique identifier for the additional content category'
});
}
);
say $out "Renamed additional_contents.idnew with 'id'";
$dbh->do(q{
$dbh->do(
q{
CREATE TABLE `additional_contents_localizations` (
`id` int(10) unsigned NOT NULL AUTO_INCREMENT COMMENT 'unique identifier for the additional content',
`additional_content_id` int(10) unsigned NOT NULL COMMENT 'link to the additional content',
@ -25,24 +28,36 @@ return {
UNIQUE KEY `additional_contents_localizations_uniq` (`additional_content_id`,`lang`),
CONSTRAINT `additional_contents_localizations_ibfk1` FOREIGN KEY (`additional_content_id`) REFERENCES `additional_contents` (`id`) ON DELETE CASCADE ON UPDATE CASCADE
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;
});
}
);
say $out "Added new table 'additional_contents_localizations'";
my $contents = $dbh->selectall_arrayref(q{SELECT MIN(id) AS id, category, code, branchcode FROM additional_contents GROUP BY category, code, branchcode}, { Slice => {} });
my $sth_insert = $dbh->prepare(q{
my $contents = $dbh->selectall_arrayref(
q{SELECT MIN(id) AS id, category, code, branchcode FROM additional_contents GROUP BY category, code, branchcode},
{ Slice => {} }
);
my $sth_insert = $dbh->prepare(
q{
INSERT INTO additional_contents_localizations(additional_content_id, title, content, lang, updated_on)
VALUES(?, ?, ?, ?, ?)
});
for my $content ( @$contents ) {
}
);
for my $content (@$contents) {
my $q = q{
SELECT title, content, lang, branchcode, updated_on
FROM additional_contents
WHERE category=? AND code=? AND
};
$q .= defined $content->{branchcode} ? " branchcode = ?" : " branchcode IS NULL";
my $translated_contents = $dbh->selectall_arrayref($q, { Slice => {} }, $content->{category}, $content->{code}, defined $content->{branchcode} ? $content->{branchcode} : ());
for my $translated_content ( @$translated_contents ) {
$sth_insert->execute($content->{id}, $translated_content->{title}, $translated_content->{content}, $translated_content->{lang}, $translated_content->{updated_on});
my $translated_contents = $dbh->selectall_arrayref(
$q, { Slice => {} }, $content->{category}, $content->{code},
defined $content->{branchcode} ? $content->{branchcode} : ()
);
for my $translated_content (@$translated_contents) {
$sth_insert->execute(
$content->{id}, $translated_content->{title}, $translated_content->{content},
$translated_content->{lang}, $translated_content->{updated_on}
);
}
# Delete duplicates
@ -51,55 +66,67 @@ return {
WHERE category=? AND code=? AND id<>? AND
};
$q .= defined $content->{branchcode} ? " branchcode = ?" : " branchcode IS NULL";
$dbh->do($q, undef, $content->{category}, $content->{code}, $content->{id}, $content->{branchcode});
$dbh->do( $q, undef, $content->{category}, $content->{code}, $content->{id}, $content->{branchcode} );
}
$dbh->do(q{
$dbh->do(
q{
ALTER TABLE additional_contents
DROP INDEX additional_contents_uniq
});
$dbh->do(q{
}
);
$dbh->do(
q{
ALTER TABLE additional_contents
ADD UNIQUE KEY `additional_contents_uniq` (`category`,`code`,`branchcode`)
});
$dbh->do(q{
}
);
$dbh->do(
q{
ALTER TABLE additional_contents
DROP COLUMN title,
DROP COLUMN content,
DROP COLUMN lang
});
}
);
say $out "Removed 'title', 'content', 'lang' columns from additional_contents";
}
my $notice_templates = $dbh->selectall_arrayref(q{
my $notice_templates = $dbh->selectall_arrayref(
q{
SELECT id, module, code, content
FROM letter
WHERE content LIKE "%<news>%"
}, { Slice => {} });
for my $template ( @$notice_templates ) {
}, { Slice => {} }
);
for my $template (@$notice_templates) {
my $new_content = $template->{content};
$new_content =~ s|<news>|[% FOR content IN additional_contents %]|g;
$new_content =~ s|</news>|[% END %]|g;
$new_content =~ s{<<\s*additional_contents\.title\s*>>}{[% content.title | html %]}g;
$new_content =~ s{<<\s*additional_contents\.content\s*>>}{[% content.content | html %]}g;
$new_content =~ s{<<\s*additional_contents\.published_on\s*>>}{[% content.published_on | \$KohaDates %]}g;
$dbh->do(q{
$dbh->do(
q{
UPDATE letter
SET content = ?
WHERE id = ?
}, undef, $new_content, $template->{id});
}, undef, $new_content, $template->{id}
);
say $out "Adjusted notice template '" . $template->{code} . "'";
}
$notice_templates = $dbh->selectall_arrayref(q{
$notice_templates = $dbh->selectall_arrayref(
q{
SELECT id, code
FROM letter
WHERE content LIKE "%<<additional_content%" OR content LIKE "%<< additional_content%"
});
}
);
if ( @$notice_templates ) {
if (@$notice_templates) {
say $out "WARNING - Some notice templates need manual adjustments!";
for my $template ( @$notice_templates ) {
for my $template (@$notice_templates) {
say $out sprintf "\t %s (id=%s)", $template->{code}, $template->{id};
}
}

72
t/db_dependent/Koha/AdditionalContents.t
View file

@ -52,7 +52,7 @@ subtest 'Koha::AdditionalContents basic test' => sub {
content => 'content for news 1',
lang => 'default',
};
$new_news_item_1->translated_contents([$content_1]);
$new_news_item_1->translated_contents( [$content_1] );
my $new_news_item_2 = Koha::AdditionalContent->new(
{
category => 'news',
@ -66,14 +66,17 @@ subtest 'Koha::AdditionalContents basic test' => sub {
content => 'content for news 2',
lang => 'default',
};
$new_news_item_2->translated_contents([$content_2]);
$new_news_item_2->translated_contents( [$content_2] );
like( $new_news_item_1->id, qr|^\d+$|, 'Adding a new news_item should have set the id');
like( $new_news_item_1->id, qr|^\d+$|, 'Adding a new news_item should have set the id' );
is( Koha::AdditionalContents->search->count, $nb_of_news + 2, 'The 2 news should have been added' );
my $retrieved_news_item_1 = Koha::AdditionalContents->find( $new_news_item_1->id )->translated_contents->next;
is( $retrieved_news_item_1->title, $content_1->{title}, 'Find a news_item by id should return the correct news_item' );
is( $retrieved_news_item_1->content, $content_1->{content}, 'The content method return the content of the news');
is(
$retrieved_news_item_1->title, $content_1->{title},
'Find a news_item by id should return the correct news_item'
);
is( $retrieved_news_item_1->content, $content_1->{content}, 'The content method return the content of the news' );
my $default_content = $new_news_item_2->default_localization;
is( $default_content->content, $content_2->{content}, 'default_localization return the default content' );
@ -82,9 +85,15 @@ subtest 'Koha::AdditionalContents basic test' => sub {
$default_content = $new_news_item_2->default_localization;
is( $default_content->content, $content_2->{content}, 'default_localization still return the default content' );
my $retrieved_translated_content = $new_news_item_2->translated_content('en');
is( $retrieved_translated_content->content, $content_2->{content}, 'default content is returned for non-existing translated interface' );
is(
$retrieved_translated_content->content, $content_2->{content},
'default content is returned for non-existing translated interface'
);
$retrieved_translated_content = $new_news_item_2->translated_content('nl-NL');
is( $retrieved_translated_content->content, $translated_content->{content}, 'translated content is returned if it existsOB' );
is(
$retrieved_translated_content->content, $translated_content->{content},
'translated content is returned if it existsOB'
);
$new_news_item_1->delete;
is( Koha::AdditionalContents->search->count, $nb_of_news + 1, 'Delete should have deleted the news_item' );
@ -167,7 +176,7 @@ subtest '->author' => sub {
$author->delete;
$news_item = Koha::AdditionalContents->find($news_item->id);
$news_item = Koha::AdditionalContents->find( $news_item->id );
is( ref($news_item), 'Koha::AdditionalContent', 'News are not deleted alongwith the author' );
is( $news_item->author, undef, '->author returns undef is the author has been deleted' );
@ -199,7 +208,7 @@ subtest '->search_for_display' => sub {
number => 1,
}
});
$new_expired->translated_contents( [ { lang => 'default', content => ''} ] );
$new_expired->translated_contents( [ { lang => 'default', content => '' } ] );
my $new_not_expired = $builder->build_object({
class => 'Koha::AdditionalContents',
value => {
@ -283,27 +292,40 @@ subtest 'find_best_match' => sub {
plan tests => 3;
$schema->storage->txn_begin;
my $library01 = $builder->build_object({ class => 'Koha::Libraries' });
my $html01 = $builder->build_object({
class => 'Koha::AdditionalContents',
value => { category => 'html_customizations', location => 'test_best_match', branchcode => undef },
});
my ( $default_content ) = $html01->translated_contents( [ { lang => 'default', content => '' } ] )->as_list;
my $library01 = $builder->build_object( { class => 'Koha::Libraries' } );
my $html01 = $builder->build_object(
{
class => 'Koha::AdditionalContents',
value => { category => 'html_customizations', location => 'test_best_match', branchcode => undef },
}
);
my ($default_content) = $html01->translated_contents( [ { lang => 'default', content => '' } ] )->as_list;
my $params = { category => 'html_customizations', location => 'test_best_match', lang => 'nl-NL' };
is( Koha::AdditionalContents->find_best_match($params)->id, $default_content->id, 'Found all branches, lang default' );
is(
Koha::AdditionalContents->find_best_match($params)->id, $default_content->id,
'Found all branches, lang default'
);
my $html02 = $builder->build_object({
class => 'Koha::AdditionalContents',
value => { category => 'html_customizations', location => 'test_best_match', branchcode => undef },
});
my ( $translated_content ) = $html02->translated_contents( [ { lang => 'nl-NL', content => '' } ] )->as_list;
is( Koha::AdditionalContents->find_best_match($params)->id, $translated_content->id, 'Found all branches, lang nl-NL' );
my $html02 = $builder->build_object(
{
class => 'Koha::AdditionalContents',
value => { category => 'html_customizations', location => 'test_best_match', branchcode => undef },
}
);
my ($translated_content) = $html02->translated_contents( [ { lang => 'nl-NL', content => '' } ] )->as_list;
is(
Koha::AdditionalContents->find_best_match($params)->id, $translated_content->id,
'Found all branches, lang nl-NL'
);
$params->{ library_id } = $library01->id;
$params->{library_id} = $library01->id;
$html02->branchcode( $library01->id )->store;
is( Koha::AdditionalContents->find_best_match($params)->id, $translated_content->id, 'Found library01, lang nl-NL' );
is(
Koha::AdditionalContents->find_best_match($params)->id, $translated_content->id,
'Found library01, lang nl-NL'
);
# Note: find_best_match is tested further via $libary->opac_info; see t/db_dependent/Koha/Library.t
$schema->storage->txn_rollback;
}
}

43
tools/additional-contents.pl
View file

@ -77,7 +77,7 @@ if ( $op eq 'add_form' ) {
);
}
elsif ( $op eq 'add_validate' ) {
output_and_exit_if_error($cgi, $cookie, $template, { check => 'csrf_token' });
output_and_exit_if_error( $cgi, $cookie, $template, { check => 'csrf_token' } );
my $location = $cgi->param('location');
my $code = $cgi->param('code');
my $branchcode = $cgi->param('branchcode') || undef;
@ -93,15 +93,15 @@ elsif ( $op eq 'add_validate' ) {
sub {
my $additional_content;
my $params = {
location => $location,
branchcode => $branchcode,
expirationdate => $expirationdate,
published_on => $published_on,
number => $number,
borrowernumber => $borrowernumber,
};
location => $location,
branchcode => $branchcode,
expirationdate => $expirationdate,
published_on => $published_on,
number => $number,
borrowernumber => $borrowernumber,
};
if ( $id ) {
if ($id) {
$additional_content = Koha::AdditionalContents->find($id);
$additional_content->set($params)->store;
} else {
@ -117,9 +117,9 @@ elsif ( $op eq 'add_validate' ) {
unless ($code) {
$additional_content->discard_changes;
$code =
$category eq 'news'
? 'News_' . $additional_content->id
: $location . '_' . $additional_content->id;
$category eq 'news'
? 'News_' . $additional_content->id
: $location . '_' . $additional_content->id;
$additional_content->code($code)->store;
$id = $additional_content->id;
}
@ -142,16 +142,19 @@ elsif ( $op eq 'add_validate' ) {
};
my $existing_content = $existing_contents->find($id);
if ( $existing_content ) {
if ($existing_content) {
if ( $existing_content->title ne $title || $existing_content->content ne $content ) {
if ( C4::Context->preference("NewsLog") ) {
logaction('NEWS', 'MODIFY' , undef, sprintf("%s|%s|%s|%s", $code, $title, $lang, $content));
logaction(
'NEWS', 'MODIFY', undef,
sprintf( "%s|%s|%s|%s", $code, $title, $lang, $content )
);
}
} else {
$translated_content->{updated_on} = $existing_content->updated_on;
}
} elsif( C4::Context->preference("NewsLog") ) {
logaction('NEWS', 'ADD' , undef, sprintf("%s|%s|%s|%s", $code, $title, $lang, $content))
} elsif ( C4::Context->preference("NewsLog") ) {
logaction( 'NEWS', 'ADD', undef, sprintf( "%s|%s|%s|%s", $code, $title, $lang, $content ) );
}
push @translated_contents, $translated_content;
@ -159,10 +162,10 @@ elsif ( $op eq 'add_validate' ) {
if ( C4::Context->preference("NewsLog") ) {
my @existing_ids = $existing_contents->get_column('id');
my @deleted_ids = array_minus( @existing_ids, @seen_ids );
for my $id ( @deleted_ids ) {
my @deleted_ids = array_minus( @existing_ids, @seen_ids );
for my $id (@deleted_ids) {
my $c = $existing_contents->find($id);
logaction('NEWS', 'DELETE' , undef, sprintf("%s|%s|%s", $code, $c->lang, $c->content));
logaction( 'NEWS', 'DELETE', undef, sprintf( "%s|%s|%s", $code, $c->lang, $c->content ) );
}
}
@ -171,7 +174,7 @@ elsif ( $op eq 'add_validate' ) {
);
} catch {
warn $_;
if ( $id ) {
if ($id) {
push @messages, { type => 'error', code => 'error_on_update' };
} else {
push @messages, { type => 'error', code => 'error_on_insert' };