Bug 17897 - Koha::SearchMappingManager - Simplify search mapping modifications
Summary: Koha::SearchMappingManager - Simplify search mapping modifications
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Emmi Takkinen
QA Contact: Testopia
URL:
Keywords:
Depends on: 17885
Blocks:
  Show dependency treegraph
 
Reported: 2017-01-13 10:50 UTC by Olli-Antti Kivilahti
Modified: 2023-11-06 20:33 UTC (History)
5 users (show)

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


Attachments
Bug 17897 - Koha::SearchMappingManager - Simplify search mapping modifications (13.85 KB, patch)
2017-01-13 10:53 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 17897 - (follow-up) Cleanup after tests (3.18 KB, patch)
2017-08-01 12:28 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 17897: Koha::SearchMappingManager - Simplify search mapping modifications (14.20 KB, patch)
2020-04-07 07:36 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 17897: (follow-up) Cleanup after tests (3.19 KB, patch)
2020-04-07 07:37 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 17897: Koha::SearchMappingManager - Simplify search mapping modifications (14.29 KB, patch)
2021-01-19 13:54 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 17897: (follow-up) Cleanup after tests (3.25 KB, patch)
2021-01-19 13:55 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 17897: Fix failing tests (2.70 KB, patch)
2021-01-19 13:56 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 17897: Fix failing tests (3.50 KB, patch)
2022-11-11 08:09 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 17897: Fix failing tests and QA issues (7.65 KB, patch)
2023-10-17 10:52 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 17897: Koha::SearchMappingManager - Simplify search mapping modifications (14.37 KB, patch)
2023-10-17 19:33 UTC, David Nind
Details | Diff | Splinter Review
Bug 17897: (follow-up) Cleanup after tests (3.29 KB, patch)
2023-10-17 19:33 UTC, David Nind
Details | Diff | Splinter Review
Bug 17897: Fix failing tests and QA issues (7.69 KB, patch)
2023-10-17 19:33 UTC, David Nind
Details | Diff | Splinter Review
Bug 17897: Koha::SearchMappingManager - Simplify search mapping modifications (14.40 KB, patch)
2023-11-06 06:07 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 17897: (follow-up) Cleanup after tests (3.25 KB, patch)
2023-11-06 06:08 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 17897: Koha::SearchMappingManager - Simplify search mapping modifications (14.45 KB, patch)
2023-11-06 06:13 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 17897: (follow-up) Cleanup after tests (3.29 KB, patch)
2023-11-06 06:13 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 17897: Fix failing tests and QA issues (7.70 KB, patch)
2023-11-06 06:13 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 17897: Add search_marc_to_field.filter to SearhMappingManager (5.75 KB, patch)
2023-11-06 06:36 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 17897: Koha::SearchMappingManager - Simplify search mapping modifications (14.44 KB, patch)
2023-11-06 20:33 UTC, David Nind
Details | Diff | Splinter Review
Bug 17897: (follow-up) Cleanup after tests (3.29 KB, patch)
2023-11-06 20:33 UTC, David Nind
Details | Diff | Splinter Review
Bug 17897: Fix failing tests and QA issues (7.69 KB, patch)
2023-11-06 20:33 UTC, David Nind
Details | Diff | Splinter Review
Bug 17897: Add search_marc_to_field.filter to SearhMappingManager (5.79 KB, patch)
2023-11-06 20:33 UTC, David Nind
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Olli-Antti Kivilahti 2017-01-13 10:50:59 UTC
Currently there is a lot of DBIC-magic everywhere where search mappings (search_fields, search_marc_to_field, search_marc_map -tables) are CRUD:ed.

Encapsulate this hard-to-read DBIC-magic into a Manager class which promotes code-reusability and thus, simpler test case writing.

Includes tests for Bug 17885
Comment 1 Olli-Antti Kivilahti 2017-01-13 10:53:03 UTC
Created attachment 58942 [details] [review]
Bug 17897 - Koha::SearchMappingManager - Simplify search mapping modifications

Currently there is a lot of DBIC-magic everywhere where search mappings
(search_fields, search_marc_to_field, search_marc_map -tables) are CRUD:ed.

