[Koha-bugs] [Bug 10662] Build OAI-PMH Harvesting Client

bugzilla-daemon at bugs.koha-community.org bugzilla-daemon at bugs.koha-community.org
Tue Sep 18 15:01:31 CEST 2018


https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10662

--- Comment #201 from Josef Moravec <josef.moravec at gmail.com> ---
Comment on attachment 79039
  --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=79039
Bug 10662 - Build OAI-PMH Harvesting Client

Review of attachment 79039:
 --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=10662&attachment=79039)
-----------------------------------------------------------------

Hi David, 
thanks for your great job!

There are still some issues which I think should be solved to make this part of
master

::: Koha/Daemon.pm
@@ +1,2 @@
> +package Koha::Daemon;
> +

there is missing POD in this package

::: Koha/OAI/Harvester.pm
@@ +1,2 @@
> +package Koha::OAI::Harvester;
> +

there is missing POD for some methods in this package

@@ +29,5 @@
> +use DateTime;
> +use DateTime::Format::Strptime;
> +
> +use C4::Context;
> +use Koha::Database;

You should use Koha::Object[s] based classes not Koha::Database itself

@@ +114,5 @@
> +            my $active_tasks = $poe_kernel->call("harvester","list_tasks","active");
> +            my @active_uuids = map { $_->{uuid} } @$active_tasks;
> +
> +            my $schema = Koha::Database->new()->schema();
> +            my $rs = $schema->resultset('OaiHarvesterImportQueue')->search({

You should create and use class based on Koha::Object - could be something like
Koha::OAI::Harvester::ImportQueue[s] in this case

@@ +311,5 @@
> +sub reset_imports_status {
> +    my ($self, $kernel, $heap, $session) = @_[OBJECT, KERNEL,HEAP,SESSION];
> +
> +    my $schema = Koha::Database->new()->schema();
> +    my $rs = $schema->resultset('OaiHarvesterImportQueue')->search({

Use Koha::Object[s]

@@ +421,5 @@
> +    my ($self,$uuid) = @_;
> +    my $count = undef;
> +    if ($uuid){
> +        my $schema = Koha::Database->new()->schema();
> +        my $items = $schema->resultset('OaiHarvesterImportQueue')->search({

Use Koha::Object[s]

@@ +443,5 @@
> +    my $schema = Koha::Database->new()->schema();
> +    my @tasks = ();
> +    foreach my $uuid (sort keys %{$heap->{tasks}}){
> +        my $task = $heap->{tasks}->{$uuid};
> +        my $items = $schema->resultset('OaiHarvesterImportQueue')->search({

Use Koha::Object[s]

@@ +594,5 @@
> +        }
> +
> +        #Step Three: stop pending imports for this task
> +        my $schema = Koha::Database->new()->schema();
> +        my $items = $schema->resultset('OaiHarvesterImportQueue')->search({

Use Koha::Object[s]

@@ +625,5 @@
> +        $kernel->call($session,"stop_task",$task_uuid);
> +
> +        #Step Two: delete pending imports in database
> +        my $schema = Koha::Database->new()->schema();
> +        my $items = $schema->resultset('OaiHarvesterImportQueue')->search({

Use Koha::Object[s]

::: Koha/OAI/Harvester/Client.pm
@@ +1,2 @@
> +package Koha::OAI::Harvester::Client;
> +

There is missing POD in this package

::: Koha/OAI/Harvester/Downloader.pm
@@ +95,5 @@
> +        return;
> +    }
> +}
> +
> +=head2 OpenXMLStream

the method is called "GetXMLStream"

@@ +167,5 @@
> +        return;
> +    }
> +}
> +
> +sub ParseXMLStream {

Missing POD

@@ +244,5 @@
> +        warn "ParseXMLStream() requires a 'file_handle' argument.";
> +    }
> +}
> +
> +sub harvest {

missing POD

::: Koha/OAI/Harvester/Import/MARCXML.pm
@@ +1,1 @@
> +package Koha::OAI::Harvester::Import::MARCXML;

There is missing pod in this package

::: Koha/OAI/Harvester/Import/Record.pm
@@ +26,5 @@
> +
> +use C4::Context;
> +use C4::Biblio;
> +
> +use Koha::Database;

use Koha::Object[s] based classes please

@@ +164,5 @@
> +    my ($self, $args) = @_;
> +    my $record_type = $args->{record_type} // "biblio";
> +    my $link_id;
> +    if ($record_type eq "biblio"){
> +        my $link = $schema->resultset('OaiHarvesterBiblio')->find(

Use Koha::Object[s]

::: Koha/OAI/Harvester/Listener.pm
@@ +1,1 @@
> +package Koha::OAI::Harvester::Listener;

Missing POD in this package

::: Koha/OAI/Harvester/Request.pm
@@ +46,5 @@
> +sub _type {
> +    return 'OaiHarvesterRequest';
> +}
> +
> +sub validate {

Please add POD for this sub

::: Koha/OAI/Harvester/Worker.pm
@@ +1,1 @@
> +package Koha::OAI::Harvester::Worker;

There is missing POD in this package

::: Koha/OAI/Harvester/Worker/Download/Stream.pm
@@ +1,1 @@
> +package Koha::OAI::Harvester::Worker::Download::Stream;

There is missing POD in this package

::: Koha/OAI/Harvester/Worker/Import.pm
@@ +1,1 @@
> +package Koha::OAI::Harvester::Worker::Import;

There is missing POD in this package

::: installer/data/mysql/atomicupdate/bug_10662.sql
@@ +10,5 @@
> +  PRIMARY KEY (`import_oai_biblio_id`),
> +  UNIQUE KEY `oai_record` (`oai_identifier`,`oai_repository`) USING BTREE,
> +  KEY `FK_import_oai_biblio_1` (`biblionumber`),
> +  CONSTRAINT `FK_import_oai_biblio_1` FOREIGN KEY (`biblionumber`) REFERENCES `biblio` (`biblionumber`) ON DELETE CASCADE ON UPDATE NO ACTION
> +) ENGINE=InnoDB AUTO_INCREMENT=1 DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;

CHARSET should be utf8mb4 and collation utf8mb4_unicode_ci - for all tables

:::
koha-tmpl/intranet-tmpl/prog/en/modules/tools/oai-pmh-harvester/dashboard.tt
@@ +3,5 @@
> +[% INCLUDE 'doc-head-close.inc' %]
> +[% INCLUDE 'datatables.inc' %]
> +[% dashboard_page = '/cgi-bin/koha/tools/oai-pmh-harvester/dashboard.pl' %]
> +[% request_page = '/cgi-bin/koha/tools/oai-pmh-harvester/request.pl' %]
> +<script type="text/javascript">

Javascript should be at end of page - see bug 17858

::: koha-tmpl/intranet-tmpl/prog/en/modules/tools/oai-pmh-harvester/request.tt
@@ +1,5 @@
> +[% INCLUDE 'doc-head-open.inc' %]
> +<title>Koha › Tools › OAI-PMH harvester › Request</title>
> +[% INCLUDE 'doc-head-close.inc' %]
> +[% INCLUDE 'calendar.inc' %]
> +<script type="text/javascript" src="[% interface %]/lib/jquery/plugins/jquery-ui-timepicker-addon.min.js"></script>

Use assets for external js and css - see
https://wiki.koha-community.org/wiki/Coding_Guidelines#HTML8:_use_Asset_TT_plugin_for_linking_javascript_and_css_files

@@ +3,5 @@
> +[% INCLUDE 'doc-head-close.inc' %]
> +[% INCLUDE 'calendar.inc' %]
> +<script type="text/javascript" src="[% interface %]/lib/jquery/plugins/jquery-ui-timepicker-addon.min.js"></script>
> +[% INCLUDE 'timepicker.inc' %]
> +<script type="text/javascript">

Javascript should be at end of file

::: rewrite-config.PL
@@ +152,3 @@
>    "__MEMCACHED_NAMESPACE__" => "",
>    "__FONT_DIR__" => "/usr/share/fonts/truetype/ttf-dejavu",
>    "__TEMPLATE_CACHE_DIR__" => "/tmp/koha"

there is missing colon at the end of line

::: svc/oai-pmh-harvester/history
@@ +87,5 @@
> +    }
> +}
> +
> +my $page = ( $start / $length ) + 1;
> +my $schema = Koha::Database->new()->schema();

Please use Koha::Objects

::: tools/oai-pmh-harvester/record.pl
@@ +36,5 @@
> +my $import_oai_id = $input->param('import_oai_id');
> +if ($import_oai_id){
> +    my $schema = Koha::Database->new()->schema();
> +    if ($schema){
> +        my $rs = $schema->resultset("OaiHarvesterHistory");

Please use Koha::Object[s]

-- 
You are receiving this mail because:
You are watching all bug changes.


More information about the Koha-bugs mailing list