Created attachment 21364 [details] [review] Bug 10933: The PurgeSearchHistory should be merge into the C4::Search::History module Since bug 10803 adds a C4::Search::History module, the PurgeSearchHistory routine should be moved. Test plan: - run misc/cronjobs/cleanup_database.pl with the searchhistory param and verify behavior is the same as before applying this patch. - run prove t/Search/History.t
Applying: Bug 10807: Add an authority search history for the OPAC Using index info to reconstruct a base tree... M installer/data/mysql/kohastructure.sql M installer/data/mysql/updatedatabase.pl Falling back to patching base and 3-way merge... Removing t/db_dependent/Auth_ParseSearchHistoryCookie.t Auto-merging installer/data/mysql/updatedatabase.pl CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl Auto-merging installer/data/mysql/kohastructure.sql Patch failed at 0001 Bug 10807: Add an authority search history for the OPAC The copy of the patch that failed is found in: /home/christopher/git/koha/.git/rebase-apply/patch When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort".
This bug depends on bug10807 which is in status Signed Off but the patches for it do not apply cleanly
Created attachment 22058 [details] [review] Bug 10933: The PurgeSearchHistory should be merge into the C4::Search::History module Since bug 10803 adds a C4::Search::History module, the PurgeSearchHistory routine should be moved. Test plan: - run misc/cronjobs/cleanup_database.pl with the searchhistory param and verify behavior is the same as before applying this patch. - run prove t/Search/History.t
Applying: Bug 10807: Add an authority search history for the OPAC Using index info to reconstruct a base tree... M C4/Auth.pm M C4/Search.pm M installer/data/mysql/kohastructure.sql M installer/data/mysql/updatedatabase.pl M opac/opac-search-history.pl Falling back to patching base and 3-way merge... Removing t/db_dependent/Auth_ParseSearchHistoryCookie.t Auto-merging opac/opac-search-history.pl CONFLICT (content): Merge conflict in opac/opac-search-history.pl Auto-merging installer/data/mysql/updatedatabase.pl CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl Auto-merging installer/data/mysql/kohastructure.sql Auto-merging C4/Search.pm Auto-merging C4/Auth.pm CONFLICT (content): Merge conflict in C4/Auth.pm Patch failed at 0001 Bug 10807: Add an authority search history for the OPAC The copy of the patch that failed is found in: /home/christopher/git/koha/.git/rebase-apply/patch When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort".
apply cleanly, patches for bug 10807 have been rebased.
Sorry, this no longer cleanly applies on top of Bug 10807.
Created attachment 27452 [details] [review] Bug 10933: The PurgeSearchHistory should be merge into the C4::Search::History module Since bug 10803 adds a C4::Search::History module, the PurgeSearchHistory routine should be moved. Test plan: - run misc/cronjobs/cleanup_database.pl with the searchhistory param and verify behavior is the same as before applying this patch. - run prove t/Search/History.t
Patch doesn't apply. Of more note, the change to C4::Search::History::add seems unrelated and dangerous, as no current callers of C4::Search::History:add seem to be passing a time parameter.
(In reply to Galen Charlton from comment #10) > Patch doesn't apply. It is only a unit file which was already deleted. > Of more note, the change to C4::Search::History::add seems unrelated and > dangerous, as no current callers of C4::Search::History:add seem to be > passing a time parameter. I don't understand why it is dangerous. Actually the search_history.time field has a default value in DB. I need to pass some hardcoded time (further back in time) for unit tests.
Created attachment 28022 [details] [review] Bug 10933: The PurgeSearchHistory should be merge into the C4::Search::History module Since bug 10803 adds a C4::Search::History module, the PurgeSearchHistory routine should be moved. Test plan: - run misc/cronjobs/cleanup_database.pl with the searchhistory param and verify behavior is the same as before applying this patch. - run prove t/Search/History.t
Created attachment 42114 [details] [review] Bug 10933: The PurgeSearchHistory should be merge into the C4::Search::History module Since bug 10803 adds a C4::Search::History module, the PurgeSearchHistory routine should be moved. Test plan: - run misc/cronjobs/cleanup_database.pl with the searchhistory param and verify behavior is the same as before applying this patch. - run prove t/Search/History.t Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>
Created attachment 42116 [details] [review] Bug 10933: (follow-up) fix the previous patch to work with master This makes the previous patch to work with the master branch.
Hi Jonathan, can you take a quick look at the failing test please? not ok 16 - Calling delete without userid raises warning # Failed test 'Calling delete without userid raises warning' # at t/db_dependent/Search/History.t line 128. # found warning: ERROR: userid, id or interval is required for history deletion at /home/katrin/kohaclone/C4/Search/History.pm line 77. # expected to find warning: (?^u:^ERROR: userid or id is required for history deletion)
Never mind - I fixed it, continuing with testing.
Created attachment 43786 [details] [review] [PASSED QA] Bug 10933: The PurgeSearchHistory should be merge into the C4::Search::History module Since bug 10803 adds a C4::Search::History module, the PurgeSearchHistory routine should be moved. Test plan: - run misc/cronjobs/cleanup_database.pl with the searchhistory param and verify behavior is the same as before applying this patch. - run prove t/Search/History.t Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 43787 [details] [review] [PASSED QA] Bug 10933: (follow-up) fix the previous patch to work with master Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Patches pushed to master. Thanks Jonathan!
Created attachment 44095 [details] [review] ug 10933: (follow-up of the follow-up) fix the previous patch to work with master