Custom metadata support boolean values #10480
Conversation
with values "true", "1", "yes", "Y", "On", "false", "0", "no", "N", "Off" (case insensitive)
|
@jeromeroucou thanks for the pull request! @Asbjoedt and I have been discussing possible implementations at https://dataverse.zulipchat.com/#narrow/stream/379673-dev/topic/The.20boolean/near/405225599 and then in person in Mexico so I'd love for him to take a look to see if this will work for him. You don't happen to have the code deployed to a server do you? |
|
Sure, if @Asbjoedt can review the PR, this could be a good idea. And no, this PR doesn't have been tested on a server yet, only on my laptop. Thanks for asking the question @pdurbin, it reminds me that having an integration server is going to become a good thing for Research Data Gouv team given the PR proposals we're doing. |
There was a problem hiding this comment.
This is a great start! I left some comments.
@jeromeroucou would you be able to add some screenshots in the description of this PR to show how the boolean looks and behaves in the GUI? Nevermind. I see you did this already. I'm a little confused by the GUI, actually. Let me think about it some more.
| @@ -0,0 +1,5 @@ | |||
| Boolean support added for fieldType definition. Accepted values are "true", "1", "yes", "Y", "On", "false", "0", "no", "N", "Off". All values are case insensitive. | |||
|
|
|||
| A HTML preview can be found [here](https://dataverse-guide--7961.org.readthedocs.build/en/7961/admin/metadatacustomization.html#fieldtype-definitions). | |||
There was a problem hiding this comment.
| A HTML preview can be found [here](https://dataverse-guide--7961.org.readthedocs.build/en/7961/admin/metadatacustomization.html#fieldtype-definitions). | |
| Documentation can be found at https://dataverse-guide--10480.org.readthedocs.build/en/10480/admin/metadatacustomization.html#fieldtype-definitions |
| @@ -36,8 +36,8 @@ public class DatasetFieldType implements Serializable, Comparable<DatasetFieldTy | |||
| * The set of possible metatypes of the field. Used for validation and layout. | |||
| */ | |||
| public enum FieldType { | |||
| TEXT, TEXTBOX, DATE, EMAIL, URL, FLOAT, INT, NONE | |||
| }; | |||
| TEXT, TEXTBOX, DATE, EMAIL, URL, FLOAT, INT, BOOLEAN, NONE | |||
There was a problem hiding this comment.
What about SolrField.java? Should we add BOOLEAN to the list:
STRING("string"), TEXT_EN("text_en"), INTEGER("int"), LONG("long"), DATE("text_en"), EMAIL("text_en");
And should we use BoolField? See https://solr.apache.org/guide/solr/latest/indexing-guide/field-types-included-with-solr.html
| @@ -0,0 +1,5 @@ | |||
| Boolean support added for fieldType definition. Accepted values are "true", "1", "yes", "Y", "On", "false", "0", "no", "N", "Off". All values are case insensitive. | |||
There was a problem hiding this comment.
@jeromeroucou what is the default value? True or false?
Also, did you consider if the field could be nullable or not? @Asbjoedt and I talked about this.
There was a problem hiding this comment.
@jeromeroucou please see this new pull request where @Asbjoedt has a field that can be yes, no, or unknown:
What do you think?
There was a problem hiding this comment.
Thanks @pdurbin for your comment.
I agree with the discussion in the issue, it repeats what was said in the chat (Zulip), and that by indicating that the new archival.tsv file is experimental, I would take its modification into this PR (it will be the first demonstrator of the feature).
Do you agree with me?
There was a problem hiding this comment.
@jeromeroucou first, sorry for the slow reply. I was on vacation and I'm still catching up!
I'm a little confused. Are you saying you would modify this PR to all for "unknown"? And then we'd use archival.tsv to test it?
There was a problem hiding this comment.
Hi @pdurbin , I hope you vacation was great !
I wanted to say that I think the proposal made in the archival.tsv file seems to correspond to what was exchanged in zulip, i.e. either "yes", "no" or "unknown". And I would modify my PR accordingly to take into account what is proposed in the archival.tsv file. As for the order, I think that the PR #10626 can be processed first, and that I would take it into account via a merge.
Is it more clearly?
There was a problem hiding this comment.
What this PR does / why we need it:
This PR add possibility for a depositor to write boolean values for a metadata field.
Which issue(s) this PR closes:
Special notes for your reviewer:
I take a large possible value, view on documentation Feature Flag.
On dataset edition, it's a input text to write metadata value.
Suggestions on how to test this:
On a metadata file, add a new metadata like below (exemple for citation.tsv file) :
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Boolean metadata on dataset edition :
Invalid boolean value :
Boolean value on dataset metadata view :
dataverse_jsonexport :{"typeName":"booleanMetadata","multiple":false,"typeClass":"primitive","value":"TRUE"}Is there a release notes update needed for this change?:
Yes, a new commit will be added to add more detail on release note snippet
Additional documentation:
Added boolean support on metadata customization documentation. Preview at
https://dataverse-guide--10480.org.readthedocs.build/en/10480/admin/metadatacustomization.html#fieldtype-definitions