Description
Nick Clemens (kidclamp)
2016-08-23 15:25:26 UTC
+1! Created attachment 56141 [details] [review] Bug 17169 - Add a facet for ccode fields to ElasticSearch To test: 1 - Apply patch 2 - Backup your db 3 - Drop and create a new db to ensure your mappings are refreshed from the patch 4 - add some titles with items with collection codes 5 - search and see collection code facets 6 - sign off Work to be done: 1 - Replace codes with descriptions Created attachment 59946 [details] [review] Bug 17169: Add a facet for ccode fields to ElasticSearch To test: 1 - Apply patch 2 - Backup your db 3 - Drop and create a new db to ensure your mappings are refreshed from the patch 4 - add some titles with items with collection codes 5 - search and see collection code facets 6 - sign off Work to be done: 1 - Replace codes with descriptions Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 59947 [details] [review] Bug 17169: (QA followup) Add NORMARC and UNIMARC definitions for CCODE Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Nick, can you file a new one for the material language? I wrongly applied it on top of my patch for bug 18068, will make it dependent on it... If needed, I can do the work of rebasing the other way around... > Work to be done:
> 1 - Replace codes with descriptions
I think this is mandatory to make it in
Created attachment 60354 [details] [review] Bug 17679 - Use CCODE descriptions instead of codes Nick, My DB is dirty and I have Collection values that do not have CCODE AV related. I think we should display the code if there is no lib, something like SET facet.facet_label_value = AuthorisedValues.GetByCode('CCODE',facet.facet_label_value,0) || facet.facet_label_value What do you think? Otherwise there are empty li created. Created attachment 60664 [details] [review] Bug 17679 - Use CCODE descriptions instead of codes Seems reasonable to me - I think I assumed the plugin returned the code if it didn't match. I would almost prefer that, I can't think of many situations where we don't want to return it, but it seems to be used in lots of places right now and I don't want to break anything (In reply to Nick Clemens from comment #11) > Seems reasonable to me - I think I assumed the plugin returned the code if > it didn't match. I would almost prefer that, I can't think of many > situations where we don't want to return it, but it seems to be used in lots > of places right now and I don't want to break anything So? You did not implement what I suggested, it would fix the issue without breaking anything. Created attachment 60741 [details] [review] Bug 17169 - Use CCODE descriptions instead of codes (In reply to Jonathan Druart from comment #12) > So? You did not implement what I suggested, it would fix the issue without > breaking anything. I did, I just missed an add for the opac too :-) last patch should include your suggestion Created attachment 60890 [details] [review] Bug 17169: Add a facet for ccode fields to ElasticSearch To test: 1 - Apply patch 2 - Backup your db 3 - Drop and create a new db to ensure your mappings are refreshed from the patch 4 - add some titles with items with collection codes 5 - search and see collection code facets 6 - sign off Work to be done: 1 - Replace codes with descriptions Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 60891 [details] [review] Bug 17169: (QA followup) Add NORMARC and UNIMARC definitions for CCODE Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 60892 [details] [review] Bug 17169 - Use CCODE descriptions instead of codes Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Pushed to Master - Should be in the 17.05 Release. Thanks This won't get ported back to 16.11.x as it is an enhancement. I actually have some zebra code for this that I could contribute at some point as well... (In reply to David Cook from comment #20) > I actually have some zebra code for this that I could contribute at some > point as well... Ah for non-native facets in Zebra... |