-
Notifications
You must be signed in to change notification settings - Fork 501
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
10217 source name harvesting client #11217
Conversation
cb3fba3
to
668ac77
Compare
Thank you for the improvement @pdurbin |
There was a problem hiding this comment.
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!
…ources/db/migration/V6.5.0.6.sql
There was a problem hiding this 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.
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this 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.
Getting a failure: https://jenkins.dataverse.org/blue/organizations/jenkins/IQSS-Dataverse-Develop-PR/detail/PR-11217/13/pipeline Please advise. |
Needs a merge to pick up the revert PR |
Merging away and seeing what jenkins has in store for us. |
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 flagindex-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 :
Does this PR introduce a user interface change?:
The new field is visible in both creation and edition harvesting client popup :
Examples of harvesting result in metadata source :
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: