[Koha-bugs] [Bug 18823] Advanced editor - Rancor - add ability to edit records in import batches

[bugzilla-daemon at bugs.koha-community.org]

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

Katrin Fischer <katrin.fischer at bsz-bw.de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |katrin.fischer at bsz-bw.de
             Status|Signed Off                  |Failed QA

--- Comment #89 from Katrin Fischer <katrin.fischer at bsz-bw.de> ---
Starting with a first code review and the tests. All in all this is quite a
MASSSIVE patch.
12 files changed, 992 insertions(+), 156 deletions(-)
 I can't shake the feeling that this should have been broken up into smaller
patches in a dependency tree.

1) QA test tools

a) FAIL koha-tmpl/intranet-tmpl/prog/en/includes/cateditor-ui.inc
   FAIL   filters
                missing_filter at line 807 (                   
Preferences.Save( [% loggelogged_in_user.borrowernumber %] );)
                missing_filter at line 45 (                name: _("Batch: ") +
`[% batch.file_name %]`,)
                missing_filter at line 44 (            'batch:[%
batch.import_batch_id %]': {)
                missing_filter at line 327 (            [%
batch.import_batch_id %]: {)
                missing_filter at line 328 (                'name': `[%
batch.file_name %]`,)

b)  FAIL        C4/Biblio.pm
   FAIL   pod coverage
                POD is missing for 'UpdateMarcTimestamp'

2) Unit tests

There are no unit tests included in this patch, but it touches a lot of
methods:

a) New method C4/Biblio Upate005Time is untested - I can see it's just a copy
of existing code tho.
b) New method C4/ImportBatch _get_import_record_timestamp is untested and
should have POD.
c) Changes to existing methods are not accompanied by tests:
  - _update_import_record_marc
  - _add_biblio_fields
c) Changes to Koha/MetaSearcher are untested.
d) New svc/cataloguing/import_batches is untested.
e) New svc/cataloguing/metasearch

3) Question
+use C4::Koha qw(); # Purely for GetNormalizedISBN
Why not list the method inside qw if only GetNormalizedISBN is needed?

4) Can you point me to the most current and complete test plan? (including
Controlnumber handling etc.)

5) Some first notes from GUI testing

a) I am not completely sold on the placement of the 'Save to' box. It already
looks a bit 'squashed' and it will look worse in other languages (being German
I know our words for things tend to be longer)

b) Main issue: Should we not preselect something or at least remember what was
selected? I think 'Save to catalog' would probably the most common for normal
cataloguing. 

Once saved for the first time, both: 'new record' and 'catalog record #' are
checked. I'd expect it to create a duplicate in that case. (blocker for me -
please check)

c) When a record is saved, the message appears no longer centered, but far to
the right. I think it's easier to miss this way.

d) The modal for 'Import batches...' covers the whole screen. Changing
something there will take immediate effect. I wonder if a close button would
help usability or just making the modal a little smaller.

e) Why is it 'Save as MARC' but 'New MARCXML'?

e) When checking both, only the MARCXML is saved. I think maybe multi-select
should not allowed or should be handled?

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