Skip to content

Commit c84b55a

Browse files
vagishavagisha
authored
Panorama Public updates and bug fixes (#333)
- Added text about license in confirm view when data is made public - Add submitters to the "Panorama Public Submitters" group. Add reviewer to "Reviewers" group. These groups exist on panoramaweb.org. - Fix container links and "Make Public" link in notifications. These have to be absolute URLs - Replace "Access URL/Link" with "Permanent Link". - Added DOI in the message body of the data-copied notification. - Display the DOI as a complete link as recommended by the DataCite DOI Display Guidelines.
1 parent d861fa8 commit c84b55a

14 files changed

Lines changed: 201 additions & 45 deletions

File tree

‎panoramapublic/resources/schemas/panoramapublic.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@
216216
<url>/panoramapublic-showExperimentAnnotations.view?id=${ExperimentAnnotationsId}</url>
217217
</column>
218218
<column columnName="ShortAccessURL">
219-
<columnTitle>Access Link</columnTitle>
219+
<columnTitle>Permanent Link</columnTitle>
220220
<fk>
221221
<fkColumnName>EntityId</fkColumnName>
222222
<fkDbSchema>core</fkDbSchema>

‎panoramapublic/src/org/labkey/panoramapublic/PanoramaPublicController.java‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2406,7 +2406,7 @@ void validateForm(PublishExperimentForm form, Errors errors)
24062406
}
24072407
else
24082408
{
2409-
errors.reject(ERROR_MSG, "Please enter a short access URL.");
2409+
errors.reject(ERROR_MSG, "Please enter a permanent link.");
24102410
}
24112411

24122412
if (_experimentAnnotations.isIncludeSubfolders())
@@ -2723,7 +2723,7 @@ public static final class PublishExperimentFormBean
27232723
private final List<Journal> _journalList;
27242724
private final List<DataLicense> _dataLicenseList;
27252725
private final ExperimentAnnotations _experimentAnnotations;
2726-
private final boolean _accessUrlEditable; // flag which determines if the "Short Access URL" field in the form is editable
2726+
private final boolean _accessUrlEditable; // flag which determines if the "Permanent Link" field in the form is editable
27272727

27282728
public PublishExperimentFormBean(PublishExperimentForm form, List<Journal> journalList, List<DataLicense> dataLicenseList, ExperimentAnnotations experimentAnnotations)
27292729
{
@@ -2732,7 +2732,7 @@ public PublishExperimentFormBean(PublishExperimentForm form, List<Journal> journ
27322732
_dataLicenseList = dataLicenseList;
27332733
_experimentAnnotations = experimentAnnotations;
27342734
JournalSubmission js = SubmissionManager.getJournalSubmission(experimentAnnotations.getId(), form.getJournalId(), experimentAnnotations.getContainer());
2735-
// "Short Access URL" field in the form should not be editable if one or more copies of this experiment already exist in the journal project
2735+
// "Permanent Link" field in the form should not be editable if one or more copies of this experiment already exist in the journal project
27362736
_accessUrlEditable = js == null ? true : js.getCopiedSubmissions().size() == 0;
27372737
}
27382738

@@ -6633,13 +6633,15 @@ public static class PublicationDetailsBean
66336633
private final String _accessUrl;
66346634
private final boolean _isPublic;
66356635
private final boolean _isPeerReviewed;
6636+
private final DataLicense _license;
66366637

66376638
public PublicationDetailsBean(PublicationDetailsForm form, ExperimentAnnotations copiedExperiment)
66386639
{
66396640
_form = form;
66406641
_isPublic = copiedExperiment.isPublic();
66416642
_isPeerReviewed = copiedExperiment.isPeerReviewed();
66426643
_accessUrl = copiedExperiment.getShortUrl().renderShortURL();
6644+
_license = copiedExperiment.getDataLicense();
66436645
}
66446646

66456647
public PublicationDetailsForm getForm()
@@ -6661,6 +6663,11 @@ public String getAccessUrl()
66616663
{
66626664
return _accessUrl;
66636665
}
6666+
6667+
public DataLicense getLicense()
6668+
{
6669+
return _license;
6670+
}
66646671
}
66656672