Encapsulate this hard-to-read DBIC-magic into a Manager class which promotes
code-reusability and thus, simpler test case writing.

Includes tests for Buug 17885
Comment 2 Olli-Antti Kivilahti 2017-01-13 10:54:08 UTC
Test plan is to run the elasticsearch-dependent tests in t/db_dependent/Koha*
Comment 3 Lari Taskula 2017-08-01 12:28:56 UTC
Created attachment 65392 [details] [review]
Bug 17897 - (follow-up) Cleanup after tests

Adds rollback to test data in t/db_dependent/Koha/SearchEngine/ElasticSearch.t

To test:
1. prove t/db_dependent/Koha/SearchEngine/ElasticSearch.t
Comment 4 Nick Clemens 2018-03-12 14:47:33 UTC
fatal: sha1 information is lacking or useless (Koha/SearchEngine/Elasticsearch.pm).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Comment 5 Emmi Takkinen 2020-04-07 07:36:08 UTC
Created attachment 102475 [details] [review]
Bug 17897: Koha::SearchMappingManager - Simplify search mapping modifications

Rebased patch. Added 'search' to 'get_search_mappings' and adjusted tests to work with changed mappings.
Comment 6 Emmi Takkinen 2020-04-07 07:37:03 UTC
Created attachment 102476 [details] [review]
Bug 17897: (follow-up) Cleanup after tests
Comment 7 Victoria Faafia 2021-01-18 23:10:17 UTC
git bz apply 17897
Bug 17897 - Koha::SearchMappingManager - Simplify search mapping modifications

102475 - Bug 17897: Koha::SearchMappingManager - Simplify search mapping modifications
102476 - Bug 17897: (follow-up) Cleanup after tests

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 17897: Koha::SearchMappingManager - Simplify search mapping modifications
Using index info to reconstruct a base tree...
M	Koha/SearchEngine/Elasticsearch.pm
Falling back to patching base and 3-way merge...
Auto-merging Koha/SearchEngine/Elasticsearch.pm
CONFLICT (content): Merge conflict in Koha/SearchEngine/Elasticsearch.pm
error: Failed to merge in the changes.
Patch failed at 0001 Bug 17897: Koha::SearchMappingManager - Simplify search mapping modifications
The copy of the patch that failed is found in: .git/rebase-apply/patch
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-17897-KohaSearchMappingManager---Simplify-sear-JiTslW.patch

patch does not apply
Comment 8 Emmi Takkinen 2021-01-19 13:54:59 UTC
Created attachment 115320 [details] [review]
Bug 17897: Koha::SearchMappingManager - Simplify search mapping modifications

Currently there is a lot of DBIC-magic everywhere where search mappings
(search_fields, search_marc_to_field, search_marc_map -tables) are CRUD:ed.

Encapsulate this hard-to-read DBIC-magic into a Manager class which promotes
code-reusability and thus, simpler test case writing.

Includes tests for Buug 17885

Rebased-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Comment 9 Emmi Takkinen 2021-01-19 13:55:48 UTC
Created attachment 115321 [details] [review]
Bug 17897: (follow-up) Cleanup after tests

Adds rollback to test data in t/db_dependent/Koha/SearchEngine/ElasticSearch.t

To test:
1. prove t/db_dependent/Koha/SearchEngine/ElasticSearch.t

Rebased-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Comment 10 Emmi Takkinen 2021-01-19 13:56:20 UTC
Created attachment 115322 [details] [review]
Bug 17897: Fix failing tests

Tests failed when txn_begin and rollback were used inside
of subtests.

To test prove t/db_dependent/Koha/SearchEngine/ElasticSearch.t
Comment 11 David Nind 2022-07-08 13:06:01 UTC
Patch still applies, but tests fail:

 prove -vvv t/db_dependent/Koha/SearchEngine/ElasticSearch.t
