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

Openaire fix for multiple productionPlaces #11194

Merged
merged 9 commits into from
Feb 3, 2025

Conversation

ffritze
Copy link
Contributor

@ffritze ffritze commented Jan 28, 2025

What this PR does / why we need it:

  • The current openaire implementation cannot process multiple productionPlaces when preparing metadata for the geolocation.

Which issue(s) this PR closes:

Special notes for your reviewer:

Suggestions on how to test this:
Tests are written and adapted for this pull request. They are passing.

Does this PR introduce a user interface change? If mockups are available, please link/include them here:

Is there a release notes update needed for this change?:

Additional documentation:

@coveralls
Copy link

coveralls commented Jan 28, 2025

Coverage Status

coverage: 22.761% (+0.009%) from 22.752%
when pulling 29477ea on TIK-NFL:openaire_fix
into 7283dbd on IQSS:develop.

@pdurbin
Copy link
Member

pdurbin commented Jan 28, 2025

multiple productionPlaces

Made multiple in this PR:

@ofahimIQSS ofahimIQSS added the Size: 3 A percentage of a sprint. 2.1 hours. label Jan 28, 2025
@jggautier
Copy link
Contributor

jggautier commented Jan 28, 2025

might be related:

@cmbz cmbz added FY25 Sprint 15 FY25 Sprint 15 (2025-01-15 - 2025-01-29) FY25 Sprint 16 FY25 Sprint 16 (2025-01-29 - 2025-02-12) labels Jan 29, 2025
@pdurbin pdurbin self-assigned this Jan 31, 2025
@pdurbin
Copy link
Member

pdurbin commented Jan 31, 2025

@jggautier yes, good catch. I left some comments on #9546. This PR fixes it so I added "closes #9546" to the description of this PR for the issue to get auto-closed on merge.

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.

@ffritze thank for the pull request! This actually fixes an existing issue:

I went ahead and marked this PR as closing it and left some comments in it.

I'm close to approving this PR but please see my comments in this review. Thanks again!

@ffritze
Copy link
Contributor Author

ffritze commented Feb 3, 2025

@pdurbin I have added two test functions and added the the release note snippet. So eventually we are ready for the merge?

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.

Tests are passing: https://jenkins.dataverse.org/job/IQSS-Dataverse-Develop-PR/job/PR-11194/5/testReport/

@ffritze thanks for the PR and the extra tests! I'm approving this.

@ofahimIQSS ofahimIQSS self-assigned this Feb 3, 2025
@ofahimIQSS
Copy link
Contributor

tests passed - merging PR

@ofahimIQSS ofahimIQSS merged commit 1e1bd6d into IQSS:develop Feb 3, 2025
11 checks passed
@ofahimIQSS ofahimIQSS removed their assignment Feb 3, 2025
@pdurbin pdurbin added this to the 6.6 milestone Feb 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FY25 Sprint 15 FY25 Sprint 15 (2025-01-15 - 2025-01-29) FY25 Sprint 16 FY25 Sprint 16 (2025-01-29 - 2025-02-12) Size: 3 A percentage of a sprint. 2.1 hours.
Projects
Status: Done 🧹
Development

Successfully merging this pull request may close these issues.

Export: OpenAIRE export format fails silently on more than one productionPlace (made multiple in recent pr)
6 participants