Bug 19495 - Automatic report conversion needs to do global replace on 'biblioitems' and 'marcxml'
Summary: Automatic report conversion needs to do global replace on 'biblioitems' and '...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Reports (show other bugs)
Version: master
Hardware: All All
: P5 - low blocker (vote)
Assignee: Kyle M Hall
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 17898
Blocks:
  Show dependency treegraph
 
Reported: 2017-10-19 18:37 UTC by Barton Chittenden
Modified: 2019-06-27 09:24 UTC (History)
6 users (show)

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


Attachments
Bug 19495 - Automatic report conversion needs to do global replace on 'biblioitems' and 'marcxml' (2.60 KB, patch)
2017-10-20 14:50 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19495 - Automatic report conversion needs to do global replace on 'biblioitems' and 'marcxml' (2.68 KB, patch)
2017-10-20 14:52 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19495 - Automatic report conversion needs to do global replace on 'biblioitems' and 'marcxml' (2.86 KB, patch)
2017-10-20 18:28 UTC, Dominic Pichette
Details | Diff | Splinter Review
Bug 19495 - Automatic report conversion needs to do global replace on 'biblioitems' and 'marcxml' (2.96 KB, patch)
2017-10-24 13:05 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Barton Chittenden 2017-10-19 18:37:39 UTC
Bug 17898 provides a way of converting reports that use biblioitems.marcxml so that they will use biblio_metadata.metadata instead.

This only works with reports that do not refer to other columns in the biblioitems table. This is a known limitation. It means that we should be able to do a substitution of every occurrence of biblioitems with biblio_metadata, and every occurrence of marcxml with metadata.

Unfortunately, we're not doing a global replace, we're only replacing the first occurrence. Here's the original:

SELECT
    biblio.title,
    biblio.biblionumber,
    A.barcode,ExtractValue(biblioitems.marcxml,'//controlfield[@tag="001"]') AS "field_001",
    A.itemcallnumber,
    A.itype
FROM
    items A
    JOIN biblio on (A.biblionumber = biblio.biblionumber)
    JOIN biblioitems on (A.biblionumber = biblioitems.biblionumber)
WHERE
    withdrawn = 1 AND A.biblionumber NOT in (
        SELECT
            biblionumber
        FROM
            items B
        WHERE
            B.itemnumber != A.itemnumber
    )

And here's the converted:

SELECT
    biblio.title,
    biblio.biblionumber,
    A.barcode,ExtractValue(biblio_metadata.metadata,'//controlfield[@tag="001"]') AS "field_001",
    A.itemcallnumber,
    A.itype 
FROM
    items A
    JOIN biblio on (A.biblionumber = biblio.biblionumber)
    JOIN biblioitems on (A.biblionumber = biblioitems.biblionumber) 
WHERE
    withdrawn = 1 AND A.biblionumber NOT in (
        SELECT
            biblionumber 
        FROM
            items B 
        WHERE
            B.itemnumber != A.itemnumber
    )

Note that there are three occurrences of 'biblioitems' in the first query, but only the first has been substituted in the second.
Comment 1 Kyle M Hall 2017-10-20 14:50:44 UTC
Created attachment 68323 [details] [review]
Bug 19495 - Automatic report conversion needs to do global replace on 'biblioitems' and 'marcxml'

Bug 17898 provides a way of converting reports that use biblioitems.marcxml so that they will use biblio_metadata.metadata instead.

This only works with reports that do not refer to other columns in the biblioitems table. This is a known limitation. It means that we should be able to do a substitution of every occurrence of biblioitems with biblio_metadata, and every occurrence of marcxml with metadata.

Unfortunately, we're not doing a global replace, we're only replacing the first occurrence.

Test Plan:
1) Apply this patch
2) prove t/db_dependent/Reports/Guided.t
Comment 2 Kyle M Hall 2017-10-20 14:51:30 UTC
Updating importance to blocker because we should *really* get this fixed before release.
Comment 3 Kyle M Hall 2017-10-20 14:52:55 UTC
Created attachment 68324 [details] [review]
Bug 19495 - Automatic report conversion needs to do global replace on 'biblioitems' and 'marcxml'
Comment 4 Dominic Pichette 2017-10-20 18:28:58 UTC
Created attachment 68329 [details] [review]
Bug 19495 - Automatic report conversion needs to do global replace on 'biblioitems' and 'marcxml'

Bug 17898 provides a way of converting reports that use biblioitems.marcxml so that they will use biblio_metadata.metadata instead.

This only works with reports that do not refer to other columns in the biblioitems table. This is a known limitation. It means that we should be able to do a substitution of every occurrence of biblioitems with biblio_metadata, and every occurrence of marcxml with metadata.

Unfortunately, we're not doing a global replace, we're only replacing the first occurrence.

Test Plan:
1) Apply this patch
2) prove t/db_dependent/Reports/Guided.t

All tests successful.
Files=1, Tests=9, 10 wallclock secs ( 0.11 usr  0.01 sys +  2.85 cusr  0.25 csys =  3.22 CPU)
Result: PASS

Signed-off-by: Dominic Pichette <dominic@inlibro.com>
Comment 5 Marcel de Rooy 2017-10-24 13:05:53 UTC
Created attachment 68463 [details] [review]
Bug 19495 - Automatic report conversion needs to do global replace on 'biblioitems' and 'marcxml'

Bug 17898 provides a way of converting reports that use biblioitems.marcxml so that they will use biblio_metadata.metadata instead.

This only works with reports that do not refer to other columns in the biblioitems table. This is a known limitation. It means that we should be able to do a substitution of every occurrence of biblioitems with biblio_metadata, and every occurrence of marcxml with metadata.

Unfortunately, we're not doing a global replace, we're only replacing the first occurrence.

Test Plan:
1) Apply this patch
2) prove t/db_dependent/Reports/Guided.t

All tests successful.
Files=1, Tests=9, 10 wallclock secs ( 0.11 usr  0.01 sys +  2.85 cusr  0.25 csys =  3.22 CPU)
Result: PASS

Signed-off-by: Dominic Pichette <dominic@inlibro.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 6 Marcel de Rooy 2017-10-24 13:07:12 UTC
Would not mark this one as a blocker btw
Comment 7 Jonathan Druart 2017-10-25 15:19:47 UTC
Pushed to master for 17.11, thanks to everybody involved!
Comment 8 Fridolin Somers 2017-11-20 13:05:06 UTC
Pushed to 17.05.x, will be in 17.05.06
Comment 9 Katrin Fischer 2017-11-20 21:24:04 UTC
Dependency is not in 16.11.x - patch is not needed.