t/db_dependent/Koha/SearchEngine/ElasticSearch.t .. 
# Subtest: Reset Elasticsearch mappings
    ok 1 - Scenario: Reset Elasticsearch mappings to an empty database
    ok 2 - Given empty search mapping tables
    ok 3 - When reset_elasticsearch_mappings() has been ran
    ok 4 - Then search mapping tables have been populated
    ok 5 - Scenario: Reset Elasticsearch mappings when custom mappings already exist
    1..5
ok 1 - Reset Elasticsearch mappings
DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Data truncated for column 'type' at row 1 at /kohadevbox/koha/Koha/Objects.pm line 114
# Tests were run but no plan was declared and done_testing() was not seen.
# Looks like your test exited with 11 just after 1.
Dubious, test returned 11 (wstat 2816, 0xb00)
All 1 subtests passed 

Test Summary Report
-------------------
t/db_dependent/Koha/SearchEngine/ElasticSearch.t (Wstat: 2816 Tests: 1 Failed: 0)
  Non-zero exit status: 11
  Parse errors: No plan found in TAP output
Files=1, Tests=1,  4 wallclock secs ( 0.03 usr  0.00 sys +  3.53 cusr  0.19 csys =  3.75 CPU)
Result: FAIL
Comment 12 Emmi Takkinen 2022-11-11 08:09:09 UTC
Created attachment 143744 [details] [review]
Bug 17897: Fix failing tests

Tests failed when txn_begin and rollback were used inside
of subtests.

To test prove t/db_dependent/Koha/SearchEngine/ElasticSearch.t
Comment 13 Emmi Takkinen 2022-11-11 08:10:30 UTC
Fixed tests, patches should apply cleanly.
Comment 14 David Nind 2022-11-11 10:45:22 UTC
Tests are still failing for me:

prove -vvv t/db_dependent/Koha/SearchEngine/ElasticSearch.t
t/db_dependent/Koha/SearchEngine/ElasticSearch.t .. 
# Subtest: Reset Elasticsearch mappings
    ok 1 - Scenario: Reset Elasticsearch mappings to an empty database
    ok 2 - Given empty search mapping tables
    ok 3 - When reset_elasticsearch_mappings() has been ran
    ok 4 - Then search mapping tables have been populated
    ok 5 - Scenario: Reset Elasticsearch mappings when custom mappings already exist
    1..5
ok 1 - Reset Elasticsearch mappings
DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Data truncated for column 'type' at row 1 at /kohadevbox/koha/Koha/Objects.pm line 114
# Tests were run but no plan was declared and done_testing() was not seen.
# Looks like your test exited with 11 just after 1.
Dubious, test returned 11 (wstat 2816, 0xb00)
All 1 subtests passed 

Test Summary Report
-------------------
t/db_dependent/Koha/SearchEngine/ElasticSearch.t (Wstat: 2816 Tests: 1 Failed: 0)
  Non-zero exit status: 11
  Parse errors: No plan found in TAP output
Files=1, Tests=1,  4 wallclock secs ( 0.02 usr  0.00 sys +  3.65 cusr  0.15 csys =  3.82 CPU)
Result: FAIL
Comment 15 Emmi Takkinen 2023-10-17 09:57:18 UTC
I just realized tests also lack "tests", "plan tests" etc. and subtests aren't declared inside sub. Also, should test file be named something else? It's now ElasticSearch.t but we also have Elasticsearch.t.
Comment 16 Emmi Takkinen 2023-10-17 10:27:07 UTC
(In reply to David Nind from comment #14)
> Tests are still failing for me:
> 
> prove -vvv t/db_dependent/Koha/SearchEngine/ElasticSearch.t
> t/db_dependent/Koha/SearchEngine/ElasticSearch.t .. 
> # Subtest: Reset Elasticsearch mappings
>     ok 1 - Scenario: Reset Elasticsearch mappings to an empty database
>     ok 2 - Given empty search mapping tables
>     ok 3 - When reset_elasticsearch_mappings() has been ran
>     ok 4 - Then search mapping tables have been populated
>     ok 5 - Scenario: Reset Elasticsearch mappings when custom mappings
> already exist
>     1..5
> ok 1 - Reset Elasticsearch mappings
> DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st
> execute failed: Data truncated for column 'type' at row 1 at
> /kohadevbox/koha/Koha/Objects.pm line 114
> # Tests were run but no plan was declared and done_testing() was not seen.
> # Looks like your test exited with 11 just after 1.
> Dubious, test returned 11 (wstat 2816, 0xb00)
> All 1 subtests passed 
> 
> Test Summary Report
> -------------------
> t/db_dependent/Koha/SearchEngine/ElasticSearch.t (Wstat: 2816 Tests: 1
> Failed: 0)
>   Non-zero exit status: 11
>   Parse errors: No plan found in TAP output
> Files=1, Tests=1,  4 wallclock secs ( 0.02 usr  0.00 sys +  3.65 cusr  0.15
> csys =  3.82 CPU)
> Result: FAIL

Tables search_field column type only accepts following values: '','string','date','number','boolean','sum','isbn','stdno','year','callnumber'. Tests use value "keyword".
Comment 17 Emmi Takkinen 2023-10-17 10:52:54 UTC
Created attachment 157224 [details] [review]
Bug 17897: Fix failing tests and QA issues

Tests failed because
1) txn_begin and rollback were used inside
of subtests.
2) column search_field.type was filled with
unallowed value.

