AZList VS AtoZList

[straleyb]

@jechols @wickr @luisgreg99 @lsat12357 @mccallum This is more of a place for discussion. I have two branches open currently with two different ways we can implement the AtoZ list for collections.

https://github.com/OregonDigital/oregondigital_2/commit/87d55a4cc9df3e48d72131750469a7df89804924

This branch uses the params hash to decide whether or not someone wanted to view a specific set or all the sets. There is some magic that goes on with blacklight which causes home_text to render when it loads the index page(home_text is currently programmed to support single sets rather than all the sets). I found a way around this by forcing it to render a page I created. This uses the existing sets controller.

https://github.com/OregonDigital/oregondigital_2/commit/9886c080bc36ff436df07eab13373fb92b836d72

This branch uses a new controller and route to simply go to the index page of the collections controller. This creates a new controller which inherently bypasses the home_text rendering of the index action in the sets controller. This also adds a bit of extra code since we now have a dedicated controller for the collections.

Which is the better option? We can also talk about other alternate methods of doing this too.

Thanks!

[wickr]

I strongly prefer keeping it all in the same controller. I'd also prefer the view to be called 'index' and not 'all_sets'.

Maybe the sets_index in the routing needs to be cleaned up too. sets/:set_id', :to => "sets#index", :as => :sets_index so it's clear what that is going to.

[wickr]

Also https://github.com/OregonDigital/oregondigital/blob/master/app/controllers/sets_controller.rb#L11 should be preserved somewhere.

[straleyb]

@wickr If im understanding this correctly, if I call the view "index.html.erb" instead of "all_sets.html.erb" it tries to render the partial "_home_text.html.erb" which causes an error when a list of sets is passed in instead of a single set. So I could account for this issue by making home_text.html.erb account for both groups of sets, and single sets, but I feel like that might be a violation of Single responsibility principle, I dont know for sure though.

What do you mean by cleaning up the routing?

[wickr]

_home_text.html.erb was modified to handle both in OD1. But if there's no room anywhere for a simple list of Sets without adding another controller than maybe we need to move the Set Page views down a level.

_home_text.html.erb probably shouldn't have to respond to both od.org/sets and od.org/sets/gwilliams

so maybe something more like the 'generic' files that were in OD1 for a Set to render and leave _home_text.html.erb for the Sets A-Z list view.

Pretty much the same applies to the routing. The Sets Index (A-Z) should go one place, but if there is a Set provided, it should go another, maybe directly to #show. Right now /sets/:set_id goes to sets#index and maybe it shouldn't.

[jechols]

I can't look closely at this right now, but I think we need to keep in mind that sets HAVE to be part of a catalog contoller in order to handle searching magic. With that magic comes side-effects. We decided those side-effects are acceptable for sets since we use so much of the search magic. I suspect one of those side effects is the need to use the index action, since that's what the catalog controller generally hacks up.

The A-Z list, on the other hand, needs either no search magic or else totally different search magic. I think that alone makes it a bad fit for a catalog controller subclass.

[tpendragon]

There's nothing that says /sets has to go to the SetsController.

[wickr]

Ok, @jechols those are good points, but the proposed additional CollectionsController currently also subclasses CatalogController, but maybe it doesn't need to at all.

So if the A-Z list warrants a new controller, does it make sense to call it SetsListController or something along those lines? Or are we going to continue the practice of naming things Sets in some places and Collections in others.