https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18823
Katrin Fischer <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |[email protected] Status|Signed Off |Failed QA --- Comment #89 from Katrin Fischer <[email protected]> --- 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. _______________________________________________ Koha-bugs mailing list [email protected] http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