This patch also fixes QA issue with test file
exec rights and incorrect license statement.

To test prove t/db_dependent/Koha/SearchEngine/ElasticSearch.t
Comment 18 Emmi Takkinen 2023-10-17 10:53:51 UTC
Fixed tests and QA issues qa-tool found. Didn't change test files name yet.
Comment 19 David Nind 2023-10-17 19:33:37 UTC
Created attachment 157282 [details] [review]
Bug 17897: Koha::SearchMappingManager - Simplify search mapping modifications

Currently there is a lot of DBIC-magic everywhere where search mappings
(search_fields, search_marc_to_field, search_marc_map -tables) are CRUD:ed.

Encapsulate this hard-to-read DBIC-magic into a Manager class which promotes
code-reusability and thus, simpler test case writing.

Includes tests for Buug 17885

Rebased-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Signed-off-by: David Nind <david@davidnind.com>
Comment 20 David Nind 2023-10-17 19:33:40 UTC
Created attachment 157283 [details] [review]
Bug 17897: (follow-up) Cleanup after tests

Adds rollback to test data in t/db_dependent/Koha/SearchEngine/ElasticSearch.t

To test:
1. prove t/db_dependent/Koha/SearchEngine/ElasticSearch.t

Rebased-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Signed-off-by: David Nind <david@davidnind.com>
Comment 21 David Nind 2023-10-17 19:33:43 UTC
Created attachment 157284 [details] [review]
Bug 17897: Fix failing tests and QA issues

Tests failed because
1) txn_begin and rollback were used inside
of subtests.
2) column search_field.type was filled with
unallowed value.

This patch also fixes QA issue with test file
exec rights and incorrect license statement.

To test prove t/db_dependent/Koha/SearchEngine/ElasticSearch.t

Signed-off-by: David Nind <david@davidnind.com>
Comment 22 Katrin Fischer 2023-11-04 23:41:59 UTC
Please rebase!
Comment 23 Emmi Takkinen 2023-11-06 06:07:49 UTC
Created attachment 158487 [details] [review]
Bug 17897: Koha::SearchMappingManager - Simplify search mapping modifications

Currently there is a lot of DBIC-magic everywhere where search mappings
(search_fields, search_marc_to_field, search_marc_map -tables) are CRUD:ed.

Encapsulate this hard-to-read DBIC-magic into a Manager class which promotes
code-reusability and thus, simpler test case writing.

Includes tests for Buug 17885

Rebased-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Comment 24 Emmi Takkinen 2023-11-06 06:08:18 UTC
Created attachment 158488 [details] [review]
Bug 17897: (follow-up) Cleanup after tests

Adds rollback to test data in t/db_dependent/Koha/SearchEngine/ElasticSearch.t

To test:
1. prove t/db_dependent/Koha/SearchEngine/ElasticSearch.t