66666673
public static class PublishSuccessViewBean
@@ -8539,7 +8546,7 @@ public ModelAndView getView(CatalogEntryForm form, boolean reshow, BindException
85398546
// We expect this action to be invoked in the Panorama Public copy of an experiment, so there should be a shortUrl.
85408547
if (expAnnot.getShortUrl() == null)
85418548
{
8542-
errors.reject(ERROR_MSG, "Experiment does not have a short access URL. Catalog entry cannot be added.");
8549+
errors.reject(ERROR_MSG, "Experiment does not have a permanent link. Catalog entry cannot be added.");
85438550
return new SimpleErrorView(errors);
85448551
}
85458552

‎panoramapublic/src/org/labkey/panoramapublic/PanoramaPublicNotification.java‎

Lines changed: 30 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,12 @@
1919
import org.labkey.api.view.ActionURL;
2020
import org.labkey.api.view.NotFoundException;
2121
import org.labkey.panoramapublic.datacite.DataCiteException;
22+
import org.labkey.panoramapublic.datacite.DataCiteService;
2223
import org.labkey.panoramapublic.model.ExperimentAnnotations;
2324
import org.labkey.panoramapublic.model.Journal;
2425
import org.labkey.panoramapublic.model.JournalExperiment;
2526
import org.labkey.panoramapublic.model.Submission;
27+
import org.labkey.panoramapublic.proteomexchange.ProteomeXchangeService;
2628
import org.labkey.panoramapublic.query.JournalManager;
2729
import org.labkey.panoramapublic.query.SubmissionManager;
2830

@@ -155,7 +157,10 @@ else if (addedPublication)
155157
{
156158
messageBody.append(NL).append("* Citation: ").append(escape(journalCopy.getCitation()));
157159
}
158-
160+
if (madePublic && journalCopy.getDataLicense() != null)
161+
{
162+
messageBody.append(NL2).append("The data will be available under the ").append(journalCopy.getDataLicense().getDisplayName()).append(" license.");
163+
}
159164
if (journalCopy.hasPxid())
160165
{
161166
messageBody.append(NL2);
@@ -205,11 +210,9 @@ private static void postNotification(ExperimentAnnotations experimentAnnotations
205210
messageBody.append(NL2).append("Best regards,");
206211
messageBody.append(NL).append(getUserName(journalAdmin));
207212

208-
messageBody.append(NL2).append("---").append(NL2);
209-
appendActionSubmitterDetails(user, messageBody);
210-
211213
if (addCopyLink)
212214
{
215+
messageBody.append(NL2).append("---");
213216
messageBody.append(NL).append("For Panorama administrators:").append(" ").append(link("Copy Link", je.getShortCopyUrl().renderShortURL()));
214217
}
215218

@@ -286,7 +289,7 @@ private static void appendSubmissionDetails(ExperimentAnnotations exptAnnotation
286289
text.append(NL2);
287290
text.append(italics("Submission details:"));
288291
text.append(NL).append("* Source folder: ").append(getContainerLink(exptAnnotations.getContainer()));
289-
text.append(NL).append("* Permanent URL: ").append(link(journalExperiment.getShortAccessUrl().renderShortURL()));
292+
text.append(NL).append("* Permanent link: ").append(link(journalExperiment.getShortAccessUrl().renderShortURL()));
290293
text.append(NL).append("* Reviewer account requested: ").append(bold(submission.isKeepPrivate() ? "Yes" : "No"));
291294
text.append(NL).append("* PX ID requested: ").append(bold(submission.isPxidRequested() ? "Yes" : "No"));
292295
if (submission.isIncompletePxSubmission())
@@ -308,11 +311,6 @@ private static void appendUserDetails(User user, String userType, StringBuilder
308311
}
309312
}
310313

311-
private static void appendActionSubmitterDetails(User user, StringBuilder text)
312-
{
313-
text.append("Requested by: ").append(getUserDetails(user));
314-
}
315-
316314
public static String getUserName(User user)
317315
{
318316
return !StringUtils.isBlank(user.getFullName()) ? user.getFullName() : user.getFriendlyName();
@@ -326,7 +324,7 @@ private static String getUserDetails(User user)
326324
private static String getContainerLink(Container container)
327325
{
328326
ActionURL url = PageFlowUtil.urlProvider(ProjectUrls.class).getBeginURL(container);
329-
return link(container.getPath(), url.getEncodedLocalURIString());
327+
return link(container.getPath(), url.getURIString());
330328
}
331329

332330
public static String bolditalics(String text)
@@ -390,8 +388,8 @@ public static String getExperimentCopiedMessageBody(ExperimentAnnotations source
390388
{
391389
message.append("Thank you for your request to submit data to ").append(journalName).append(". ");
392390
}
393-
message.append(String.format("Your data has been %s, and the permanent URL (%s)", recopy ? "recopied" : "copied", accessUrlLink))
394-
.append(String.format(" now links to the %scopy on %s.", recopy ? "new " : "", journalName))
391+
message.append(String.format("Your data has been %s, and the permanent link (%s)", recopy ? "recopied" : "copied", accessUrlLink))
392+
.append(String.format(" now points to the %scopy on %s.", recopy ? "new " : "", journalName))
395393
.append(" Please take a moment to verify that the copy is accurate.");
396394

397395
if(reviewer != null)
@@ -416,7 +414,7 @@ public static String getExperimentCopiedMessageBody(ExperimentAnnotations source
416414
if(targetExperiment.hasPxid())
417415
{
418416
message.append(NL2)
419-
.append("The ProteomeXchange ID reserved for your data is:")
417+
.append("The ProteomeXchange ID reserved for the data is:")
420418
.append(NL).append(targetExperiment.getPxid())
421419
.append(" (").append(link(pxdLink(targetExperiment.getPxid()))).append(")");
422420

@@ -426,18 +424,24 @@ public static String getExperimentCopiedMessageBody(ExperimentAnnotations source
426424
}
427425
}
428426

427+
if (targetExperiment.hasDoi())
428+
{
429+
// Display the complete link: https://support.datacite.org/docs/datacite-doi-display-guidelines
430+
message.append(NL2).append("The DOI for the data is: ").append(link(getDoiLink(targetExperiment)));
431+
}
432+
429433
message.append(NL2)
430-
.append("The permanent URL (").append(accessUrlLink).append(")")
434+
.append("The permanent link (").append(accessUrlLink).append(")")
431435
.append(" is the unique identifier of your data on ").append(journalName).append(".")
432-
.append(" You can put the permanent URL")
436+
.append(" You can put the permanent link")
433437
.append(targetExperiment.hasPxid() ? " and the ProteomeXchange ID " : " ")
434438
.append("in your manuscript. ");
435439

436440
if (submission.isKeepPrivate())
437441
{
438442
message.append(NL2)
439443
.append("When you are ready to make the data public you can click the \"Make Public\" button in your data folder or click this link: ")
440-
.append(bold(link("Make Data Public", PanoramaPublicController.getMakePublicUrl(targetExperiment.getId(), targetExperiment.getContainer()).getEncodedLocalURIString())));
444+
.append(bold(link("Make Data Public", PanoramaPublicController.getMakePublicUrl(targetExperiment.getId(), targetExperiment.getContainer()).getURIString())));
441445
}
442446

443447
message.append(NL2).append("Best regards,");
@@ -456,15 +460,22 @@ public static String getExperimentCopiedMessageBody(ExperimentAnnotations source
456460
message.append(NL).append(String.format("* %s to folder: ", recopy ? "Recopied": "Copied")).append(getContainerLink(targetExperiment.getContainer()));
457461
if (targetExperiment.hasDoi())
458462
{
459-
message.append(NL).append("* DOI: ").append(link(targetExperiment.getDoi(), "https://doi.org/" + PageFlowUtil.encode(targetExperiment.getDoi())));
463+
// Display the complete link: https://support.datacite.org/docs/datacite-doi-display-guidelines
464+
message.append(NL).append("* DOI: ").append(link(getDoiLink(targetExperiment)));
460465
}
461466

462467
return message.toString();
463468
}
464469

470+
@NotNull
471+
private static String getDoiLink(ExperimentAnnotations targetExperiment)
472+
{
473+
return DataCiteService.toUrl(targetExperiment.getDoi());
474+
}
475+
465476
private static String pxdLink(String pxdAccession)
466477
{
467-
return "http://proteomecentral.proteomexchange.org/cgi/GetDataset?ID=" + pxdAccession;
478+
return ProteomeXchangeService.toUrl(pxdAccession);
468479
}
469480

470481
public static class TestCase extends Assert

‎panoramapublic/src/org/labkey/panoramapublic/datacite/DataCiteService.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import org.jetbrains.annotations.Nullable;
77
import org.json.old.JSONObject;
88
import org.labkey.api.data.PropertyManager;
9+
import org.labkey.api.util.Link;
910
import org.labkey.panoramapublic.model.ExperimentAnnotations;
1011

1112
import java.io.DataOutputStream;
@@ -235,6 +236,23 @@ private static DataCiteConfig getConfig(PropertyManager.PropertyMap map, @Nullab
235236
return config;
236237
}
237238

239+
public static String toUrl(@NotNull String doi)
240+
{
241+
// Regular expression for modern DOIs is /^10.\d{4,9}/[-._;()/:A-Z0-9]+$/i (https://www.crossref.org/blog/dois-and-matching-regular-expressions/)
242+
// DOI example from Panorama Public: 10.6069/9cd7-b485; Prefix: 10.6069, Suffix: 9cd7-b485
243+
// The suffix part of the DOI can be anything (https://support.datacite.org/docs/doi-basics#suffix)
244+
// BUT, auto-generated DOI suffixes contain only a-z, 0-9 and -. (https://support.datacite.org/docs/what-characters-should-i-use-in-the-suffix-of-my-doi)
245+
// So we don't need to URI-encode the DOI.
246+
return "https://doi.org/" + doi;
247+
}
248+
249+
public static Link.LinkBuilder toLink(@NotNull String doi)
250+
{
251+
// Display the complete link: https://support.datacite.org/docs/datacite-doi-display-guidelines
252+
String url = toUrl(doi);
253+
return new Link.LinkBuilder(url).href(url).rel("noopener noreferrer");
254+
}
255+
238256
/**
239257
* Class encapsulating the DataCite account name, password, url, and prefix assigned to the DataCite account (test or live)
240258
*/

‎panoramapublic/src/org/labkey/panoramapublic/pipeline/CopyExperimentFinalTask.java‎

Lines changed: 81 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@
3838
import org.labkey.api.query.BatchValidationException;
3939
import org.labkey.api.query.ValidationException;
4040
import org.labkey.api.security.Group;
41+
import org.labkey.api.security.InvalidGroupMembershipException;
42+
import org.labkey.api.security.MemberType;
4143
import org.labkey.api.security.MutableSecurityPolicy;
4244
import org.labkey.api.security.SecurityManager;
4345
import org.labkey.api.security.SecurityPolicy;
@@ -87,9 +89,11 @@
8789
import java.nio.file.Files;
8890
import java.nio.file.Path;
8991
import java.util.ArrayList;
92+
import java.util.Arrays;
9093
import java.util.Collections;
9194
import java.util.Date;
9295
import java.util.List;
96+
import java.util.Objects;
9397
import java.util.Set;
9498
import java.util.stream.Collectors;
9599

@@ -327,6 +331,12 @@ private Pair<User, String> assignReviewer(JournalSubmission js, ExperimentAnnota
327331
assignReader(reviewer, targetExperiment.getContainer());
328332
js.getJournalExperiment().setReviewer(reviewer.getUserId());
329333
SubmissionManager.updateJournalExperiment(js.getJournalExperiment(), user);
334+
335+
// Add reviewer to the "Reviewers" group
336+
// This group already exists in the Panorama Public project on panoramaweb.org.
337+
// CONSIDER: Make this configurable through the Panorama Public admin console.
338+
addToGroup(reviewer, "Reviewers", targetExperiment.getContainer().getProject(), log);
339+
330340
return new Pair<>(reviewer, reviewerPassword);
331341
}
332342
}
@@ -453,14 +463,14 @@ private ExperimentAnnotations createNewExperimentAnnotations(ExpExperiment exper
453463
targetExperiment = ExperimentAnnotationsManager.save(targetExperiment, user);
454464

455465
// Update the target of the short access URL to the journal's copy of the experiment.
456-
log.info("Updating access URL to point to the new copy of the data.");
466+
log.info("Updating permanent link to point to the new copy of the data.");
457467
try
458468
{
459469
SubmissionManager.updateAccessUrlTarget(js.getShortAccessUrl(), targetExperiment, user);
460470
}
461471
catch (ValidationException e)
462472
{
463-
throw new PipelineJobException("Error updating the target of the short access URL '" + js.getShortAccessUrl().getShortURL() + "' to '"
473+
throw new PipelineJobException("Error updating the target of the permanent link '" + js.getShortAccessUrl().getShortURL() + "' to '"
464474
+ targetExperiment.getContainer().getPath() + "'", e);
465475
}
466476

@@ -507,10 +517,13 @@ private void updatePermissions(User formSubmitter, ExperimentAnnotations targetE
507517
{
508518
newPolicy.addRoleAssignment(folderAdmin, ReaderRole.class);
509519
}
520+
510521
// Assign the PanoramaPublicSubmitterRole so that the submitter or lab head is able to make the copied folder public, and add publication information.
511-
assignPanoramaPublicSubmitterRole(targetExperiment.getSubmitterUser(), newPolicy);
512-
assignPanoramaPublicSubmitterRole(targetExperiment.getLabHeadUser(), newPolicy);
513-
assignPanoramaPublicSubmitterRole(formSubmitter, newPolicy); // User that submitted the form. Can be different from the user selected as the data "submitter".
522+
assignPanoramaPublicSubmitterRole(newPolicy, log, targetExperiment.getSubmitterUser(), targetExperiment.getLabHeadUser(),
523+
formSubmitter); // User that submitted the form. Can be different from the user selected as the data submitter
524+
525+
addToSubmittersGroup(target.getProject(), log, targetExperiment.getSubmitterUser(), targetExperiment.getLabHeadUser(), formSubmitter);
526+
514527

515528
if (previousCopy != null)
516529
{
@@ -524,12 +537,71 @@ private void updatePermissions(User formSubmitter, ExperimentAnnotations targetE
524537
SecurityPolicyManager.savePolicy(newPolicy);
525538
}
526539

527-
private void assignPanoramaPublicSubmitterRole(User user, MutableSecurityPolicy policy)
540+
private void assignPanoramaPublicSubmitterRole(MutableSecurityPolicy policy, Logger log, User... users)
528541
{
529-
if (user != null)
530-
{
542+
Arrays.stream(users).filter(Objects::nonNull).forEach(user -> {
543+
log.info("Assigning " + PanoramaPublicSubmitterRole.class.getSimpleName() + " to " + user.getEmail());
531544
policy.addRoleAssignment(user, PanoramaPublicSubmitterRole.class, false);
545+
});
546+
}
547+
548+
private void addToSubmittersGroup(Container project, Logger log, User... users)
549+
{
550+
String groupName = "Panorama Public Submitters";
551+
Group group = getGroup(groupName, project, log);
552+
if (group != null)
553+
{
554+
Arrays.stream(users).filter(Objects::nonNull).forEach(user -> {
555+
// This group already exists in the Panorama Public project on panoramaweb.org.
556+
// CONSIDER: Make this configurable through the Panorama Public admin console.
557+
addToGroup(user, group, log);
558+
});
559+
}
560+
}
561+
562+
private void addToGroup(User user, String groupName, Container project, Logger log)
563+
{
564+
Group group = getGroup(groupName, project, log);
565+
if (group != null)
566+
{
567+
addToGroup(user, group, log);
568+
return;
569+
}
570+
}
571+
572+
private void addToGroup(User user, Group group, Logger log)
573+
{
574+
try
575+
{
576+
if (SecurityManager.getGroupMembers(group, MemberType.ACTIVE_USERS).contains(user))
577+
{
578+
log.info("User " + user.getEmail() + " is already a member of group " + group.getName());
579+
}
580+
else
581+
{
582+
log.info("Adding user " + user.getEmail() + " to group " + group.getName());
583+
SecurityManager.addMember(group, user);
584+
}
585+
}
586+
catch (InvalidGroupMembershipException e)
587+
{
588+
log.warn("Unable to add user " + user.getEmail() + " to group " + group.getName(), e);
589+
}
590+
}
591+
592+
private Group getGroup(String groupName, Container project, Logger log)
593+
{
594+
Integer groupId = SecurityManager.getGroupId(project, groupName, false);
595+
Group group = null;
596+
if (groupId != null)
597+
{
598+
group = SecurityManager.getGroup(groupId);
599+
}
600+
if (group == null)
601+
{
602+
log.warn("Did not find a security group with name " + groupName + " in the project " + project.getName());
532603
}
604+
return group;
533605
}
534606

535607
private List<User> getUsersWithRole(Container container, Role role)
@@ -818,7 +890,7 @@ private void updateLastCopiedExperiment(ExperimentAnnotations previousCopy, Jour
818890
throw new PipelineJobException("Error saving shortUrl '" + versionedShortUrl + "'", e);
819891
}
820892

821-
log.info("Setting the short access URL on the previous copy to " + newShortUrl.getShortURL());
893+
log.info("Setting the permanent link on the previous copy to " + newShortUrl.getShortURL());
822894
previousCopy.setShortUrl(newShortUrl);
823895
if (previousCopy.getDoi() != null)
824896
{

0 commit comments

Comments
 (0)