Description
Jonathan Druart
2013-09-23 11:33:52 UTC
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". This bug depends on bug10807 which is in status Signed Off but the patches for it do not apply cleanly 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 |