Rebased-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Comment 25 Emmi Takkinen 2023-11-06 06:13:05 UTC
Created attachment 158489 [details] [review]
Bug 17897: Koha::SearchMappingManager - Simplify search mapping modifications

Currently there is a lot of DBIC-magic everywhere where search mappings
(search_fields, search_marc_to_field, search_marc_map -tables) are CRUD:ed.

Encapsulate this hard-to-read DBIC-magic into a Manager class which promotes
code-reusability and thus, simpler test case writing.

Includes tests for Buug 17885

Rebased-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Signed-off-by: David Nind <david@davidnind.com>
Comment 26 Emmi Takkinen 2023-11-06 06:13:34 UTC
Created attachment 158490 [details] [review]
Bug 17897: (follow-up) Cleanup after tests

Adds rollback to test data in t/db_dependent/Koha/SearchEngine/ElasticSearch.t

To test:
1. prove t/db_dependent/Koha/SearchEngine/ElasticSearch.t

Rebased-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Signed-off-by: David Nind <david@davidnind.com>
Comment 27 Emmi Takkinen 2023-11-06 06:13:55 UTC
Created attachment 158491 [details] [review]
Bug 17897: Fix failing tests and QA issues

Tests failed because
1) txn_begin and rollback were used inside
of subtests.
2) column search_field.type was filled with
unallowed value.

This patch also fixes QA issue with test file
exec rights and incorrect license statement.

To test prove t/db_dependent/Koha/SearchEngine/ElasticSearch.t

Signed-off-by: David Nind <david@davidnind.com>
Comment 28 Emmi Takkinen 2023-11-06 06:15:46 UTC
Made some mess and accidentally rebased patches without Davids sign off first. Also these are missing "filter" param, will add it in a moment.
Comment 29 Emmi Takkinen 2023-11-06 06:36:21 UTC
Created attachment 158492 [details] [review]
Bug 17897: Add search_marc_to_field.filter to SearhMappingManager

Methods in SearchMappingManager didn't take into account
search_marc_to_field.filter indtroduced in bug 27153.

To test, prove t/db_dependent/Koha/SearchEngine/ElasticSearch.t

Sponsored-by: Koha-Suomi Oy
Comment 30 David Nind 2023-11-06 20:33:35 UTC
Created attachment 158561 [details] [review]
Bug 17897: Koha::SearchMappingManager - Simplify search mapping modifications

Currently there is a lot of DBIC-magic everywhere where search mappings
(search_fields, search_marc_to_field, search_marc_map -tables) are CRUD:ed.

Encapsulate this hard-to-read DBIC-magic into a Manager class which promotes
code-reusability and thus, simpler test case writing.

Includes tests for Buug 17885

Rebased-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Signed-off-by: David Nind <david@davidnind.com>
Comment 31 David Nind 2023-11-06 20:33:38 UTC
Created attachment 158562 [details] [review]
Bug 17897: (follow-up) Cleanup after tests

Adds rollback to test data in t/db_dependent/Koha/SearchEngine/ElasticSearch.t

To test:
1. prove t/db_dependent/Koha/SearchEngine/ElasticSearch.t

Rebased-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Signed-off-by: David Nind <david@davidnind.com>
Comment 32 David Nind 2023-11-06 20:33:41 UTC
Created attachment 158563 [details] [review]
Bug 17897: Fix failing tests and QA issues

Tests failed because
1) txn_begin and rollback were used inside
of subtests.
2) column search_field.type was filled with
unallowed value.

This patch also fixes QA issue with test file
exec rights and incorrect license statement.

To test prove t/db_dependent/Koha/SearchEngine/ElasticSearch.t

Signed-off-by: David Nind <david@davidnind.com>
Comment 33 David Nind 2023-11-06 20:33:44 UTC
Created attachment 158564 [details] [review]
Bug 17897: Add search_marc_to_field.filter to SearhMappingManager

Methods in SearchMappingManager didn't take into account
search_marc_to_field.filter indtroduced in bug 27153.

To test, prove t/db_dependent/Koha/SearchEngine/ElasticSearch.t

Sponsored-by: Koha-Suomi Oy
Signed-off-by: David Nind <david@davidnind.com>