ETT-1600: retire alicorn - #249
Conversation
… still called from search.choke
| my $config = $C->get_object('MdpConfig'); | ||
| my $records_per_page = $config->get('default_records_per_page'); | ||
|
|
||
| if ( $total_records > $records_per_page && $cgi->param('skin') ne 'alicorn' ) { |
There was a problem hiding this comment.
skin will never equal alicorn (or it could, but there won't be a corresponding theme to load), so I removed this bit from the conditional
| return $s; | ||
| } | ||
|
|
||
| sub handle_CACHE_TIMESTAMP_PI |
There was a problem hiding this comment.
This was only ever used with alicorn assets.
| <head> | ||
| <title>Unauthorized</title> | ||
| <link rel="stylesheet" type="text/css" href="/common/alicorn/css/main.201910.css" /> | ||
| <script type="text/javascript" src="/common/alicorn/js/utils.201910.js"></script> |
There was a problem hiding this comment.
I'm not even sure these reports live in pt anymore, but I removed the stylesheet in here for good measure.
|
|
||
| <head> | ||
| <title>Indexing in Progress | HathiTrust Digital Library</title> | ||
| <link rel="stylesheet" type="text/css" href="/common/alicorn/css/main.201910.css" /> |
There was a problem hiding this comment.
PT does use this file, which I found out when testing pt-search for Lianet a couple months ago, but it's only ever returned as a response from search.choke and we never see the UI in page turner. There is no modal with this info in it, I only saw the text as a response in the network tab of my browser.
I only removed the stylesheet here, but this could probably use a rework at some point.
The rest of the "changes" here are linting/formatting.
There was a problem hiding this comment.
I removed a few ancient html pages and unused stylesheets.
There was a problem hiding this comment.
I removed this one after determining that it isn't called from anywhere anymore. I could see that it used to be called (in ls.choke, for example), but I couldn't find evidence of that for current-day ls.
aelkiss
left a comment
There was a problem hiding this comment.
I had one question about the change in pageturner, but otherwise all looks fine to me.
We should test this carefully via test.babel before deploying (and after making sure any old assets under test.babel are cleaned up).
| @@ -30,12 +30,6 @@ | |||
| <xsl:if test="$gIsCRMS = 'true'"> | |||
| <xsl:text> crms </xsl:text> | |||
| </xsl:if> | |||
| <xsl:choose> | |||
There was a problem hiding this comment.
Removed because there are no other skins to support currently?
There was a problem hiding this comment.
@aelkiss Ah, yes, I explained this in the description area up top before removing a bunch of other stuff, I should've put this in with the other diff comments:
In pt, I left crms as an option as a skin class name in the html, but removed the others since there's only firebird now. I looked all over for any evidence that the class skin-default (or skin-alicorn, skin-2021) was tied to anything, but if it's doing anything, I can't find it.
@aelkiss Agreed, and will-do. There are no alicorn assets on test (see Here's what test.babel...&skin=alicorn pt looked like just moments ago, trying to fetch assets from
|

This fix removes alicorn and 2021 from the skin options. In pt, I left
crmsas an option as a skin class name in the html, but removed the others since there's only firebird now. I looked all over for any evidence that the classskin-default(orskin-alicorn,skin-2021) was tied to anything, but if it's doing anything, I can't find it. I was originally planning on using theskin-defaultclass in a playwright test to check thatskin=alicornparam falls back to the firebird theme, but it felt silly to keep a class in the code that we don't actually use for anything. I changed the test to locatehathi-website-header, which doesn't exist in the alicorn theme.At first I wasn't too happy with this fix because
Skin.pmsets the<Skin>tag to whatever is passed in via theskin=query param. But then I realized it's only doing that in dev environments, which I think is the point of the skin param, so that's fine.I don't know a lot about perl and I have a question about this ASSERT:
babel/mdp-lib/View/Fallback.pm
Line 177 in 276c125
This isn't throwing an error I don't think, but is it getting logged somewhere every time there's a request for a skin that isn't on the list?
Removal of alicorn directories and stylesheets
Unsurprisingly, there was a lot of legacy stuff to go through in here.
I tested ls, mb, pt, and wayf on dev-3 and everything seems fine. Playwright tests are passing for pt.
I left some comments in the diff but this was mainly just deleting anything in
/alicorndirectories and cleaning up files that used the stylesheets or other alicorn-related functions.