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.

Sign up for GitHub

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 2 commits into from
Feb 25, 2025
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1f4403d
contrib: Remove redundant fields from affiliations, funders search
sakshamarora1 Feb 11, 2025
872fe8a
tests: contrib: Add test to search by ROR and country
sakshamarora1 Feb 17, 2025
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
View file
Open in desktop
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
View file
Open in desktop
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
17 changes: 17 additions & 0 deletions tests/contrib/affiliations/test_affiliations_resource.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,14 @@ def _create_affiliations(service, identity):
"en": "Northwestern University",
},
},
{
"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:
service.create(identity, aff)
Expand Down Expand Up @@ -159,3 +167,12 @@ def test_affiliations_suggest_sort(
assert res.json["hits"]["total"] == 2
assert res.json["hits"]["hits"][0]["id"] == "nu"
assert res.json["hits"]["hits"][1]["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"
Loading