-
Notifications
You must be signed in to change notification settings - Fork 359
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
LibGuides and AZ: Display descriptions #3128
LibGuides and AZ: Display descriptions #3128
Conversation
The test failure is occurring because the fixture doesn't match what an actual LibGuides response looks like. I'm guessing it was made with version 1 of the search widgets "API" instead of v2. I will need to create a new fixture (and will look for the instructions on that again -- it's binary so a pain.) Details: The fixture is returning just a series of |
module/VuFindSearch/src/VuFindSearch/Backend/LibGuides/Connector.php
Outdated
Show resolved
Hide resolved
themes/bootstrap3/templates/RecordDriver/LibGuides/result-list.phtml
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @maccabeelevine -- a few comments. Let me know if I can of more help with the test fixture issue.
module/VuFindSearch/src/VuFindSearch/Backend/LibGuides/Connector.php
Outdated
Show resolved
Hide resolved
module/VuFindSearch/src/VuFindSearch/Backend/LibGuides/Connector.php
Outdated
Show resolved
Hide resolved
module/VuFindSearch/src/VuFindSearch/Backend/LibGuides/Connector.php
Outdated
Show resolved
Hide resolved
themes/bootstrap3/templates/RecordDriver/LibGuides/result-list.phtml
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the improvements -- see below for one more thought. Other than that, let me know when it's ready to test and I can kick the tires more closely. :-)
@demiankatz ready to test now, thanks. For future reference, this is the query used to generate the new fixture. The old one was clearly Bryn Mawr so I repeated that, but the response format has changed thoroughly. The data of course changed as well so tests are updated. IMHO saving the uncompressed version of the response is easier to understand when fixing tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @maccabeelevine! I spotted one last tiny issue which I just took care of myself to expedite things (the type annotations in one file said "boolean" instead of the expected "bool"). I believe it's ready to merge now, as soon as CI passes.
Optionally display the description field from LibGuides (research guides) and LibGuidesAZ (databases).
Future: