-
Notifications
You must be signed in to change notification settings - Fork 131
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
VCards in entry forms, list views, search indexing and visualization #4015
base: main
Are you sure you want to change the base?
Conversation
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.
In my opinion, an explanation for Vcard is required. Maybe something like this: "Please select VCARD only, when the person has not and is not supposed to have a profile in VIVO. Otherwise choose the type "Person"."
@tawahle |
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.
Everything is working fine.
…twork and non-vcard collaborators boost
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.
@litvinovg thanks for the PR. Looks nice and works well. I have posted a couple of comments.
I am wondering whether we should add a middle class between VIVOBaseGenerator on side, and AddEditorsToInformationResourceGenerator and AddAuthorsToInformationResourceGenerator on the other side, to encapsulate shared code in that middle class (for instance it might be entitled AddContributorToInformationResourceGenerator).
webapp/src/main/webapp/config/listViewConfig-informationResourceInEditorship.xml
Outdated
Show resolved
Hide resolved
uil-data:select_vcard_or_person.VIVO | ||
rdf:type owl:NamedIndividual ; | ||
rdf:type uil:UILabel ; | ||
rdfs:label "Please select VCARD only, when the person doesn't have and is not supposed to have a profile in VIVO. Otherwise choose the type \"Person\"."@en-CA ; |
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.
Maybe we can use here:
Please select external co-author, when the person doesn't have and is not supposed to have a profile in VIVO. Otherwise choose the type "Person"."
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.
Maybe. In that case VCARD option should be changed accordingly on entry form pages. In any case I think we should change all the translations altogether when there is an agreement on how it should look like to avoid changing the same thing multiple times. It was already changed once in accordance with this comment #4015 (review)
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.
As discussed in recent meetings, we should avoid mentioning vCards (not VCARDs or VCards) in UI messages. If we need this binary choice, I would suggest "add to coauthor's profile page" -- ideally with a checkbox rather than a dropdown. Checking would link/create a foaf:Person; unchecking would create a vCard. I haven't yet spun up this PR to see what the page looks like, but ideally we would allow the user to search for and link to an existing foaf:Person; if not found (or the search is not used), the form for entering the coauthor's name would just have a checkbox for "create profile page for coauthor" which would govern whether new a foaf:Person or vCard is created.
webapp/src/main/webapp/config/listViewConfig-relatedExternalSpeakerRole.xml
Outdated
Show resolved
Hide resolved
webapp/src/main/webapp/config/listViewConfig-relatedExternalSpeakerRole.xml
Outdated
Show resolved
Hide resolved
webapp/src/main/webapp/config/listViewConfig-relatedExternalSpeakerRole.xml
Outdated
Show resolved
Hide resolved
webapp/src/main/webapp/config/listViewConfig-relatedExternalSpeakerRole.xml
Outdated
Show resolved
Hide resolved
webapp/src/main/webapp/templates/freemarker/edit/forms/js/addEditorsToInformationResource.js
Outdated
Show resolved
Hide resolved
webapp/src/main/webapp/templates/freemarker/visualization/personlevel/coAuthorPersonLevelD3.ftl
Show resolved
Hide resolved
...webapp/edit/n3editing/configuration/generators/AddAuthorsToInformationResourceGenerator.java
Outdated
Show resolved
Hide resolved
...webapp/edit/n3editing/configuration/generators/AddAuthorsToInformationResourceGenerator.java
Show resolved
Hide resolved
...webapp/edit/n3editing/configuration/generators/AddAuthorsToInformationResourceGenerator.java
Show resolved
Hide resolved
...lib/vitro/webapp/edit/n3editing/configuration/generators/ProjectHasParticipantGenerator.java
Outdated
Show resolved
Hide resolved
...lib/vitro/webapp/edit/n3editing/configuration/generators/ProjectHasParticipantGenerator.java
Outdated
Show resolved
Hide resolved
...ava/edu/cornell/mannlib/vitro/webapp/visualization/coauthorship/CoAuthorshipQueryRunner.java
Outdated
Show resolved
Hide resolved
...ava/edu/cornell/mannlib/vitro/webapp/visualization/coauthorship/CoAuthorshipQueryRunner.java
Outdated
Show resolved
Hide resolved
|
||
if (stmt.getObject().isLiteral()) { | ||
switch (stmt.getPredicate().getURI()) { | ||
case "http://www.w3.org/2006/vcard/ns#familyName": |
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.
These should already be constants somewhere.
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.
Statement stmt = iter.nextStatement(); | ||
|
||
if (stmt.getObject().isLiteral()) { | ||
switch (stmt.getPredicate().getURI()) { |
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.
This nesting can be reduced with a continue statement instead.
log.error(e, e); | ||
} | ||
|
||
if (!StringUtils.isEmpty(familyName)) { |
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.
Why not use StringUtils.isNotEmpty?
// ---------------------------------------------------------------------- | ||
|
||
private static final String LOCATE_PARTNERS_WITHOUT_RESTRICTION = "" | ||
+ "SELECT ?partner \n" // |
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.
Isn't there a way not to have all the SPARQL in code?
return givenName; | ||
} | ||
|
||
return null; |
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.
Isn't it possible to avoid the conditioning above by constructing the return value in the iteration?
Model model = ModelFactory.createDefaultModel(); | ||
rdfService.sparqlConstructQuery(sparqlConstruct, model); | ||
|
||
StmtIterator iter = model.listStatements(); |
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.
Can the StmtIterator
be closed with try-with-resources?
rdf:type uil:UILabel ; | ||
rdfs:label "Ne sélectionnez VCARD que si la personne n'a pas et n'est pas censée avoir un profil dans VIVO. Dans le cas contraire, choisissez le type \"Personne\"."@fr-CA ; | ||
uil:hasApp "VIVO" ; | ||
uil:hasKey "select_vcard_or_person" . |
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.
Missing POSIX ending newline.
rdf:type uil:UILabel ; | ||
rdfs:label "Izaberite samo VCARD, kada osoba nema i ne bi trebalo da ima profil u VIVO-u. U suprotnom izaberite tip \"Osoba\"."@sr-Latn-RS ; | ||
uil:hasApp "VIVO" ; | ||
uil:hasKey "select_vcard_or_person" . |
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.
Missing POSIX ending newline.
@@ -4,16 +4,17 @@ | |||
<!-- See guidelines at https://wiki.lyrasis.org/display/VIVODOC112x/Custom+List+View+Configurations --> | |||
|
|||
<list-view-config> | |||
<query-select> | |||
PREFIX rdfs: <http://www.w3.org/2000/01/rdf-schema#> | |||
<query-select> |
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.
The extraneous white-space characters can be cleaned up.
VIVO GitHub issue
Vitro PR
What does this pull request do?
Added option to store participants as VCards for authorships, editorships and project participants.
Added option in editorship entry form to select organization as editor.
Added search indexing extension to index VCard participants.
Support visualization of VCard co-authors
What's new?
Modified entry forms:
AddAuthorsToInformationResourceGenerator.java
addAuthorsToInformationResource.ftl
addAuthorsToInformationResource.js
AddEditorsToInformationResourceGenerator.java
addEditorsToInformationResource.ftl
addEditorsToInformationResource.js
ProjectHasParticipantGenerator.java
projectHasParticipant.ftl
Created list view for presentation participant listViewConfig-relatedExternalSpeakerRole.xml
Applied list view for participant faux property PropertyConfig.n3
Modified editorship list view to support VCards listViewConfig-informationResourceInEditorship.xml and propStatement-informationResourceInEditorship.ftl
VCard visualization modifications:
Collaborator.java
CollaborationDataViewHelper.java
coAuthorPersonLevelD3.ftl
collaboratorToActivityCountTable.ftl
Added new properties configured in runtime.properties:
Maximum number of co-authors displayed on co-authors network.
visualization.coAuthorNetwork.maxCollaborators = 35
Prioritize co-authors stored as Persons in lists with VCard co-authors
visualization.coAuthorNetwork.personBoost = 0
How should this be tested?
This is deployed on https://vivo.tib.eu/vivorc
A description of what steps someone could take to:
Interested parties
@VIVO-project/vivo-committers
Reviewers' expertise
Candidates for reviewing this PR should have some of the following expertises:
Reviewers' report template
General comment
A reviewer should provide here comments and suggestions for requested changes if any.
Testing
A reviewer should briefly describe here how it was tested
Code reviewing
A reviewer should briefly describe here which part was code reviewed