Skip to content

Commit

Permalink
The user is informed during project creation if a provided projectcod…
Browse files Browse the repository at this point in the history
…e is not unique (#625)

* Validate against project code within the stepper and only generate unique project codes

* Address Code Review

Add maximum number of retries for project code.

* Address Code Review

Replace return value for finding a project by projectCode from list to optional

* Apply Code Review:

Extract uniqueProjectCode Validation and check in separate methods in ProjectDesignLayout

# Co-authored-by: KochTobi <[email protected]>

* Remove unnecessary double parenthesis

* Apply Code Review

Propagate boolean instead of project to ensure that project information cannot be accessed.

# Co-authored-by: KochTobi <[email protected]>

---------

Co-authored-by: Tobias Koch <[email protected]>
  • Loading branch information
Steffengreiner and KochTobi authored Jun 14, 2024
1 parent 30c293d commit 15f8161
Show file tree
Hide file tree
Showing 7 changed files with 113 additions and 59 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import static life.qbic.logging.service.LoggerFactory.logger;

import java.time.Instant;
import java.util.List;
import java.util.Optional;
import life.qbic.logging.api.Logger;
import life.qbic.projectmanagement.application.authorization.QbicUserDetails;
Expand All @@ -15,12 +14,10 @@
import life.qbic.projectmanagement.domain.model.project.ProjectId;
import life.qbic.projectmanagement.domain.repository.ProjectRepository;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.security.access.prepost.PostFilter;
import org.springframework.security.access.prepost.PreAuthorize;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.stereotype.Component;
import org.springframework.transaction.annotation.Transactional;


/**
Expand Down Expand Up @@ -90,9 +87,8 @@ public void update(Project project) {
}

@Override
@PostFilter("hasPermission(filterObject, 'READ')")
public List<Project> find(ProjectCode projectCode) {
return projectRepo.findProjectByProjectCode(projectCode);
public boolean existsProjectByProjectCode(ProjectCode projectCode) {
return projectRepo.existsProjectByProjectCode(projectCode);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package life.qbic.projectmanagement.infrastructure.project;

import java.util.List;
import life.qbic.projectmanagement.domain.model.project.Project;
import life.qbic.projectmanagement.domain.model.project.ProjectCode;
import life.qbic.projectmanagement.domain.model.project.ProjectId;
Expand All @@ -19,6 +18,6 @@
*/
public interface QbicProjectRepo extends CrudRepository<Project, ProjectId> {

List<Project> findProjectByProjectCode(ProjectCode projectCode);
boolean existsProjectByProjectCode(ProjectCode projectCode);

}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import life.qbic.projectmanagement.domain.model.project.Contact;
import life.qbic.projectmanagement.domain.model.project.Funding;
import life.qbic.projectmanagement.domain.model.project.Project;
import life.qbic.projectmanagement.domain.model.project.ProjectCode;
import life.qbic.projectmanagement.domain.model.project.ProjectId;
import life.qbic.projectmanagement.domain.model.project.ProjectObjective;
import life.qbic.projectmanagement.domain.model.project.ProjectTitle;
Expand Down Expand Up @@ -91,6 +92,12 @@ public Optional<Project> find(String projectId) throws IllegalArgumentException
return find(ProjectId.parse(projectId));
}

public boolean isProjectCodeUnique(String projectCode) throws IllegalArgumentException {
boolean isUnique = !projectRepository.existsProjectByProjectCode(
ProjectCode.parse(projectCode));
return isUnique;
}

