Skip to content

Bug/bi 1158 #117

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.

Sign up for GitHub

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

Merged
davedrp merged 6 commits into from
Dec 2, 2021
Merged

Bug/bi 1158 #117

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9879152
[BI-1158] removed validations for abbreviations
davedrp Nov 15, 2021
a647e26
[BI-1158] Reomove abbreviations for table
davedrp Nov 16, 2021
4d04f43
[BI-1158] change column names
davedrp Nov 17, 2021
5093648
[BI-1158] logic for Scale Unit
davedrp Nov 22, 2021
68305bc
[BI-1158] update test and remove abbreviations from trait table
davedrp Nov 22, 2021
ef34383
[BI-1158] incorporated comments form PR
davedrp Nov 29, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions src/main/java/org/breedinginsight/daos/TraitDAO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,6 @@ public List<Trait> createTraitsBrAPI(List<Trait> traits, User actingUser, Progra
.status("active")
.entity(trait.getEntity())
.mainAbbreviation(trait.getMainAbbreviation())
.alternativeAbbreviations(trait.getAbbreviations() != null ? List.of(trait.getAbbreviations()) : null)
.traitClass(trait.getTraitClass())
.externalReferences(List.of(traitReference))
.attribute(trait.getAttribute());
Expand Down Expand Up @@ -407,7 +406,6 @@ public Trait updateTraitBrAPI(Trait trait, Program program) {
existingVariable.getTrait().setSynonyms(trait.getSynonyms());
existingVariable.getTrait().setEntity(trait.getProgramObservationLevel().getName());
existingVariable.getTrait().setMainAbbreviation(trait.getMainAbbreviation());
existingVariable.getTrait().setAlternativeAbbreviations(trait.getAbbreviations() != null ? List.of(trait.getAbbreviations()) : null);
existingVariable.getTrait().setTraitClass(trait.getTraitClass());
existingVariable.getTrait().setAttribute(trait.getAttribute());

Expand Down Expand Up @@ -541,7 +539,6 @@ public List<Trait> getTraitsByAbbreviation(UUID programId, List<String> abbrevia
.from(TRAIT)
.join(PROGRAM_ONTOLOGY).on(TRAIT.PROGRAM_ONTOLOGY_ID.eq(PROGRAM_ONTOLOGY.ID))
.join(PROGRAM).on(PROGRAM_ONTOLOGY.PROGRAM_ID.eq(PROGRAM.ID))
.where(TRAIT.ABBREVIATIONS.cast(String[].class).contains(abbreviations.toArray(String[]::new)))
.and(PROGRAM.ID.eq(programId))
.fetch();

Expand Down
8 changes: 0 additions & 8 deletions src/main/java/org/breedinginsight/model/Trait.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,18 +74,11 @@ public class Trait extends TraitEntity {
// It is not populated by the database.
private Boolean isDup = null;

@Override
@JsonDeserialize(using = ArrayOfStringDeserializer.class)
public void setAbbreviations(String... abbreviations) {
super.setAbbreviations(abbreviations);
}

public Trait(TraitEntity traitEntity) {
this.setId(traitEntity.getId());
this.setMethodId(traitEntity.getMethodId());
this.setScaleId(traitEntity.getScaleId());
this.setObservationVariableName(traitEntity.getObservationVariableName());
this.setAbbreviations(traitEntity.getAbbreviations());
this.setProgramOntologyId(traitEntity.getProgramOntologyId());
this.setProgramObservationLevelId(traitEntity.getProgramObservationLevelId());
this.setCreatedAt(traitEntity.getCreatedAt());
Expand All @@ -101,7 +94,6 @@ public static Trait parseSqlRecord(Record record) {
.methodId(record.getValue(TRAIT.METHOD_ID))
.scaleId(record.getValue(TRAIT.SCALE_ID))
.observationVariableName(record.getValue(TRAIT.OBSERVATION_VARIABLE_NAME))
.abbreviations(record.getValue(TRAIT.ABBREVIATIONS))
.programOntologyId(record.getValue(TRAIT.PROGRAM_ONTOLOGY_ID))
.programObservationLevelId(record.getValue(TRAIT.PROGRAM_OBSERVATION_LEVEL_ID))
.createdAt(record.getValue(TRAIT.CREATED_AT))
Expand Down
14 changes: 0 additions & 14 deletions src/main/java/org/breedinginsight/services/TraitService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,6 @@ public List<Trait> createTraits(UUID programId, List<Trait> traits, Authenticate
// Create trait
TraitEntity jooqTrait = TraitEntity.builder()
.observationVariableName(trait.getObservationVariableName())
.abbreviations(trait.getAbbreviations())
.programOntologyId(programOntology.getId())
.programObservationLevelId(trait.getProgramObservationLevel().getId())
.methodId(jooqMethod.getId())
Expand Down Expand Up @@ -265,7 +264,6 @@ private void checkAndPrepareTraits(UUID programId, List<Trait> traits, Boolean t
// Ignore duplicate traits
ValidationErrors duplicateErrors = new ValidationErrors();
List<Trait> duplicateTraits = traitValidator.checkDuplicateTraitsExistingByName(programId, traits);
List<Trait> duplicateTraitsByAbbrev = traitValidator.checkDuplicateTraitsExistingByAbbreviation(programId, traits);
List<Integer> traitIndexToRemove = new ArrayList<>();
for (Trait duplicateTrait: duplicateTraits){

Expand All @@ -278,16 +276,6 @@ private void checkAndPrepareTraits(UUID programId, List<Trait> traits, Boolean t
}
}

for (Trait duplicateTraitAbbrev: duplicateTraitsByAbbrev){

Integer i = traits.indexOf(duplicateTraitAbbrev);
if (i == -1){
throw new InternalServerException("Duplicate trait was not referenced correctly");
} else {
duplicateErrors.addError(traitValidatorError.getRowNumber(i), traitValidatorError.getDuplicateTraitByAbbreviationsMsg());
traitIndexToRemove.add(i);
}
}

// Check the rest of our validations
Optional<ValidationErrors> optionalValidationErrors = traitValidator.checkAllTraitValidations(traits, traitValidatorError);
Expand All @@ -298,7 +286,6 @@ private void checkAndPrepareTraits(UUID programId, List<Trait> traits, Boolean t
// Remove our duplicate traits if we are not going to throw an error on them
if (!throwDuplicateErrors) {
Set<Trait> duplicateTraitSet = new HashSet<>(duplicateTraits);
duplicateTraitSet.addAll(duplicateTraitsByAbbrev);
traits.removeAll(duplicateTraitSet);
}

Expand Down Expand Up @@ -406,7 +393,6 @@ public List<Trait> updateTraits(UUID programId, List<Trait> traits, Authenticate

// Update trait
existingTraitEntity.setObservationVariableName(updatedTrait.getObservationVariableName());
existingTraitEntity.setAbbreviations(updatedTrait.getAbbreviations());
existingTraitEntity.setProgramObservationLevelId(updatedTrait.getProgramObservationLevel().getId());
existingTraitEntity.setUpdatedBy(user.getId());
traitDAO.update(existingTraitEntity);
Expand Down
43 changes: 40 additions & 3 deletions src/main/java/org/breedinginsight/services/TraitUploadService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,11 @@
import org.apache.tika.mime.MediaType;
import org.breedinginsight.api.auth.AuthenticatedUser;
import org.breedinginsight.api.model.v1.response.ValidationErrors;
import org.breedinginsight.dao.db.enums.DataType;
import org.breedinginsight.dao.db.enums.UploadType;
import org.breedinginsight.daos.ProgramUploadDAO;
import org.breedinginsight.model.ProgramUpload;
import org.breedinginsight.model.Scale;
import org.breedinginsight.services.constants.SupportedMediaType;
import org.breedinginsight.services.exceptions.*;

Expand Down Expand Up @@ -124,7 +126,7 @@ public ProgramUpload updateTraitUpload(UUID programId, CompletedFileUpload file,
traitService.preprocessTraits(traits);

ValidationErrors validationErrors = new ValidationErrors();

traits = this.markScaleNames(traits);
Optional<ValidationErrors> optionalValidationErrors = traitValidator.checkAllTraitValidations(traits, traitValidatorError);
if (optionalValidationErrors.isPresent()){
validationErrors.merge(optionalValidationErrors.get());
Expand All @@ -135,6 +137,7 @@ public ProgramUpload updateTraitUpload(UUID programId, CompletedFileUpload file,
}

traitService.assignTraitsProgramObservationLevel(traits, programId);

traits = this.markDups(traits, programId);

String json = null;
Expand Down Expand Up @@ -170,7 +173,6 @@ public ProgramUpload updateTraitUpload(UUID programId, CompletedFileUpload file,
}
private List<Trait> markDups(List<Trait> traits, UUID programId){
List<Trait> dupTraitsByName = this.traitValidator.checkDuplicateTraitsExistingByName(programId, traits);
List<Trait> dupAbbrevTraits = this.traitValidator.checkDuplicateTraitsExistingByAbbreviation(programId, traits);

for(Trait trait: traits){
boolean isDup = false;
Expand All @@ -182,12 +184,47 @@ private List<Trait> markDups(List<Trait> traits, UUID programId){
}
}

isDup = isDup || this.traitValidator.hasAbbreviation(dupAbbrevTraits, trait, true);
trait.setIsDup(isDup);
}

return traits;
}

private List<Trait> markScaleNames(List<Trait> traits){
for(Trait trait: traits){
Scale scale = trait.getScale();
if(scale != null){
scale.setScaleName( calcDefaultScaleName(trait) );
}
}
return traits;
}

private String calcDefaultScaleName(Trait trait){
Scale scale = trait.getScale();
if(scale != null){
String scaleName = scale.getScaleName();
// if scaleName already exist, use it
if(scaleName != null) {
return scaleName;
}
else {
DataType datatype = scale.getDataType();
if( datatype != null ) {
// if the dataType is not Numerical, use the dataType value
if(datatype!=DataType.NUMERICAL){
Copy link
Member

@timparsons timparsons Nov 30, 2021

Choose a reason for hiding this comment

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

Does this mean that if the datatype is NUMERICAL that there will not be a scale name derived?

Copy link
Contributor Author

@davedrp davedrp Dec 2, 2021

Choose a reason for hiding this comment

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

Correct, the user is requried to provide the scale name (aka unit) if the datatype is NUMERIC.

String defaultValue = datatype.getLiteral();
//Make Title Case
String titleCase =
defaultValue.substring(0, 1).toUpperCase() +
defaultValue.substring(1).toLowerCase();
return titleCase;
}
}
}
}
return null;
}

public Optional<ProgramUpload<Trait>> getTraitUpload(UUID programId, AuthenticatedUser actingUser) {

Expand Down
11 changes: 5 additions & 6 deletions src/main/java/org/breedinginsight/services/parsers/trait/TraitFileColumns.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,16 @@

public enum TraitFileColumns {

ONTOLOGY_TERM_NAME("Ontology term name"),
TRAIT_ABBREVIATIONS("Trait abbreviations"),
TRAIT_SYNONYMS("Trait synonyms"),
NAME("Name"),
SYNONYMS("Synonyms"),
TRAIT_ENTITY("Trait entity"),
TRAIT_ATTRIBUTE("Trait attribute"),
TRAIT_DESCRIPTION("Trait description"),
TRAIT_STATUS("Trait status"),
DESCRIPTION("Description"),
STATUS("Status"),
METHOD_DESCRIPTION("Method description"),
METHOD_CLASS("Method class"),
METHOD_FORMULA("Method formula"),
SCALE_NAME("Scale name"),
SCALE_NAME("Units"),
SCALE_CLASS("Scale class"),
SCALE_DECIMAL_PLACES("Scale decimal places"),
SCALE_LOWER_LIMIT("Scale lower limit"),
Expand Down
20 changes: 11 additions & 9 deletions src/main/java/org/breedinginsight/services/parsers/trait/TraitFileParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,12 +130,12 @@ private List<Trait> excelRecordsToTraits(List<ExcelRecord> records) throws Valid


Boolean active;
String traitStatus = parseExcelValueAsString(record, TraitFileColumns.TRAIT_STATUS);
String traitStatus = parseExcelValueAsString(record, TraitFileColumns.STATUS);
if (traitStatus == null) {
active = true;
} else {
if (!TRAIT_STATUS_VALID_VALUES.contains(traitStatus.toLowerCase())) {
ValidationError error = new ValidationError(TraitFileColumns.TRAIT_STATUS.toString(),
ValidationError error = new ValidationError(TraitFileColumns.STATUS.toString(),
ParsingExceptionType.INVALID_TRAIT_STATUS.toString(), HttpStatus.UNPROCESSABLE_ENTITY);
validationErrors.addError(traitValidatorError.getRowNumber(i), error);
}
Expand All @@ -158,6 +158,12 @@ private List<Trait> excelRecordsToTraits(List<ExcelRecord> records) throws Valid
if (dataTypeString != null) {
try {
dataType = DataType.valueOf(dataTypeString.toUpperCase());

//This if statement can be removed once DURATION is no longer a valid Data Type
if( DataType.DURATION == dataType ){
throw new IllegalArgumentException("DURATION is not a valid datatype for batch uploading");
}

} catch (IllegalArgumentException e) {
log.error(e.getMessage());
ValidationError error = new ValidationError(TraitFileColumns.SCALE_CLASS.toString(),
Expand Down Expand Up @@ -230,21 +236,17 @@ private List<Trait> excelRecordsToTraits(List<ExcelRecord> records) throws Valid
.categories(categories)
.build();

String abbreviationsString = parseExcelValueAsString(record, TraitFileColumns.TRAIT_ABBREVIATIONS);
List<String> traitAbbreviations = parseListValue(abbreviationsString);

String synonymsString = parseExcelValueAsString(record, TraitFileColumns.TRAIT_SYNONYMS);
String synonymsString = parseExcelValueAsString(record, TraitFileColumns.SYNONYMS);
List<String> traitSynonyms = parseListValue(synonymsString);

String tagsString = parseExcelValueAsString(record, TraitFileColumns.TAGS);
List<String> traitTags = parseListValue(tagsString);

Trait trait = Trait.builder()
.observationVariableName(parseExcelValueAsString(record, TraitFileColumns.ONTOLOGY_TERM_NAME))
.traitDescription(parseExcelValueAsString(record, TraitFileColumns.TRAIT_DESCRIPTION))
.observationVariableName(parseExcelValueAsString(record, TraitFileColumns.NAME))
.traitDescription(parseExcelValueAsString(record, TraitFileColumns.DESCRIPTION))
.entity(parseExcelValueAsString(record, TraitFileColumns.TRAIT_ENTITY))
.attribute(parseExcelValueAsString(record, TraitFileColumns.TRAIT_ATTRIBUTE))
.abbreviations(traitAbbreviations.toArray(String[]::new))
.synonyms(traitSynonyms)
.programObservationLevel(level)
.active(active)
Expand Down
17 changes: 2 additions & 15 deletions src/main/java/org/breedinginsight/services/validators/TraitFileValidatorError.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public ValidationError getMissingScaleMsg() {

@Override
public ValidationError getMissingScaleNameMsg() {
return new ValidationError("Scale name", "Missing scale name", HttpStatus.UNPROCESSABLE_ENTITY);
return new ValidationError("Unit", "Missing unit", HttpStatus.UNPROCESSABLE_ENTITY);
}

@Override
Expand Down Expand Up @@ -158,12 +158,7 @@ public ValidationError getCharLimitMethodDescriptionMsg() {
public ValidationError getDuplicateTraitByNamesMsg() {
return new ValidationError("Trait name", "Trait name already exists", HttpStatus.CONFLICT);
}

@Override
public ValidationError getDuplicateTraitByAbbreviationsMsg() {
return new ValidationError("Trait abbreviations", "Trait abbreviation already exists", HttpStatus.CONFLICT);
}


@Override
public ValidationError getDuplicateTraitsByNameInFileMsg(List<Integer> matchingRows) {
matchingRows = matchingRows.stream().map(rowIndex -> getRowNumber(rowIndex)).collect(Collectors.toList());
Expand All @@ -172,12 +167,4 @@ public ValidationError getDuplicateTraitsByNameInFileMsg(List<Integer> matchingR
HttpStatus.CONFLICT);
}

@Override
public ValidationError getDuplicateTraitsByAbbreviationInFileMsg(List<Integer> matchingRows) {
// Make rows match excel rows
matchingRows = matchingRows.stream().map(rowIndex -> getRowNumber(rowIndex)).collect(Collectors.toList());
return new ValidationError("Trait abbreviations",
"One or more abbreviations is a duplicate of abbreviations. Set of traits with these matching abbreviations found in rows " + matchingRows.toString(),
HttpStatus.CONFLICT);
}
}
11 changes: 0 additions & 11 deletions src/main/java/org/breedinginsight/services/validators/TraitValidatorError.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,10 +159,6 @@ public ValidationError getDuplicateTraitByNamesMsg() {
return new ValidationError("traitName", "Trait name already exists", HttpStatus.CONFLICT);
}

@Override
public ValidationError getDuplicateTraitByAbbreviationsMsg() {
return new ValidationError("abbreviations", "Trait abbreviation already exists", HttpStatus.CONFLICT);
}

@Override
public ValidationError getDuplicateTraitsByNameInFileMsg(List<Integer> matchingRows) {
Expand All @@ -172,11 +168,4 @@ public ValidationError getDuplicateTraitsByNameInFileMsg(List<Integer> matchingR
HttpStatus.CONFLICT);
}

@Override
public ValidationError getDuplicateTraitsByAbbreviationInFileMsg(List<Integer> matchingRows) {
matchingRows = matchingRows.stream().map(rowIndex -> getRowNumber(rowIndex)).collect(Collectors.toList());
return new ValidationError("abbreviations",
"One or more abbreviations is a duplicate of abbreviations. Set of traits with these matching abbreviations found in rows " + matchingRows.toString(),
HttpStatus.CONFLICT);
}
}
2 changes: 0 additions & 2 deletions src/main/java/org/breedinginsight/services/validators/TraitValidatorErrorInterface.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,5 @@ public interface TraitValidatorErrorInterface {
ValidationError getCharLimitTraitAttributeMsg();
ValidationError getCharLimitMethodDescriptionMsg();
ValidationError getDuplicateTraitByNamesMsg();
ValidationError getDuplicateTraitByAbbreviationsMsg();
ValidationError getDuplicateTraitsByNameInFileMsg(List<Integer> matchingRows);
ValidationError getDuplicateTraitsByAbbreviationInFileMsg(List<Integer> matchingRows);
}
Loading