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

10217 source name harvesting client #11217

Conversation

luddaniel
Copy link
Contributor

@luddaniel luddaniel commented Feb 5, 2025

What this PR does / why we need it:

This PR will help admin to customize their harvesting clients name in metadata source facet.
A new field called sourceName, a nullable TEXT, has been added.
If set, it will overload the nickName in metadata source facet while indexing harvested dataset.
This will allow to have many harvesting client to be under the same sourceName if wished.

Please have in mind that displaying nickName or sourceName requires to activate the feature flag index-harvested-metadata-source.

This PR contains some refactoring and update on guide documentation.

Which issue(s) this PR closes:

Special notes for your reviewer:

I tested this code from user interface (Dashboard > create and edit havesting client popup) .
I tested this code from API also.
I was not able to test the HarvestingClientsIT in localhost.

My samples for testing :

{
"nickName": "CIRAD_1",
"dataverseAlias": "root",
"type": "oai",
"style": "dataverse",
"harvestUrl": "https://dataverse.cirad.fr/oai",
"archiveUrl": "https://dataverse.cirad.fr",
"archiveDescription": "This Dataset is harvested from our partners. Clicking the link will take you directly to the archival source of the data.",
"metadataFormat": "oai_dc",
"set": "entrepot_recherche_data_gouv",
"schedule": "none",
"allowHarvestingMissingCVV": false,
"useOaiIdentifiersAsPids": false
}

{
"nickName": "IRD",
"sourceName": "RDG Partners",
"dataverseAlias": "root",
"type": "oai",
"style": "dataverse",
"harvestUrl": "https://dataverse.ird.fr//oai",
"archiveUrl": "https://dataverse.ird.fr",
"archiveDescription": "This Dataset is harvested from our partners. Clicking the link will take you directly to the archival source of the data.",
"metadataFormat": "oai_dc",
"schedule": "none",
"allowHarvestingMissingCVV": false,
"useOaiIdentifiersAsPids": false
}

{
"nickName": "progedo",
"sourceName": "RDG Partners",
"dataverseAlias": "root",
"type": "oai",
"style": "default",
"harvestUrl": "https://data.progedo.fr/oai",
"archiveUrl": "https://data.progedo.fr",
"archiveDescription": "This Dataset is harvested from our partners. Clicking the link will take you directly to the archival source of the data.",
"metadataFormat": "oai_dc",
"schedule": "none",
"allowHarvestingMissingCVV": true,
"useOaiIdentifiersAsPids": false
}

Does this PR introduce a user interface change?:

The new field is visible in both creation and edition harvesting client popup :

sourcename popup

Examples of harvesting result in metadata source :

harvesting clients
metadata sources

Please note that both progedo and IRD are merged under "RDG Partners" in metadata source and CIRAD is displayed under CIRAD because the sourceName is not set.

Is there a release notes update needed for this change?

Yes

Preview doc changes:

@coveralls
Copy link

coveralls commented Feb 5, 2025

Coverage Status

coverage: 22.727%. remained the same
when pulling f7c4c42 on Recherche-Data-Gouv:10217-source-name-harvesting-client
into f5f2c72 on IQSS:develop.

@luddaniel luddaniel force-pushed the 10217-source-name-harvesting-client branch from cb3fba3 to 668ac77 Compare February 5, 2025 15:19
@cmbz cmbz added Size: 10 A percentage of a sprint. 7 hours. Feature: Harvesting GREI 3 Search and Browse labels Feb 7, 2025
@pdurbin pdurbin added this to the 6.6 milestone Feb 12, 2025
@cmbz cmbz added the FY25 Sprint 18 FY25 Sprint 18 (2025-02-26 - 2025-03-12) label Feb 27, 2025
@pdurbin pdurbin self-assigned this Feb 27, 2025
@luddaniel
Copy link
Contributor Author

Thank you for the improvement @pdurbin

Copy link
Member

Choose a reason for hiding this comment

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

@luddaniel can you please look again at this release not snippet? I just rewrote it again, based on my updated understanding of the history of this feature. Thanks!

Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

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

@luddaniel I ran the code and it seems to work fine. Please see individual comments in this review for changes I think we should make and other stuff to consider.

Copy link
Member

Choose a reason for hiding this comment

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

Tests are failing: https://jenkins.dataverse.org/job/IQSS-Dataverse-Develop-PR/job/PR-11217/

However, tests are failing on the develop branch too: https://jenkins.dataverse.org/job/IQSS-dataverse-develop/

Let's keep an eye on this (@luddaniel I realize you don't have visibility - #9916) as we get closer to moving this to "ready for QA".

@@ -0,0 +1,2 @@
-- Add this text will help to customized name in metadata source facet
ALTER TABLE harvestingclient ADD COLUMN IF NOT EXISTS sourcename TEXT;
Copy link
Member

Choose a reason for hiding this comment

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

We are close to releasing 6.6 and will be at code freeze (no more PRs merged) in a week on March 7th. With lots of merging going on I've already bumped the version of of this SQL script twice.

Mostly I want to communicate that with little time left, this PR might need to get bumped to 6.7. We'll see.

Copy link
Contributor

@landreev landreev Mar 4, 2025

Choose a reason for hiding this comment

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

Yeah, V6.5.0.6 has already been merged; and it appears that there are multiple contenders for V6.5.0.7... So, some last minute renaming will be needed, depending on which one is ready to be merged first.

Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

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

I think this is ready for QA. Approved!

There's an SQL script so let's keep on eye out for merge conflicts and bump the version as needed.

@ofahimIQSS ofahimIQSS self-assigned this Mar 5, 2025
@ofahimIQSS
Copy link
Contributor

Everything looks good here. - ready to merge once I see tests are passing (right now, jenkin has a long queue list)

image
image

@ofahimIQSS
Copy link
Contributor

@qqmyers
Copy link
Member

qqmyers commented Mar 5, 2025

Needs a merge to pick up the revert PR

@ofahimIQSS
Copy link
Contributor

Needs a merge to pick up the revert PR

Merging away and seeing what jenkins has in store for us.

@ofahimIQSS ofahimIQSS merged commit 7a29522 into IQSS:develop Mar 5, 2025
13 of 14 checks passed
@ofahimIQSS ofahimIQSS removed their assignment Mar 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature: Harvesting FY25 Sprint 18 FY25 Sprint 18 (2025-02-26 - 2025-03-12) GREI 3 Search and Browse Size: 10 A percentage of a sprint. 7 hours.
Projects
Status: Done 🧹
Status: 🚀 Done (Recherche Data Gouv)
Development

Successfully merging this pull request may close these issues.

Add Source/Server name to OAI-PMH Harvesting clients
7 participants