Skip to content
This repository has been archived by the owner on Apr 11, 2022. It is now read-only.

Add ProQuest integration #1208

Merged

Conversation

vbessonov
Copy link
Contributor

@vbessonov vbessonov commented Nov 1, 2020

Description

This PR contains changes required for ProQuestOPDS2Importer implemented in PR # 1520.

Motivation and Context

SIMPLY-3251

How Has This Been Tested?

Checklist:

  • I have updated the documentation accordingly.
  • All new and existing tests passed.

model/work.py Outdated
def calculate_presentation(
self, policy=None, search_index_client=None, exclude_search=False,
default_fiction=None, default_audience=None
default_fiction=None, default_audience=Classifier.AUDIENCE_ADULT
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@leonardr, I set default_audience to Classifier.AUDIENCE_ADULT instead of implementing SIMPLY-2610

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is OK. Since this code was written we have had many incoming feeds with no audience information and the implication was always that the works are for adults.

Copy link
Contributor

Choose a reason for hiding this comment

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

Christina pushed back a little in https://jira.nypl.org/browse/SIMPLY-2610; I'm waiting for her response. It may be better to simply implement 2610.

Another temporary way to solve this problem would be to continue to set audience=None for these books but to change the way books are placed into lanes. Specifically, we could say that a lanes whose audiences include AUDIENCE_ADULT or AUDIENCE_RESEARCH should also include any books that have no audience set.

The real problem we're trying to address is not that these books don't have an audience set (that's the most accurate conclusion -- we don't have the information) but that they don't show up in the default lanes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Based on the conversation with Christina in https://jira.nypl.org/browse/SIMPLY-2610, I don't think users will accept a change like this which adds metadata that was not provided by the vendor, the administrator, or a librarian.

My preference would be to resolve this by resolving SIMPLY-2610.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let me make this stronger: based on Christina's feedback I think you need to implement SIMPLY-2610 instead of setting this default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, will do

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I created PR # 1214 in server_core and merged it to this branch.

@vbessonov vbessonov force-pushed the feature/proquest-integration branch 2 times, most recently from a2485ca to c0718fb Compare November 2, 2020 11:19
model/credential.py Outdated Show resolved Hide resolved
util/__init__.py Outdated Show resolved Hide resolved
@leonardr
Copy link
Contributor

leonardr commented Nov 2, 2020

I suspect there may be some new files missing from this PR; there's nothing really Proquest-specific, there are no new tests, and you define an interface class that has no implementations.

@vbessonov vbessonov merged commit 5fce4b3 into NYPL-Simplified:develop Nov 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants