Skip to content
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

contrib: Remove redundant fields from affiliations, funders search #460

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions invenio_vocabularies/contrib/affiliations/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,9 @@ class AffiliationsSearchOptions(SearchOptions):
# Aliases can sometimes be shorter, so we boost them a bit.
"aliases^5",
localized_title,
"id^2",
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 going to break searching by ROR. The test data might be misleading here…in real data the id isn’t the same as the acronym.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe not because of identifiers…but could you add a test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I've added a test, it works

# Allow to search identifiers directly (e.g. ROR)
"identifiers.identifier",
"country",
Copy link
Contributor

Choose a reason for hiding this comment

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

This might make it difficult to disambiguate funders with the same name but in different countries.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, the country is the abbreviation and the country_name is the full name of the country, which is what is shown in the UI too. I've also added a test for search with country.

"country_name",
"types",
],
)

Expand Down
5 changes: 1 addition & 4 deletions invenio_vocabularies/contrib/funders/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,10 @@ class FundersSearchOptions(SearchOptions):
# Aliases can sometimes be shorter, so we boost them a bit.
"aliases^5",
localized_title,
"id^2",
# Allow to search identifiers directly (e.g. ROR)
"identifiers.identifier",
"country",
"country_name",
"types",
]
],
)

sort_default = "bestmatch"
Expand Down
48 changes: 44 additions & 4 deletions tests/contrib/affiliations/test_affiliations_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,24 @@ def _create_affiliations(service, identity):
"title": {
"en": "Northwestern University",
},
"identifiers": [{"identifier": "000e0be47", "scheme": "ror"}],
},
{
"acronym": "NU",
"id": "chnu",
"name": "Northwestern University",
"title": {
"en": "Northwestern University",
},
"country_name": "Switzerland",
},
{
"acronym": "LONG",
"id": "LONG",
"name": "!!! The Official Global Alliance of the Most Prestigious, Highly-Renowned, and Exceptionally Innovative Solutions for Business, Commerce, Trade, Finance, Investment, Corporate Services, Logistics, and International Market Expansion—Providing Cutting-Edge, World-Class, and State-of-the-Art Consulting, Advisory, and Strategic Development Services for Organizations of All Sizes, Sectors, and Industries Across the Globe, Inc. ???",
"title": {
"en": "!!! The Official Global Alliance of the Most Prestigious, Highly-Renowned, and Exceptionally Innovative Solutions for Business, Commerce, Trade, Finance, Investment, Corporate Services, Logistics, and International Market Expansion—Providing Cutting-Edge, World-Class, and State-of-the-Art Consulting, Advisory, and Strategic Development Services for Organizations of All Sizes, Sectors, and Industries Across the Globe, Inc. ???"
},
},
]
for aff in affiliations:
Expand All @@ -127,10 +145,10 @@ def test_affiliations_prefix_search(
"""Test a successful search."""
_create_affiliations(service, identity)

# Should show 1 result
# Should show 2 results
res = client.get(f"{prefix}?q=uni", headers=h)
assert res.status_code == 200
assert res.json["hits"]["total"] == 1
assert res.json["hits"]["total"] == 2


def test_affiliations_suggest_sort(
Expand All @@ -156,6 +174,28 @@ def test_affiliations_suggest_sort(
# Should show 2 results, but id=nu as first due to acronym
res = client.get(f"{prefix}?suggest=NU", headers=h)
assert res.status_code == 200
assert res.json["hits"]["total"] == 2
assert res.json["hits"]["total"] == 3
assert res.json["hits"]["hits"][0]["id"] == "nu"
assert res.json["hits"]["hits"][1]["id"] == "cern" # due to nucleaire
assert res.json["hits"]["hits"][1]["id"] == "chnu"
assert res.json["hits"]["hits"][2]["id"] == "cern" # due to nucleaire

# Should show a result and not error out for longer names
res = client.get(
f"{prefix}?suggest=The%20Official%20Global%20Alliance%20of%20the%20Most%20Prestigious%2C%20Highly-Renowned%2C%20and%20Exceptionally%20Innovative%20Solutions%20for%20Business%2C%20Commerce%2C%20Trade%2C%20Finance%2C%20Investment%2C%20Corporate%20Services%2C%20Logistics%2C%20and%20International%20Market%20Expansion%E2%80%94Providing%20Cutting-Edge%2C%20World-Class%2C%20and%20State-of-the-Art%20Consulting%2C%20Advisory%2C%20and%20Strategic%20Development%20Services%20for%20Organizations%20of%20All%20Sizes%2C%20Sectors%2C%20and%20Industries%20Across%20the%20Globe",
headers=h,
)
assert res.status_code == 200
assert res.json["hits"]["total"] == 1
assert res.json["hits"]["hits"][0]["id"] == "LONG"

# Search affiliations with ROR ID
res = client.get(f"{prefix}?suggest=000e0be47", headers=h)
assert res.status_code == 200
assert res.json["hits"]["total"] == 1
assert res.json["hits"]["hits"][0]["id"] == "nu"

# Search affiliations with country
res = client.get(f"{prefix}?suggest=Switzerland", headers=h)
assert res.status_code == 200
assert res.json["hits"]["total"] == 1
assert res.json["hits"]["hits"][0]["id"] == "chnu"
7 changes: 7 additions & 0 deletions tests/contrib/funders/test_funders_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ def example_funders(service, identity, indexer):
"en": "European Organization for Nuclear Research",
"fr": "Conseil Européen pour la Recherche Nucléaire",
},
"identifiers": [{"identifier": "01ggx4157", "scheme": "ror"}],
},
{"id": "0aaaaaa11", "name": "OTHER", "country": "CH", "title": {"en": "CERN"}},
{
Expand Down Expand Up @@ -144,6 +145,12 @@ def test_funders_suggest_sort(client, h, prefix, example_funders):
assert res.status_code == 200
assert res.json["hits"]["total"] == 0

# Search affiliations with identifier
res = client.get(f"{prefix}?suggest=01ggx4157", headers=h)
assert res.status_code == 200
assert res.json["hits"]["total"] == 1
assert res.json["hits"]["hits"][0]["name"] == "CERN"


def test_funders_delete(
client_with_credentials, h, prefix, identity, service, funder_full_data
Expand Down
Loading