Bug 11876 - Add a diff view to staged MARC records
Summary: Add a diff view to staged MARC records
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: master
Hardware: All All
: P5 - low new feature (vote)
Assignee: Maxime Beaulieu
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-02-28 19:18 UTC by Frédérick Capovilla
Modified: 2015-06-04 23:33 UTC (History)
12 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Medium patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Add a diff view for staged MARC records. (8.12 KB, patch)
2014-02-28 19:18 UTC, Frédérick Capovilla
Details | Diff | Splinter Review
Bug 11876 - Add a diff view for staged MARC records. (6.70 KB, patch)
2014-08-07 17:14 UTC, Maxime Beaulieu
Details | Diff | Splinter Review
Bug 11876 - Ommited files in last patch (4.76 KB, patch)
2014-08-07 17:23 UTC, Maxime Beaulieu
Details | Diff | Splinter Review
Bug 11876 - Diff column is now displayed. (5.97 KB, patch)
2014-08-14 13:31 UTC, Maxime Beaulieu
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 11876 - Add a diff view for staged MARC records. (6.80 KB, patch)
2014-08-14 19:17 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 11876 - Ommited files in last patch (4.87 KB, patch)
2014-08-14 19:18 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 11876 - Diff column is now displayed. (6.21 KB, patch)
2014-08-14 19:18 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 11876 [Follow-up] Add a diff view to staged MARC Records (21.44 KB, patch)
2014-08-14 19:20 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 11876 [Follow-up] Add a diff view to staged MARC Records (21.43 KB, patch)
2014-09-23 12:35 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED OFF] Bug 11876 [Follow-up] Add a diff view to staged MARC Records (21.46 KB, patch)
2014-09-24 01:59 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 11876 - Add a diff view for staged MARC records. (6.16 KB, patch)
2014-09-25 12:02 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11876 - Ommited files in last patch (5.27 KB, patch)
2014-09-25 12:02 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11876 [Follow-up] Add a diff view to staged MARC Records (16.67 KB, patch)
2014-09-25 12:02 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11876: (qa-followup) some minor changes (5.23 KB, patch)
2014-09-25 12:02 UTC, Jonathan Druart
Details | Diff | Splinter Review
unit test added (2.66 KB, patch)
2014-10-30 16:42 UTC, simith.doliveira
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 11876 [Unit test] Add a diff view to staged MARC Records (2.82 KB, patch)
2014-10-30 18:45 UTC, Owen Leonard
Details | Diff | Splinter Review
[PASSED QA] Bug 11876 - Add a diff view for staged MARC records. (6.23 KB, patch)
2014-10-31 12:30 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 11876 - Ommited files in last patch (5.36 KB, patch)
2014-10-31 12:31 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 11876 [Follow-up] Add a diff view to staged MARC Records (16.74 KB, patch)
2014-10-31 12:31 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 11876: (qa-followup) some minor changes (5.30 KB, patch)
2014-10-31 12:31 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 11876 [Unit test] Add a diff view to staged MARC Records (2.85 KB, patch)
2014-10-31 12:31 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Frédérick Capovilla 2014-02-28 19:18:06 UTC Comment hidden (obsolete)
Comment 1 Maxime Beaulieu 2014-08-07 17:14:42 UTC Comment hidden (obsolete)
Comment 2 Maxime Beaulieu 2014-08-07 17:23:45 UTC Comment hidden (obsolete)
Comment 3 Owen Leonard 2014-08-12 17:09:19 UTC
I have tested an import which contains records which match existing records but I don't see a "diff" column in the table of staged records.
Comment 4 Maxime Beaulieu 2014-08-14 13:31:04 UTC Comment hidden (obsolete)
Comment 5 Owen Leonard 2014-08-14 19:17:54 UTC Comment hidden (obsolete)
Comment 6 Owen Leonard 2014-08-14 19:18:02 UTC Comment hidden (obsolete)
Comment 7 Owen Leonard 2014-08-14 19:18:16 UTC Comment hidden (obsolete)
Comment 8 Owen Leonard 2014-08-14 19:20:07 UTC Comment hidden (obsolete)
Comment 9 Nick Clemens 2014-09-19 19:41:59 UTC
Bug 11876 - Add a diff view to staged MARC Records.

30797 - [SIGNED-OFF] Bug 11876 - Add a diff view for staged MARC records.
30798 - [SIGNED-OFF] Bug 11876 - Ommited files in last patch
30799 - [SIGNED-OFF] Bug 11876 - Diff column is now displayed.
30800 - Bug 11876 [Follow-up] Add a diff view to staged MARC Records

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 11876 - Add a diff view for staged MARC records.
Applying: Bug 11876 - Ommited files in last patch
Applying: Bug 11876 - Diff column is now displayed.
Applying: Bug 11876 [Follow-up] Add a diff view to staged MARC Records
Using index info to reconstruct a base tree...
<stdin>:289: new blank line at EOF.
+
warning: 1 line adds whitespace errors.
Falling back to patching base and 3-way merge...
Removing koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/showdiffmarc.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/about.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/about.tt
Removing catalogue/showdiffmarc.pl
Failed to merge in the changes.
Patch failed at 0001 Bug 11876 [Follow-up] Add a diff view to staged MARC Records
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-11876-Follow-up-Add-a-diff-view-to-staged-MARC-iDw65M.patch
Comment 10 Owen Leonard 2014-09-23 12:35:02 UTC Comment hidden (obsolete)
Comment 11 Nick Clemens 2014-09-24 01:59:06 UTC Comment hidden (obsolete)
Comment 12 Jonathan Druart 2014-09-25 12:02:06 UTC Comment hidden (obsolete)
Comment 13 Jonathan Druart 2014-09-25 12:02:12 UTC Comment hidden (obsolete)
Comment 14 Jonathan Druart 2014-09-25 12:02:17 UTC Comment hidden (obsolete)
Comment 15 Jonathan Druart 2014-09-25 12:02:21 UTC Comment hidden (obsolete)
Comment 16 Jonathan Druart 2014-09-25 12:06:20 UTC
I tried to add readability to this patch set:
2 files were renamed and some code was added and removed later.
I squashed 2 patches and renamed the files at their creation.

I hope it's ok for everyone.

Maxime, I don't understand why you get the title of the first imported record.
What's happen if several records are imported?

If you think the logic is good, you have to provide unit tests for this new routine. Which will be quite easy todo, it only retrieve columns in DB.

It's the only concern I have on this enhancement.
Comment 17 Jonathan Druart 2014-09-29 07:55:47 UTC
I set the status to "Failed QA" (see my last note).
Comment 18 Owen Leonard 2014-10-22 15:38:13 UTC
I would be nice to see this make it into 3.18.
Comment 19 Katrin Fischer 2014-10-30 00:00:58 UTC
The feature freeze is set to Friday this week - Frederick, could take a look at comment 16 and provide a unit test?
Comment 20 simith.doliveira 2014-10-30 16:42:54 UTC Comment hidden (obsolete)
Comment 21 Owen Leonard 2014-10-30 18:45:34 UTC Comment hidden (obsolete)
Comment 22 Kyle M Hall 2014-10-31 12:30:25 UTC
Created attachment 33052 [details] [review]
[PASSED QA] Bug 11876 - Add a diff view for staged MARC records.

New patch should apply correctly.

	modified:   C4/ImportBatch.pm
	modified:   koha-tmpl/intranet-tmpl/prog/en/modules/tools/manage-marc-import.tt

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Bug 11876 - Diff column is now displayed.

If a match is found, a 'View' link appears in the Diff column.

Old patch was not implementing datatables, and therefore was not compatible with this version of Koha.
	modified:   koha-tmpl/intranet-tmpl/prog/en/modules/tools/manage-marc-import.tt
	modified:   tools/batch_records_ajax.pl

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

These patches have some issues but I'm going to submit a follow-up which
I think will address them all and offer some improvements as well.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 23 Kyle M Hall 2014-10-31 12:31:15 UTC
Created attachment 33053 [details] [review]
[PASSED QA] Bug 11876 - Ommited files in last patch

I forgot to include these files in the last patch

	new file:   tools/showdiffmarc.pl
	new file:   koha-tmpl/intranet-tmpl/prog/en/modules/tools/showdiffmarc.tt

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 24 Kyle M Hall 2014-10-31 12:31:18 UTC
Created attachment 33054 [details] [review]
[PASSED QA] Bug 11876 [Follow-up] Add a diff view to staged MARC Records

This follow up makes some corrections to the previously added files and
adds some functionality to the diff: A Javascript diff plugin highlights
the differences between the records in the two columns.

Corrections made: Converted tab indentation to spaces, corrected GPL
statement.

File organization: Moved showdiffmarc.pl and .tt to /tools/ to match the
location of the page with which it functions,
tools/manage-marc-import.pl. Corrected permissions on showdiffmarc.pl
accordingly.

Updates to the template: Added standard includes inclucing header menu
and breadcrumbs; converted custom layout to YUI Grid standard.

To test, follow the test plan previously defined:

- Stage a MARC record batch which contains at least one record match for
  something already in your catalog.
- Locate the staged MARC record batch in Tools -> Manage staged records
  and click to view the contents.
- Find the record which matched an existing record and click the "View"
  link in the Diff column.
- The compare screen should include the header menu and breadcrumbs. The
  differences between your staged file and the existing record should be
  higlighted.
- You should be able to return to the MARC batch you were previously
  viewing by following the link in the breadcrumbs or the link at the
  bottom of the page.
- Confirm that the "About" page includes information about the new
  JavaScript plugin on the Licenses tab.

Signed-off-by: Nick Clemens <nick@quecheelibrary.org>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 25 Kyle M Hall 2014-10-31 12:31:21 UTC
Created attachment 33055 [details] [review]
[PASSED QA] Bug 11876: (qa-followup) some minor changes

- GetImportBiblios: prepare the query is useless here
- use Modern::Perl
- Remove useless indentation level in pl file and redirect to 404 if
  required params are not given.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 26 Kyle M Hall 2014-10-31 12:31:24 UTC
Created attachment 33056 [details] [review]
[PASSED QA] Bug 11876 [Unit test] Add a diff view to staged MARC Records

Unit test added

http://bugs.koha-community.org/show_bug.cgi?id=11876
Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 27 Tomás Cohen Arazi 2014-10-31 17:55:54 UTC
Patches pushed to master.

Nice feature!
Thanks Fréférik and Maxime!