@PreAuthorize("hasPermission(#projectId, 'life.qbic.projectmanagement.domain.model.project.Project','READ')")
private Project loadProject(ProjectId projectId) {
Objects.requireNonNull(projectId);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package life.qbic.projectmanagement.domain.repository;

import java.time.Instant;
import java.util.List;
import java.util.Optional;
import life.qbic.projectmanagement.domain.model.project.Project;
import life.qbic.projectmanagement.domain.model.project.ProjectCode;
Expand Down Expand Up @@ -34,13 +33,13 @@ public interface ProjectRepository {
void update(Project project);

/**
* Searches for projects that contain the provided project code
* Searches for a project that contain the provided project code
*
* @param projectCode the project code to search for in projects
* @return projects that contain the project code
* @return boolean indicating that a project with the specified code already exists
* @since 1.0.0
*/
List<Project> find(ProjectCode projectCode);
boolean existsProjectByProjectCode(ProjectCode projectCode);

Optional<Project> find(ProjectId projectId);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import life.qbic.datamanager.views.projects.create.ProjectDesignLayout.ProjectDesign;
import life.qbic.finances.api.FinanceService;
import life.qbic.projectmanagement.application.ContactRepository;
import life.qbic.projectmanagement.application.ProjectInformationService;
import life.qbic.projectmanagement.application.ontology.OntologyLookupService;
import life.qbic.projectmanagement.domain.model.project.Project;

Expand Down Expand Up @@ -66,15 +67,17 @@ private StepIndicator addStep(Stepper stepper, String label, Component layout) {
return stepper.addStep(label);
}

public AddProjectDialog(FinanceService financeService,
public AddProjectDialog(ProjectInformationService projectInformationService,
FinanceService financeService,
OntologyLookupService ontologyLookupService,
ContactRepository contactRepository) {
super();
addClassName("add-project-dialog");
requireNonNull(projectInformationService, "project information service must not be null");
requireNonNull(financeService, "financeService must not be null");
requireNonNull(ontologyLookupService,
"ontologyTermInformationService must not be null");
this.projectDesignLayout = new ProjectDesignLayout(financeService);
this.projectDesignLayout = new ProjectDesignLayout(projectInformationService, financeService);
this.fundingInformationLayout = new FundingInformationLayout();
this.collaboratorsLayout = new CollaboratorsLayout();
this.experimentalInformationLayout = new ExperimentalInformationLayout(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,22 @@
import com.vaadin.flow.component.textfield.TextField;
import com.vaadin.flow.data.binder.Binder;
import com.vaadin.flow.data.binder.ValidationException;
import com.vaadin.flow.data.binder.ValidationResult;
import com.vaadin.flow.data.binder.ValueContext;
import com.vaadin.flow.data.renderer.ComponentRenderer;
import com.vaadin.flow.data.value.ValueChangeMode;
import jakarta.validation.constraints.NotEmpty;
import java.io.Serial;
import java.io.Serializable;
import java.util.Objects;
import java.util.Optional;
import life.qbic.datamanager.views.general.HasBinderValidation;
import life.qbic.datamanager.views.projects.create.ProjectDesignLayout.ProjectDesign;
import life.qbic.finances.api.FinanceService;
import life.qbic.finances.api.Offer;
import life.qbic.finances.api.OfferSummary;
import life.qbic.logging.api.Logger;
import life.qbic.projectmanagement.application.ProjectInformationService;
import life.qbic.projectmanagement.domain.model.project.ProjectCode;
import life.qbic.projectmanagement.domain.model.project.ProjectObjective;
import life.qbic.projectmanagement.domain.model.project.ProjectTitle;
Expand All @@ -49,17 +53,44 @@ public class ProjectDesignLayout extends Div implements HasBinderValidation<Proj
private final TextArea projectDescription = new TextArea("Description");
private final Button generateCodeButton = new Button(new Icon(VaadinIcon.REFRESH));
private final Binder<ProjectDesign> projectDesignBinder = new Binder<>(ProjectDesign.class);
private final FinanceService financeService;
private final transient ProjectInformationService projectInformationService;
private final transient FinanceService financeService;
private final Span projectDesignDescription = new Span(
"Specify the name and objective of the research project.");

public ProjectDesignLayout(FinanceService financeService) {
this.financeService = financeService;
public ProjectDesignLayout(ProjectInformationService projectInformationService,
FinanceService financeService) {
this.projectInformationService = Objects.requireNonNull(projectInformationService,
"Project Information service cannot be null");
this.financeService = Objects.requireNonNull(financeService, "Finance Service cannot be null");
initLayout();
initFieldValidators();
bindOfferDataProvider(financeService);
}

private static void addConsumedLengthHelper(TextField textField, String newValue) {
int maxLength = textField.getMaxLength();
int consumedLength = newValue.length();
textField.setHelperText(consumedLength + "/" + maxLength);
}

private static void addConsumedLengthHelper(TextArea textArea, String newValue) {
int maxLength = textArea.getMaxLength();
int consumedLength = newValue.length();
textArea.setHelperText(consumedLength + "/" + maxLength);
}

/**
* Render the preview like `#offer-id, #project title`
*
* @param offerSummary the offer preview
* @return the formatted String representation
* @since 1.0.0
*/
private static String previewToString(OfferSummary offerSummary) {
return offerSummary.offerId() + ", " + offerSummary.title();
}

private void initLayout() {
Span projectDesignTitle = new Span(TITLE);
projectDesignTitle.addClassName("title");
Expand Down Expand Up @@ -96,17 +127,38 @@ public void enableOfferSearch() {
private void initCodeGenerationButton() {
generateCodeButton.setTooltipText("Generate Project ID");
generateCodeButton.addThemeVariants(ButtonVariant.LUMO_ICON);
generateCodeButton.addClickListener(
buttonClickEvent -> codeField.setValue(ProjectCode.random().value()));
generateCodeButton.addClickListener(event -> {
var projectCode = generateUniqueProjectCode();
codeField.setValue(projectCode);
});
}


private String generateUniqueProjectCode() {
String newProjectCode = ProjectCode.random().value();
boolean isProjectCodeDuplicated = !isProjectCodeUnique(newProjectCode);
int retries = 0;
final int MAX_NUMBER_OF_RETRIES = 20;
while (isProjectCodeDuplicated) {
//Ensure that we have an exit condition and the while loop does not iterate endlessly if most project codes are taken
if (retries > MAX_NUMBER_OF_RETRIES) {
break;
}
newProjectCode = ProjectCode.random().value();
isProjectCodeDuplicated = isProjectCodeUnique(newProjectCode);
retries++;
}
return newProjectCode;
}

private void initFieldValidators() {
codeField.setRequired(true);
titleField.setRequired(true);
projectDescription.setRequired(true);
projectDesignBinder.forField(codeField).withValidator(ProjectCode::isValid,
"A project code starts with Q followed by 4 letters/numbers").
bind(ProjectDesign::getProjectCode, ProjectDesign::setProjectCode);
"A project code starts with Q followed by 4 letters/numbers")
.withValidator(this::uniqueProjectCodeValidator)
.bind(ProjectDesign::getProjectCode, ProjectDesign::setProjectCode);
projectDesignBinder.forField(titleField)
.withValidator(it -> !it.isBlank(), "Please provide a project title")
.bind((ProjectDesign::getProjectTitle),
Expand All @@ -119,6 +171,16 @@ private void initFieldValidators() {
restrictProjectObjectiveLength();
}

private ValidationResult uniqueProjectCodeValidator(String value, ValueContext context) {
if (isProjectCodeUnique(value)) {
return ValidationResult.ok();
} else {
return ValidationResult.error(
String.format("Project code %s is already taken. Please choose a different code",
value));
}
}

private void restrictProjectTitleLength() {
titleField.setMaxLength((int) ProjectTitle.maxLength());
titleField.setValueChangeMode(ValueChangeMode.EAGER);
Expand All @@ -134,18 +196,6 @@ private void restrictProjectObjectiveLength() {
e -> addConsumedLengthHelper(e.getSource(), e.getValue()));
}

private static void addConsumedLengthHelper(TextField textField, String newValue) {
int maxLength = textField.getMaxLength();
int consumedLength = newValue.length();
textField.setHelperText(consumedLength + "/" + maxLength);
}

private static void addConsumedLengthHelper(TextArea textArea, String newValue) {
int maxLength = textArea.getMaxLength();
int consumedLength = newValue.length();
textArea.setHelperText(consumedLength + "/" + maxLength);
}

private void bindOfferDataProvider(FinanceService financeService) {
offerSearchField.setItems(
query -> financeService.findOfferContainingProjectTitleOrId(
Expand Down Expand Up @@ -173,22 +223,16 @@ private void setOffer(String offerId) {
() -> log.error("No offer found with id: " + offerId));
}

/**
* Render the preview like `#offer-id, #project title`
*
* @param offerSummary the offer preview
* @return the formatted String representation
* @since 1.0.0
*/
private static String previewToString(OfferSummary offerSummary) {
return offerSummary.offerId() + ", " + offerSummary.title();
}

private void fillProjectInformationFromOffer(Offer offer) {
titleField.setValue(offer.title());
projectDescription.setValue(offer.objective().replace("\n", " "));
}

private boolean isProjectCodeUnique(String projectCode) {
return projectInformationService.isProjectCodeUnique(projectCode);

}


/**
* Returns the project design. Fails for invalid designs with an exception.
Expand Down Expand Up @@ -233,36 +277,36 @@ public static final class ProjectDesign implements Serializable {
@NotEmpty
private String projectObjective = "";

public void setOfferId(String offerId) {
this.offerId = offerId;
public String getOfferId() {
return offerId;
}

public void setProjectTitle(String projectTitle) {
this.projectTitle = projectTitle;
public void setOfferId(String offerId) {
this.offerId = offerId;
}

public void setProjectObjective(String projectObjective) {
this.projectObjective = projectObjective;
public String getProjectCode() {
return projectCode;
}

public void setProjectCode(String projectCode) {
this.projectCode = projectCode;
}

public String getOfferId() {
return offerId;
}

public String getProjectCode() {
return projectCode;
}

public String getProjectTitle() {
return projectTitle;
}

public void setProjectTitle(String projectTitle) {
this.projectTitle = projectTitle;
}

public String getProjectObjective() {
return projectObjective;
}

public void setProjectObjective(String projectObjective) {
this.projectObjective = projectObjective;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import life.qbic.projectmanagement.application.AddExperimentToProjectService;
import life.qbic.projectmanagement.application.ContactRepository;
import life.qbic.projectmanagement.application.ProjectCreationService;
import life.qbic.projectmanagement.application.ProjectInformationService;
import life.qbic.projectmanagement.application.ontology.OntologyLookupService;
import life.qbic.projectmanagement.domain.model.project.Funding;
import life.qbic.projectmanagement.domain.model.project.Project;
Expand All @@ -53,6 +54,7 @@ public class ProjectOverviewMain extends Main {
private static final Logger log = logger(ProjectOverviewMain.class);
private final ProjectCollectionComponent projectCollectionComponent;
private final transient ProjectCreationService projectCreationService;
private final transient ProjectInformationService projectInformationService;
private final transient FinanceService financeService;
private final transient OntologyLookupService ontologyTermInformationService;
private final transient AddExperimentToProjectService addExperimentToProjectService;
Expand All @@ -61,6 +63,7 @@ public class ProjectOverviewMain extends Main {

public ProjectOverviewMain(@Autowired ProjectCollectionComponent projectCollectionComponent,
ProjectCreationService projectCreationService, FinanceService financeService,
ProjectInformationService projectInformationService,
OntologyLookupService ontologyTermInformationService,
AddExperimentToProjectService addExperimentToProjectService,
UserInformationService userInformationService,
Expand All @@ -70,6 +73,8 @@ public ProjectOverviewMain(@Autowired ProjectCollectionComponent projectCollecti
this.projectCreationService = Objects.requireNonNull(projectCreationService,
"project creation service can not be null");
this.financeService = Objects.requireNonNull(financeService, "finance service can not be null");
this.projectInformationService = Objects.requireNonNull(projectInformationService,
"project information service can not be null");
this.ontologyTermInformationService = Objects.requireNonNull(ontologyTermInformationService,
"ontology term information service can not be null");
this.addExperimentToProjectService = Objects.requireNonNull(addExperimentToProjectService,
Expand All @@ -81,7 +86,8 @@ public ProjectOverviewMain(@Autowired ProjectCollectionComponent projectCollecti
addTitleAndDescription();
add(projectCollectionComponent);
this.projectCollectionComponent.addCreateClickedListener(projectCreationClickedEvent -> {
AddProjectDialog addProjectDialog = new AddProjectDialog(this.financeService,
AddProjectDialog addProjectDialog = new AddProjectDialog(this.projectInformationService,
this.financeService,
this.ontologyTermInformationService, this.contactRepository);
if (isOfferSearchAllowed()) {
addProjectDialog.enableOfferSearch();
Expand Down

0 comments on commit 15f8161

Please sign in to comment.