Open luisgreg99 opened 8 years ago
I think I have a better understanding of this code now. I think the code is confusing as-is, because we've got multiple checks on :set
being the property, and we're sort of hijacking the facet configuration for something that's very set-specific. Just on the UI side, the facet configuration page just doesn't feel like the right place to put this, in my opinion. But on a more technical level, it feels like we're making way too much special-case logic here. The fact that we'd have to hack topTerms
somewhere is also concerning - one more special case, and one more case where the name only means what you think in some cases.
I think this code needs to be in its own admin area where an admin can very easily and immediately see all the sets and decide which are shown and which are hidden. That should make the code a lot simpler in addition to making the workflow more clear.
As a follow-up, looking for topTerms
yielded nothing. I'm fairly certain at this point that it's part of a solr response to a query. Which means we really can't go this route. We have to directly ask solr for all sets, not just rely on however many it's giving us when we pull the top-level facets.
@jechols Thanks for taking a closer look at this code. I agree that it's a good idea to move it to it's own admin space. I was also concerned about topTerms being the only ones available for configuration, so I will take a closer review at how solr works and find a better way to retrieve all sets instead of depending on topTerms.
@luisgreg99 From the demo in the Sprint Review, it appears the developed feature was more to edit the Set facet to then turn Sets on and off in the Set facet. I don't believe that is the feature desired, and we should have all worked to make this clearer in the ticket.
Rather, I believe it was more that we wanted configurable facets (turning them on/off, changing label, etc.) extended so it's possible to have a different configuration for each Set.
So Set 1 (Braceros), would have Facet 1, 2 3 and 5 activated, where Set 2 (ChinaVine) might have Facet 2, 4, 5 and 6 activated. So for each Set, you can pick and choose what config options you want on the facets. And I think we also want a configurable option in the homepage.
If that's the case, I don't think we need a separate Admin Panel for this, but rather extend the Configurable Facets view. The default view might be you choose the settings for the Homepage, but then you could select from the Sets, maybe a dropdown, and that would let you choose another set of facet configuration, that is independent of the others.
If I'm off base or others have different interpretations, please chime in.
Thank you for the clarification @wickr. When you mention the idea about extending the "Configurable Facets" view, are you referring to expanding the view rendered at /admin/facets
?
@luisgreg99 Yes, that is the view I was referring to. Expanding "Currently Configured Facets" to maybe default to the Homepage, but then you can select a Set to get a new/different group, so "Currently Configured Facets for Gerald Williams Photograph Collection" or similar.
fixes #216