Bug 13428 - Fix facet deletion
Summary: Fix facet deletion
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching (show other bugs)
Version: 3.18
Hardware: All All
: P5 - low major (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on: 13425
Blocks:
  Show dependency treegraph
 
Reported: 2014-12-10 09:00 UTC by Jonathan Druart
Modified: 2016-06-21 21:37 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 13428: Fix regression on deleting facets (1.17 KB, patch)
2014-12-10 09:21 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13428: Fix regression on deleting facets (1.27 KB, patch)
2014-12-11 12:37 UTC, Mason James
Details | Diff | Splinter Review
[PASSED QA] Bug 13428: Fix regression on deleting facets (1.29 KB, patch)
2014-12-12 12:56 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 Jonathan Druart 2014-12-10 09:00:16 UTC
Bug 13425 introduced a regression on facets.
Comment 1 Jonathan Druart 2014-12-10 09:21:03 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2014-12-10 09:21:45 UTC
Patch for 3.16.x, 3.18.x and master.
The intranet does not need this change (already done).
Comment 3 Mason James 2014-12-11 12:37:17 UTC Comment hidden (obsolete)
Comment 4 Mason James 2014-12-11 12:38:45 UTC
> Signed-off-by: Mason James <mtj@kohaaloha.com>

this tested OK for me on 3.16, 3.18 and master branches
Comment 5 Mason James 2014-12-11 12:50:39 UTC
(In reply to Mason James from comment #4)
> > Signed-off-by: Mason James <mtj@kohaaloha.com>
> 
> this tested OK for me on 3.16, 3.18 and master branches

tested OK on prog and BS opac themes, for 3.16
Comment 6 Kyle M Hall 2014-12-12 12:56:54 UTC
Created attachment 34339 [details] [review]
[PASSED QA] Bug 13428: Fix regression on deleting facets

Test plan:
Search for something
click on a facet with diacritic
click on another facet
remove the facet with diacritic

Signed-off-by: Mason James <mtj@kohaaloha.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 7 Tomás Cohen Arazi 2014-12-17 22:32:41 UTC
Patch pushed to master.

Thanks Jonathan!
Comment 8 Chris Cormack 2014-12-18 00:03:47 UTC
Pushed to 3.18.x (no string changes) will be in 3.18.2
Comment 9 Jonathan Druart 2014-12-18 09:33:59 UTC
This should not have been pushed before bug 13425, because now the facets deletion is broken :)

It should be pushed to 3.16.x, the patch applies cleanly.

ping Mason :)
Comment 10 Chris Cormack 2014-12-18 12:04:40 UTC
(In reply to Jonathan Druart from comment #9)
> This should not have been pushed before bug 13425, because now the facets
> deletion is broken :)

Yep 13425 opac changes are pushed to master, 3.18.x and 3.16.x and this only changes the opac, so needed to be pushed. 

> 
> It should be pushed to 3.16.x, the patch applies cleanly.
> 
> ping Mason :)

So yep just to 3.16.x now
Comment 11 Mason James 2014-12-22 23:47:14 UTC
(In reply to Jonathan Druart from comment #9)
> This should not have been pushed before bug 13425, because now the facets
> deletion is broken :)
> 
> It should be pushed to 3.16.x, the patch applies cleanly.
> 
> ping Mason :)

Pushed to 3.16.x, patch will be in 3.16.6