Fixes #464 - Add support of facet_default_uri for search result page … #497
Conversation
|
@ucsdlib/developers - please review. Thanks! |
| first(:link, "View Collection Items").click | ||
|
|
||
| expect(page).to have_content("Sort: title") | ||
| expect(page).to_not have_content("Sort: relevance") |
There was a problem hiding this comment.
Would it also be worth having a check here that the two objects in this collection are indeed sorted by title?
There was a problem hiding this comment.
@mcritchlow The functionality of sort by title (by relevance and etc. ) is the blacklight built-in function. Here we want to make sure when load the page the result page has the "Sort: title" as default .
If sort: title displayed but the page is not actually sort by title, it will be blacklight built-in function bug. So Would it be redundant to write those tests again in our local app?
There was a problem hiding this comment.
Yeah, that's a very fair point. I suppose the main (only?) value-add that I can think of for adding an expect saying "I also expect this object to be shown before that object in the list" just makes it explicit that this is why we're making this change. If it wasn't a feature, or end-to-end test, I'd probably feel different.
But that said, your point is well taken, and I don't have strong feelings one way or the other. We could certainly leave it as-is.
| @unit.save! | ||
| @aCollection = DamsAssembledCollection.create(pid: "xx4473712z", visibility: "public", titleValue: "testCollection" ) | ||
| @aCollection.save! | ||
| @testObj1 = DamsObject.create(pid: "xx2801340v") |
There was a problem hiding this comment.
Only one other thought, related to these test objects and my other comment re: confirming the objects are sorted by title.
It seems like what we need these objects to have, as far as metadata, is a title. Anything else seems fairly irrelevant for the purpose of testing this new feature. I'm not sure how much difference in time it takes to persist and index these fixture files, but one (soccomObject1) appears to be a complex object. Could we instead just create two simple test objects and specify the minimal metadata we need to satisfy confirming that they appear ordered by title?
Something like:
...
@obj1 = DamsObject.create!(pid: "xx00000001", titleValue: "A Title", <unit and collection URI's>)
@obj2 = DamsObject.create!(pid: "xx00000002", titleValue: "B Title", <unit and collection URI's>)
..
There was a problem hiding this comment.
@mcritchlow Yes, I'm updating the tests to create the test objects instead of loading the existing fixtures files. Thanks for the suggestion!
There was a problem hiding this comment.
@mcritchlow After looking into it, I think it doesn't need to create or load new set of objects because there is existing loaded objects which is sufficient for testing this new feature. I've updated my tests. Please review. Thanks!
…when using View Collection Items. Fixed the failed tests. Update tests.
|
👍 |
1 similar comment
|
👍 |
…when using View Collection Items.
Fixes #464
Local Checklist
What does this PR do?
When selecting "View Collection Items" from a collection landing page, the result list that the user is taken to defaults to "sort by Title"
@ucsdlib/developers - please review