Skip to content

8838-cstr#9064

Merged
kcondon merged 35 commits intoIQSS:developfrom
cheneyfeng3:8838-cstr
Nov 15, 2022
Merged

8838-cstr#9064
kcondon merged 35 commits intoIQSS:developfrom
cheneyfeng3:8838-cstr

Conversation

@cheneyfeng3
Copy link
Contributor

@cheneyfeng3 cheneyfeng3 commented Oct 15, 2022

@pdurbin @qqmyers I recreate a branch name to initiate this merge. Can this resolve the conflict?

Closes:

xflv and others added 29 commits August 11, 2022 11:23
Publication ID Type list, we refer to the arXiv logo and Please review
the code and make the changes.
We have fixed the lowercase and sequential issues.
Please review the code and make the changes.
…ded by

qqmyers, please check if the conflict has been resolved.
As we do not have access to the conflict details, if the modification
does not resolve the conflict, please follow up with the file name and
line number of the conflict and we will follow up with the modification.
Adjusted sequence number
resolve AdminIT junit.framework.AssertionFailedError: expected:<322> but was:<323>
I should find the reason for the failure, please rebuild
@jggautier
Copy link
Contributor

jggautier commented Oct 31, 2022

Hi all. @scolapasta asked me to take a look at this PR and make sure the changes are the same as what's in the previous PR at #8913. The changes look the same to me. I simply looked at the "Files changed" pages of each PR (for example, https://github.com/IQSS/dataverse/pull/9064/files) and see that the same files are changed in the same ways in both PRs.

The only question I have is about how the new ID type will appear in the dropdown menu. I think, and I think @qqmyers confirmed, that because the ID type in the citation.properties file is in all caps,

controlledvocabulary.publicationIDType.cstr=CSTR

that's how it will appear in the GUI's metadata form, even though the ID type in the citation.tsv file is in lowercase (cstr).

Is this right? @cheneyfeng3, was it intentional that in the citation.properties file, CSTR is all caps?

In discussion in the previous PR, 8913, I suggested keeping the ID type all lowercase when it appears in the dropdown menu, at least for now, since most of the other ID Types in the dropdown menu are all lowercase.

@cheneyfeng3
Copy link
Contributor Author

@scolapasta @jggautier
We thought lowercase was fine, so we changed the Code. Please review.

@jggautier
Copy link
Contributor

Thanks so much @cheneyfeng3! We'll review today.

Copy link
Contributor

@jggautier jggautier left a comment

Choose a reason for hiding this comment

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

Thanks for changing the ID type name to lowercase @cheneyfeng3.

Adding release notes for addition of CSRT PID type.
@jggautier
Copy link
Contributor

jggautier commented Nov 14, 2022

I added a release note. Since I see that one of the files changed looks like code that creates the Schema.org metadata export, I mentioned that CSRT IDs added in the metadata will be included in the Schema.org exports.

Adding a heading
Adding required steps for updating the Citation metadata block to include the new ID type
@jggautier
Copy link
Contributor

jggautier commented Nov 14, 2022

To the release note I added required steps for updating the Citation metadatablock. I copied them from the release note included in the citation field improvements PR, which the section in the guides about reloading a repository's existing metadata block.

I'm done with the release notes 🎉

@kcondon
Copy link
Contributor

kcondon commented Nov 14, 2022

Hi @cheneyfeng3 I'm about to build and test this but the version number has changed. Would you please refresh this branch from develop so that I can build? Thanks.

@pdurbin
Copy link
Member

pdurbin commented Nov 14, 2022

please refresh this branch from develop

Done! In 2bb8291.

@jggautier
Copy link
Contributor

About the release notes, there's a command in the upgrade instructions that references the release's version number. But I put #.# as a placeholder. Is the version number for this release known?

@pdurbin
Copy link
Member

pdurbin commented Nov 14, 2022

@jggautier as of this writing the next release is 5.13: https://github.com/IQSS/dataverse/milestone/107

That said, the release note snippet doesn't have to be perfect. Placeholders are fine. We can insert the actual release number at release time.

@kcondon
Copy link
Contributor

kcondon commented Nov 14, 2022

