Skip to content

Conversation

@JPrevost
Copy link
Member

@JPrevost JPrevost commented Apr 24, 2017

  • local_browse feature now uses in_app links instead of external links
    to EDS UI for authors and subjects
  • external links are still used when local_browse is not enabled

NOTE: I'm not sure this is actually working as intended as I don't seem to get identical results browsing locally and via the EDS UI options.

HINT: https://mit-bento-staging-pr-182.herokuapp.com/toggle?feature=local_browse

@gravesm gravesm temporarily deployed to mit-bento-staging-pr-182 April 24, 2017 13:57 Inactive
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 7e69fb8 on in_app_subjects_and_authors into 8dd0a87 on master.

@JPrevost JPrevost requested a review from frrrances April 24, 2017 14:11
@frrrances
Copy link
Contributor

frrrances commented Apr 25, 2017

I'm noticing a few things:

  • Author search seems to work correctly, but not use the filters to limit format
  • Subjects don't seem to be working entirely correctly, though I can't figure out a pattern yet - example that shows results when you click "electronic books" but not when you click "Prefect, Ford -- Fiction" even though 2 of the top 5 results have that subject heading.

I'll keep digging but I think you're right that this needs some tweaking. Since it's a subtle problem, I think we should hold off on merging until we can figure out what's going on, but am happy to discuss if you have your finger on the button...

@JPrevost
Copy link
Member Author

I'm in no rush to merge. We can send this out for more feedback too.

@JPrevost JPrevost temporarily deployed to mit-bento-staging-pr-182 April 25, 2017 15:15 Inactive
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling dc9688b on in_app_subjects_and_authors into 8dd0a87 on master.

@JPrevost JPrevost temporarily deployed to mit-bento-staging-pr-182 April 25, 2017 16:45 Inactive
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 9b900b1 on in_app_subjects_and_authors into 8dd0a87 on master.

@frrrances
Copy link
Contributor

Subjects looking good now 🌈

Copy link
Contributor

@frrrances frrrances left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from my perspective, this is all 👍 - though we can wait for feedback from the team.

@JPrevost JPrevost force-pushed the in_app_subjects_and_authors branch from 9b900b1 to 87c25b7 Compare April 26, 2017 13:45
@JPrevost JPrevost temporarily deployed to mit-bento-staging-pr-182 April 26, 2017 13:45 Inactive
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 87c25b7 on in_app_subjects_and_authors into 8aa37d1 on master.

* local_browse feature now uses in_app links instead of external links
  to EDS UI for authors and subjects
* external links are still used when local_browse is not enabled
* changes subject queries to use `SU` not `DE` for subjects
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling ed0babe on in_app_subjects_and_authors into f480435 on master.

@JPrevost JPrevost merged commit 4db9e2e into master Apr 27, 2017
@JPrevost JPrevost deleted the in_app_subjects_and_authors branch April 27, 2017 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants