ualbertalib / discovery

Discovery is the University of Alberta Libraries' catalogue interface, built using Blacklight
http://search.library.ualberta.ca
12 stars 3 forks source link

[Discovery] Remove and cleanup unused blacklight features #1448

Open murny opened 5 years ago

murny commented 5 years ago

As discussed with @murny and @seanluyk, Bookmarks feature is still being used, and need to approach this as a fix to current code. Saved Searches can currently be handled by the cron job in database - if it's difficult to remove it.Devise can't be removed completely as it ties with the Bookmark feature - We will disable the routes in routes.rb so that users can't access these pages.

  1. Bookmarks

Does anyone actually use this? Especially when they are buggy and sometimes don't work properly (e.g: they go from a button to checkbox and sometimes the checkbox can't be clicked etc. Similar issue here: https://github.com/ualbertalib/discovery/issues/1302)

  1. Saved searches and tracking

Blacklight saves and tracks every search and catalog view a user makes on discovery. These are stored in a page that is not accessible from discovery website (under https://library.ualberta.ca/search_history and if your logged in under https://library.ualberta.ca/saved_searches (which about the logging in part I'll get to in #3)

Something that is never used and potentially a big privacy concern

(this feature also causes alot of issues in rollbar too: https://rollbar.com/ualbertalib/discovery/items/8/)

( Both 1/2 bombard our database as seen in this issue #https://github.com/ualbertalib/discovery/issues/1361)

  1. Devise/Authentication

We have Devise from blacklight and it's fully setup. Meaning users can create accounts, log in, forgot password, etc. (e.g: https://library.ualberta.ca/users/sign_in) All this stuff is useless in Discovery as it doesn't do anything. As we don't user users for any functionality on Discovery. If we can't disable this fully (as blacklight uses the devise guest heavily), we should at least disable these urls (routes.rb) so no user can access these pages.

After this, what is blacklight actually giving us? Just the catalog stuff that we care about. But we actually override majority of the views/controllers and other catalog code in Discovery. So blacklight is actually giving us very little when you think about it.

Random Thoughts on Tackling this Refactor and Future of Discovery

  1. Probably easiest way to tackle this is call your loses. There is probably no way you could upgrade Blacklight from 5.15.0 to its current version (7.0.1 as of writing this) without a tremendous amount of work or a full rewrite. And if we remove the features above...what is Blacklight actually giving us? Nothing much honestly just the catalog code which we have already heavily customised for discovery. So therefore, I'd push for forking Blacklight....and refactoring this fork Blacklight code base until we don't need Blacklight at all and we just have Discovery moving forward. This brings us more in line with Jupiter model where we just focus on building normal rails applications using rails best practices. And puts us much further ahead in terms of future development, on boarding, etc.

  2. Once you fork the repo, start heavily cleaning up things we don't need. Impose our team's best practices for coding style by using our linters. Start at the top with the routes and remove any routes we don't need. We want to turn off bookmarks, saved searches, search history, user authentication/devise routes? Remove them from the fork. Then follow these down as far as you can. Remove the controller code next, remove the associated models, drop the tables, remove the view code, helpers, etc. Pretty soon you will see discovery is down to a handful of catalog routes, a single controller, a couple models, a few folders of views, etc. (Heck if we remove searches, bookmarks and users....we don't even need a mysql database for discovery, which be a nice infrastructure win!)

  3. Once you slim down the fork repo of blacklight and everything is still working. Take a peak at what's in discovery and whats in blacklight fork. You will notice we overwrite a ton of code in discovery. For example we have one controller left in blacklight....CatalogController but we aren't actually using it, we redefine this in Discovery. Same thing with the models that are left in blacklight (solr_document and record_mailer), same with a lot of the catalog and other views, etc etc. It's amazing to see how little blacklight is giving us as its hardly anything at all! So any of this code that is being duplicated in discovery, remove it from blacklight. Blacklight will be down to maybe a handle of views, some helpers, a few javascript/css files (a good chunk we don't use) and a bunch of utility files (under lib/blacklight which quite a few we overwrote in discovery or may or may not be used anymore).

  4. Repeat similar steps for the Blacklight gems like Blacklight advanced search, Blacklight marc, Blacklight google analytics, Blacklight range limit all of which are locked to a version and all of which provide hardly any functionality and some of them we probably don't really care about at all or getting 2 cents of value out of them?

  5. Ultimately we should be able to wean ourselves off Blacklight and it's ecosystem completely. And anything we need from Blacklight should just live in Discovery (which is a very small subsection of code that we actually care about).

  6. After the above, we should have only a discovery codebase with no Blacklight dependencies. Everything lives in Discovery. Now that the entire codebase is in one place, you can reevaluate what you have. Lot's of things we probably don't care about that comes from Blacklight or could be implemented in a more standard rails way (for example how Blacklight does it's view code, they wrote tons of custom helpers to render partials....instead of just calling render partial: 'example'). It's time to refactor and cleanup. You will start seeing how Discovery is a real simple app and with a pretty tiny codebase. And Blacklight was mostly just a super over engineered mess that really wasn't giving us a lot. At this point we already so much further ahead and we can really own this codebase now and bring the good work we done in our other applications like Jupiter to discovery now too. As I said I wouldn't rewrite Discovery, I think Discovery just needs some love and attention. It be very easy to transition it into a new app without having to rewrite it. If we did a complete rewrite would we be any further ahead? If we rewrote it on Blacklight 7, we would probably just repeat history or do something similar to NEOSDiscovery which is pretty much dead on arrival.

  7. So I'd advocate for rewriting Discovery from the inside in instead of starting from scratch. Move towards bringing it inline with Jupiter and other projects here. Upgrade Ruby, get on the latest Rails/Bootstrap, tackle things like the catalog show page refactor #1426, and css/js/html refactors while heavily emphasising normal Ruby on Rails patterns and refactoring the silliness that blacklight gives. At the same time maybe call bankrupt on the test suite and write good meaningful tests! (we got a handful of models and catalog pages, this would be very easy to spend some time to write some good tests for, that actually give us confidence when we refactor and do deployments in the future). It really wouldn't take much effort to get Discovery to be of the same quality as something like Jupiter. After all it's a pretty simple application (catalog index, catalog show, some export like functionality and that's pretty much it?). If you do all this, you would have an application that is very minimalistic and easy to understand by any Ruby on Rails developer. Adding new features would take no time at all. Then we can put our time to better causes and start focusing our efforts on fun problems and important problems like fixing our data ingest, improving our SEO/Accessiblity, our deployment and infrastructure, figure out a better strategy with Solr etc. Instead of trying to keep rebuilding and maintaining a site that just gives a simple index/show pages (similar to what we were doing with ERA in the past...constantly spending time rebuilding a simple CRUD application).

seanluyk commented 5 years ago

@murny let's discuss at the grooming meeting tomorrow, but I'm in favour of removing these features from the code for the reasons you mention.

seanluyk commented 5 years ago

Findings from our usability study:

Saving Results Functionality: The various Tools available to participants to save, share, and cite results were rarely located, and the value of these functions were considered irrelevant in their information-seeking workflow. For example, participants were confused about where bookmarked results were saved and expected this functionality to come with the ability to personalize saved items and that this would be tied to individual user accounts. Some of the terminology in the Tools was confusing to users. The term “bookmark” caused confusion: participants tried to find their bookmarked items from the bookmarks in their web browser. Within the bookmarking tool, the wording “Email me this item” led some users to think it meant the entire full text would be emailed to them (even if the item was a print item), not the item record information."

-Recommendation was not to remove these features, but given their bugginess, we have solid reasons to do so.

seanluyk commented 5 years ago

@murny @weiweishi one big gap that will be left if we remove the bookmarks functionality is the ability to send an email with multiple records and their URLs/call numbers at a time. We should discuss if this is something possible to fit in before we remove it, as we've had some related user feedback recently about this feature