The basic field value for cstr works fine. I do have some questions on export though. I'm not sure which exports should handle this cstr field. Testing shows the following:
Fields that export the ctr field: dc-terms, openaire, json, oai_ore. Fields that do not export it: ddi, ddi html codebook, datacite, schema.org
I'll do some more testing with existing values on previous build for comparison.

@jggautier
Copy link
Contributor

jggautier commented Nov 15, 2022

Thanks @kcondon. I wrote in the issue at #8838 that "the CSTR ID will also have to be included in several metadata exports that already include what depositors enter in the ID Type and ID Number fields."

I thought this slipped through the cracks entirely with the exception of the Schema.org export, so I'm glad to hear the IDs are added to other exports.

I'm not sure why the DDI.xml, DDI HTML, and Schema.org exports don't include the CSRTs. But I'd be happy to ping the contributor and help with that. From what I can tell from the crosswalk, it should be easy to include, at least conceptually.

It shouldn't be added to the DataCite export for now. The DataCite schema requires more information (the type of relationship), so I'd put it off until we've resolved the issues related to letting depositors/curators/repository managers indicate the type of relationships between their deposits and other research objects.

@qqmyers
Copy link
Member

qqmyers commented Nov 15, 2022

Adding this cstr identifier type should not change which exports the field shows up in. I.e. if you use DOI instead, you should see that in the same exports.

@kcondon
Copy link
Contributor

kcondon commented Nov 15, 2022

Yes, testing on current version using the ark value of the field shows the exported formats are the same as with cstr in this branch, so preexisting export issue: dc-terms: yes, ddi: no, datacite: no, ddi html codebook: no, json: yes, oai_ore: yes, openaire: yes, schema.org: no

@jggautier thanks for commenting. I did not full understand the first part of that requirement but was responding to the last part: The CSTR ID will also have to be included in several metadata exports that already include what depositors enter in the ID Type and ID Number fields: DDI, DC Terms, and OpenAIRE.

So, I tested those formats and DDI wasn't showing it. I then tested all the formats to detect a pattern. I'm still not sure what this sentence means, "The CSTR ID will also have to be included in several metadata exports that already include what depositors enter in the ID Type and ID Number fields"

@kcondon kcondon self-assigned this Nov 15, 2022
@kcondon
Copy link
Contributor

kcondon commented Nov 15, 2022

@jggautier has said export can be improved in a separate issue and the scope of this pr appears to be simply to add cstr to the id type list. That works fine so will merge. Thanks for clarifying everyone and apologies for delaying this further.

@kcondon kcondon merged commit f83e508 into IQSS:develop Nov 15, 2022
@xiaya2309
Copy link

Hello @jggautier
Thank you very much for your help in moving forward with adding CSTR to the list of associated identifiers.
We see that the code has been merged into the branch.
Does this mean this feature will be available in the next release?
Would you mind telling us how long it will be before users see and use CSTR on the page?
We would like to share this good news with our users.
If you have any questions, please feel free to contact us.
Thank you very much for your help!
Have a good day!

@pdurbin pdurbin added this to the 5.13 milestone Nov 17, 2022
@pdurbin pdurbin linked an issue Nov 17, 2022 that may be closed by this pull request
@jggautier
Copy link
Contributor

Hi @xiaya2309. Yes, it looks like it'll be available in the next release. I'm not sure when the next release will be available. It depends on how soon the other changes in this upcoming release are finished, and I'll have to defer to others working on the Dataverse team to estimate that.

After the release is available, it will have to be applied to the repositories that use the Dataverse software, and then depositors and others will be able to enter related publication metadata that includes CSTR identifiers. Usually, releases are applied to the Harvard Dataverse Repository within a week of the release becoming available.

Thanks for your patience in adding this to Dataverse!

@pdurbin
Copy link
Member

pdurbin commented Nov 18, 2022

@xiaya2309 we don't have a predictable release schedule but we usually suggest simply looking at how often previous releases dropped to get a sense of timing: https://github.com/IQSS/dataverse/tags . Hope that helps! Thanks for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature Request/Idea: Add CSTR to Related Publication ID Type list

7 participants