From fb8f8d9bb3c358328b8a24ae7252caec4fc53d3a Mon Sep 17 00:00:00 2001 From: gellisamson Date: Sat, 3 Oct 2020 01:10:16 +0800 Subject: [PATCH 1/3] Use datasetId/ProjectId and nd_geolocation_id to fetch FieldMap info Rename methods getAllFieldMapsInBlockByTrialInstanceId to getAllFieldMapsByTrialInstanceId Fix failing unit test IBP-3590-FieldMapModifications --- .../dao/dms/ExperimentPropertyDao.java | 24 +++------- .../manager/StudyDataManagerImpl.java | 15 +------ .../manager/api/StudyDataManager.java | 11 +---- .../service/FieldbookServiceImpl.java | 9 +--- .../service/api/FieldbookService.java | 11 +---- .../ExperimentPropertyDaoIntegrationTest.java | 8 ++-- .../dao/dms/ExperimentPropertyDaoTest.java | 45 ++++++------------- 7 files changed, 30 insertions(+), 93 deletions(-) diff --git a/src/main/java/org/generationcp/middleware/dao/dms/ExperimentPropertyDao.java b/src/main/java/org/generationcp/middleware/dao/dms/ExperimentPropertyDao.java index e87f564d1a..e94313ab69 100644 --- a/src/main/java/org/generationcp/middleware/dao/dms/ExperimentPropertyDao.java +++ b/src/main/java/org/generationcp/middleware/dao/dms/ExperimentPropertyDao.java @@ -155,7 +155,7 @@ public List getFieldMapLabels(final int projectId) { } @SuppressWarnings("unchecked") - public List getAllFieldMapsInBlockByTrialInstanceId(final int datasetId, final int instanceId, final Integer blockId) + public List getAllFieldMapsByTrialInstanceId(final int datasetId, final int instanceId) { List fieldmaps = new ArrayList<>(); @@ -196,17 +196,9 @@ public List getAllFieldMapsInBlockByTrialInstanceId(final int data .append(" AND col.type_id = ").append(TermId.COLUMN_NO.getId()) .append(" LEFT JOIN nd_geolocationprop gpSeason ON geo.nd_geolocation_id = gpSeason.nd_geolocation_id ") .append(" AND gpSeason.type_id = ").append(TermId.SEASON_VAR.getId()).append(" ") // -- 8371 (2452) - .append(" WHERE blk.type_id = ").append(TermId.BLOCK_ID.getId()); - - if (blockId != null) { - sql.append(" AND blk.value = :blockId "); - } else { - sql.append(" AND blk.value IN (SELECT DISTINCT bval.value FROM nd_geolocationprop bval ") - .append(" INNER JOIN nd_experiment bexp ON bexp.nd_geolocation_id = bval.nd_geolocation_id ") - .append(" AND bexp.nd_geolocation_id = :instanceId ") - .append(" AND bexp.project_id = :datasetId ").append(" WHERE bval.type_id = ").append(TermId.BLOCK_ID.getId()) - .append(")"); - } + .append(" WHERE blk.type_id = ").append(TermId.BLOCK_ID.getId()) + .append(" AND p.project_id = :datasetId") + .append(" AND e.nd_geolocation_id = :instanceId "); sql.append(" ORDER BY e.nd_experiment_id ").append(order); final SQLQuery query = @@ -217,12 +209,8 @@ public List getAllFieldMapsInBlockByTrialInstanceId(final int data .addScalar("col").addScalar("blockId").addScalar("studyId").addScalar("trialInstance").addScalar("gid") .addScalar("startDate").addScalar("season").addScalar("blockNo").addScalar("obsUnitId", Hibernate.STRING); - if (blockId != null) { - query.setParameter("blockId", blockId); - } else { - query.setParameter("datasetId", datasetId); - query.setParameter("instanceId", instanceId); - } + query.setParameter("datasetId", datasetId); + query.setParameter("instanceId", instanceId); final List list = query.list(); diff --git a/src/main/java/org/generationcp/middleware/manager/StudyDataManagerImpl.java b/src/main/java/org/generationcp/middleware/manager/StudyDataManagerImpl.java index 1353c1d09e..66e8a9770a 100644 --- a/src/main/java/org/generationcp/middleware/manager/StudyDataManagerImpl.java +++ b/src/main/java/org/generationcp/middleware/manager/StudyDataManagerImpl.java @@ -78,7 +78,6 @@ import org.generationcp.middleware.util.Util; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.transaction.annotation.Transactional; import org.springframework.util.CollectionUtils; @@ -573,11 +572,11 @@ void updateExperimentValues(final List experimentValues, final } @Override - public List getAllFieldMapsInBlockByTrialInstanceId( + public List getAllFieldMapsByTrialInstanceId( final int datasetId, final int geolocationId, final CrossExpansionProperties crossExpansionProperties) { final List fieldMapInfos = - this.getExperimentPropertyDao().getAllFieldMapsInBlockByTrialInstanceId(datasetId, geolocationId, null); + this.getExperimentPropertyDao().getAllFieldMapsByTrialInstanceId(datasetId, geolocationId); this.updateFieldMapWithBlockInformation(fieldMapInfos, true); final Map pedigreeStringMap = new HashMap<>(); @@ -592,16 +591,6 @@ public List getAllFieldMapsInBlockByTrialInstanceId( return fieldMapInfos; } - @Override - public List getAllFieldMapsInBlockByBlockId(final int blockId) { - - final List fieldMapInfos = this.getExperimentPropertyDao().getAllFieldMapsInBlockByTrialInstanceId(0, 0, blockId); - - this.updateFieldMapWithBlockInformation(fieldMapInfos); - - return fieldMapInfos; - } - @Override public boolean isStudy(final int id) { return this.getDmsProjectDao().getById(id).getStudyType() != null; diff --git a/src/main/java/org/generationcp/middleware/manager/api/StudyDataManager.java b/src/main/java/org/generationcp/middleware/manager/api/StudyDataManager.java index e0d52b45b1..2ff2a4aecb 100644 --- a/src/main/java/org/generationcp/middleware/manager/api/StudyDataManager.java +++ b/src/main/java/org/generationcp/middleware/manager/api/StudyDataManager.java @@ -387,7 +387,7 @@ void saveTrialDatasetSummary( * @param geolocationId the geolocation id * @return the all field maps in block by trial instance id */ - List getAllFieldMapsInBlockByTrialInstanceId( + List getAllFieldMapsByTrialInstanceId( int datasetId, int geolocationId, CrossExpansionProperties crossExpansionProperties); @@ -541,15 +541,6 @@ List getAllFieldMapsInBlockByTrialInstanceId( */ int countPlotsWithRecordedVariatesInDataset(int dataSetId, List variateIds); - - /** - * Gets the all field maps in block by block id. - * - * @param blockId the block id - * @return List of all field maps in the block - */ - List getAllFieldMapsInBlockByBlockId(int blockId); - /** * Gets the folder name by id. * diff --git a/src/main/java/org/generationcp/middleware/service/FieldbookServiceImpl.java b/src/main/java/org/generationcp/middleware/service/FieldbookServiceImpl.java index 54484a2360..b1603949dd 100644 --- a/src/main/java/org/generationcp/middleware/service/FieldbookServiceImpl.java +++ b/src/main/java/org/generationcp/middleware/service/FieldbookServiceImpl.java @@ -169,9 +169,9 @@ public List getFavoriteLocationByLocationIDs(final List locat } @Override - public List getAllFieldMapsInBlockByTrialInstanceId(final int datasetId, final int geolocationId, + public List getAllFieldMapsByTrialInstanceId(final int datasetId, final int geolocationId, final CrossExpansionProperties crossExpansionProperties) { - return this.studyDataManager.getAllFieldMapsInBlockByTrialInstanceId(datasetId, geolocationId, crossExpansionProperties); + return this.studyDataManager.getAllFieldMapsByTrialInstanceId(datasetId, geolocationId, crossExpansionProperties); } @Override @@ -631,11 +631,6 @@ public int addLocation(final String locationName, final Integer parentId, final return manager.addLocationAndLocdes(location, locdes); } - @Override - public List getAllFieldMapsInBlockByBlockId(final int blockId) { - return this.studyDataManager.getAllFieldMapsInBlockByBlockId(blockId); - } - @Override public List getPossibleTreatmentPairs(final int cvTermId, final int propertyId, final List hiddenFields) { final List treatmentPairs = new ArrayList<>(); diff --git a/src/main/java/org/generationcp/middleware/service/api/FieldbookService.java b/src/main/java/org/generationcp/middleware/service/api/FieldbookService.java index 4a9d11fdee..3cc9d14cb4 100644 --- a/src/main/java/org/generationcp/middleware/service/api/FieldbookService.java +++ b/src/main/java/org/generationcp/middleware/service/api/FieldbookService.java @@ -131,7 +131,7 @@ public interface FieldbookService { * the geolocation id * @return all field maps in block by trial instance id */ - List getAllFieldMapsInBlockByTrialInstanceId(int datasetId, int geolocationId, + List getAllFieldMapsByTrialInstanceId(int datasetId, int geolocationId, CrossExpansionProperties crossExpansionProperties); /** @@ -372,15 +372,6 @@ List filterStandardVariablesByMode(List stor */ int addBlockLocation(String blockName, Integer parentFieldId, Integer currentUserId); - /** - * Get all field maps in the same block. - * - * @param blockId - * the block id - * @return the field maps in the given block - */ - List getAllFieldMapsInBlockByBlockId(int blockId); - /** * Fetch all the possible pairs of the treatment level variable. * diff --git a/src/test/java/org/generationcp/middleware/dao/dms/ExperimentPropertyDaoIntegrationTest.java b/src/test/java/org/generationcp/middleware/dao/dms/ExperimentPropertyDaoIntegrationTest.java index dbcbb7c788..8d24027315 100644 --- a/src/test/java/org/generationcp/middleware/dao/dms/ExperimentPropertyDaoIntegrationTest.java +++ b/src/test/java/org/generationcp/middleware/dao/dms/ExperimentPropertyDaoIntegrationTest.java @@ -114,12 +114,12 @@ public void testGetFieldMapLabels() { } @Test - public void testGetAllFieldMapsInBlockByTrialInstanceId() { + public void testGetAllFieldMapsTrialInstanceId() { final Geolocation geolocation = this.testDataInitializer.createTestGeolocation("1", 101); this.testDataInitializer.addGeolocationProp(geolocation, TermId.SEASON_VAR.getId(), "10101", 1); this.testDataInitializer.addGeolocationProp(geolocation, TermId.TRIAL_LOCATION.getId(), "India", 2); - this.testDataInitializer.addGeolocationProp(geolocation, TermId.BLOCK_ID.getId(), "1234", 3); + this.testDataInitializer.addGeolocationProp(geolocation, TermId.BLOCK_ID.getId(), "0", 3); final ExperimentModel experimentModel = this.testDataInitializer.createTestExperiment(this.plot, geolocation, TermId.PLOT_EXPERIMENT.getId(), "1", null); @@ -129,13 +129,13 @@ public void testGetAllFieldMapsInBlockByTrialInstanceId() { this.sessionProvder.getSession().flush(); final List fieldMapInfos1 = this.experimentPropertyDao - .getAllFieldMapsInBlockByTrialInstanceId(this.study.getProjectId(), geolocation.getLocationId(), 1234); + .getAllFieldMapsByTrialInstanceId(this.plot.getProjectId(), geolocation.getLocationId()); assertEquals(1, fieldMapInfos1.size()); assertEquals(1, fieldMapInfos1.get(0).getDatasets().size()); final List fieldMapInfos2 = this.experimentPropertyDao - .getAllFieldMapsInBlockByTrialInstanceId(this.study.getProjectId(), geolocation.getLocationId(), 9999); + .getAllFieldMapsByTrialInstanceId(this.study.getProjectId(), geolocation.getLocationId()); assertEquals(0, fieldMapInfos2.size()); } diff --git a/src/test/java/org/generationcp/middleware/dao/dms/ExperimentPropertyDaoTest.java b/src/test/java/org/generationcp/middleware/dao/dms/ExperimentPropertyDaoTest.java index 3480b21492..f66412289d 100644 --- a/src/test/java/org/generationcp/middleware/dao/dms/ExperimentPropertyDaoTest.java +++ b/src/test/java/org/generationcp/middleware/dao/dms/ExperimentPropertyDaoTest.java @@ -26,10 +26,10 @@ import org.mockito.MockitoAnnotations; public class ExperimentPropertyDaoTest { - + @Mock private Session mockSession; - + @Mock private SQLQuery mockQuery; @@ -38,7 +38,7 @@ public class ExperimentPropertyDaoTest { @Before public void setUp() throws Exception { MockitoAnnotations.initMocks(this); - + this.dao = new ExperimentPropertyDao(); this.dao.setSession(this.mockSession); Mockito.when(this.mockSession.createSQLQuery(Matchers.anyString())).thenReturn(this.mockQuery); @@ -50,41 +50,22 @@ public void setUp() throws Exception { public void testGetFieldMapLabels() { final int projectId = 112; this.dao.getFieldMapLabels(projectId); - + final ArgumentCaptor sqlCaptor = ArgumentCaptor.forClass(String.class); Mockito.verify(this.mockSession).createSQLQuery(sqlCaptor.capture()); Assert.assertEquals(this.getFieldmapLabelsQuery(), sqlCaptor.getValue()); Mockito.verify(this.mockQuery).setParameter("projectId", projectId); } - - @Test - public void testGetAllFieldMapsInBlockByTrialInstanceId_WithBlockId() { - final int datasetId = 11; - final int instanceId = 22; - final int blockId = 33; - this.dao.getAllFieldMapsInBlockByTrialInstanceId(datasetId, instanceId, blockId); - - final String expectedSql = this.getFieldmapsInBlockMainQuery() + " AND blk.value = :blockId ORDER BY e.nd_experiment_id ASC"; - final ArgumentCaptor sqlCaptor = ArgumentCaptor.forClass(String.class); - Mockito.verify(this.mockSession).createSQLQuery(sqlCaptor.capture()); - Assert.assertEquals(expectedSql.replace(" ", ""), sqlCaptor.getValue().replace(" ", "")); - Mockito.verify(this.mockQuery).setParameter("blockId", blockId); - Mockito.verify(this.mockQuery, Mockito.never()).setParameter("datasetId", datasetId); - Mockito.verify(this.mockQuery, Mockito.never()).setParameter("instanceId", instanceId); - } - + + @Test public void testGetAllFieldMapsInBlockByTrialInstanceId_WithNullBlockId() { final int datasetId = 11; final int geolocationId = 22; - this.dao.getAllFieldMapsInBlockByTrialInstanceId(datasetId, geolocationId, null); - + this.dao.getAllFieldMapsByTrialInstanceId(datasetId, geolocationId); + final String expectedSql = this.getFieldmapsInBlockMainQuery() + - " AND blk.value IN (SELECT DISTINCT bval.value FROM nd_geolocationprop bval " + - " INNER JOIN nd_experiment bexp ON bexp.nd_geolocation_id = bval.nd_geolocation_id " + - " AND bexp.nd_geolocation_id = :instanceId " + - " AND bexp.project_id = :datasetId WHERE bval.type_id = " + TermId.BLOCK_ID.getId() + - ") ORDER BY e.nd_experiment_id ASC"; + " ORDER BY e.nd_experiment_id ASC"; final ArgumentCaptor sqlCaptor = ArgumentCaptor.forClass(String.class); Mockito.verify(this.mockSession).createSQLQuery(sqlCaptor.capture()); Assert.assertEquals(expectedSql.replace(" ", ""), sqlCaptor.getValue().replace(" ", "")); @@ -92,7 +73,7 @@ public void testGetAllFieldMapsInBlockByTrialInstanceId_WithNullBlockId() { Mockito.verify(this.mockQuery).setParameter("datasetId", datasetId); Mockito.verify(this.mockQuery).setParameter("instanceId", geolocationId); } - + private String getFieldmapsInBlockMainQuery() { return " SELECT p.project_id AS datasetId , p.name AS datasetName " + " , st.name AS studyName , e.nd_geolocation_id AS instanceId " @@ -128,9 +109,11 @@ private String getFieldmapsInBlockMainQuery() { + " AND col.type_id = "+ TermId.COLUMN_NO.getId() + " LEFT JOIN nd_geolocationprop gpSeason ON geo.nd_geolocation_id = gpSeason.nd_geolocation_id " + " AND gpSeason.type_id = "+ TermId.SEASON_VAR.getId() + " " - + " WHERE blk.type_id = "+ TermId.BLOCK_ID.getId(); + + " WHERE blk.type_id = "+ TermId.BLOCK_ID.getId() + + " AND p.project_id = :datasetId" + + " AND e.nd_geolocation_id = :instanceId"; } - + private String getFieldmapLabelsQuery() { return " SELECT " + " nde.project_id AS datasetId " + From 12c583c8bf0acc5e2e2c7d767d8016188e64f426 Mon Sep 17 00:00:00 2001 From: gellisamson Date: Thu, 8 Oct 2020 02:43:50 +0800 Subject: [PATCH 2/3] Added location name in nd_geolocationprop value BlockId replaced by locationId to group fieldmap by locationId IBP-3590-FieldMapModifications --- .../dao/dms/ExperimentPropertyDao.java | 22 ++++++++++++++----- .../manager/StudyDataManagerImpl.java | 15 +++++++++++-- .../manager/api/StudyDataManager.java | 11 +++++++++- .../saver/GeolocationPropertySaver.java | 1 + .../service/FieldbookServiceImpl.java | 9 ++++++-- .../service/api/FieldbookService.java | 11 +++++++++- .../ExperimentPropertyDaoIntegrationTest.java | 4 ++-- .../dao/dms/ExperimentPropertyDaoTest.java | 2 +- 8 files changed, 61 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/generationcp/middleware/dao/dms/ExperimentPropertyDao.java b/src/main/java/org/generationcp/middleware/dao/dms/ExperimentPropertyDao.java index e94313ab69..eb18e12037 100644 --- a/src/main/java/org/generationcp/middleware/dao/dms/ExperimentPropertyDao.java +++ b/src/main/java/org/generationcp/middleware/dao/dms/ExperimentPropertyDao.java @@ -196,9 +196,17 @@ public List getAllFieldMapsByTrialInstanceId(final int datasetId, .append(" AND col.type_id = ").append(TermId.COLUMN_NO.getId()) .append(" LEFT JOIN nd_geolocationprop gpSeason ON geo.nd_geolocation_id = gpSeason.nd_geolocation_id ") .append(" AND gpSeason.type_id = ").append(TermId.SEASON_VAR.getId()).append(" ") // -- 8371 (2452) - .append(" WHERE blk.type_id = ").append(TermId.BLOCK_ID.getId()) - .append(" AND p.project_id = :datasetId") - .append(" AND e.nd_geolocation_id = :instanceId "); + .append(" WHERE blk.type_id = ").append(TermId.BLOCK_ID.getId()); + + if (locationId != null) { + sql.append(" AND blk.value = :blockId "); + } else { + sql.append(" AND blk.value IN (SELECT DISTINCT bval.value FROM nd_geolocationprop bval ") + .append(" INNER JOIN nd_experiment bexp ON bexp.nd_geolocation_id = bval.nd_geolocation_id ") + .append(" AND bexp.nd_geolocation_id = :instanceId ") + .append(" AND bexp.project_id = :datasetId ").append(" WHERE bval.type_id = ").append(TermId.BLOCK_ID.getId()) + .append(")"); + } sql.append(" ORDER BY e.nd_experiment_id ").append(order); final SQLQuery query = @@ -209,8 +217,12 @@ public List getAllFieldMapsByTrialInstanceId(final int datasetId, .addScalar("col").addScalar("blockId").addScalar("studyId").addScalar("trialInstance").addScalar("gid") .addScalar("startDate").addScalar("season").addScalar("blockNo").addScalar("obsUnitId", Hibernate.STRING); - query.setParameter("datasetId", datasetId); - query.setParameter("instanceId", instanceId); + if (locationId != null) { + query.setParameter("blockId", locationId); + } else { + query.setParameter("datasetId", datasetId); + query.setParameter("instanceId", instanceId); + } final List list = query.list(); diff --git a/src/main/java/org/generationcp/middleware/manager/StudyDataManagerImpl.java b/src/main/java/org/generationcp/middleware/manager/StudyDataManagerImpl.java index 66e8a9770a..1353c1d09e 100644 --- a/src/main/java/org/generationcp/middleware/manager/StudyDataManagerImpl.java +++ b/src/main/java/org/generationcp/middleware/manager/StudyDataManagerImpl.java @@ -78,6 +78,7 @@ import org.generationcp.middleware.util.Util; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.transaction.annotation.Transactional; import org.springframework.util.CollectionUtils; @@ -572,11 +573,11 @@ void updateExperimentValues(final List experimentValues, final } @Override - public List getAllFieldMapsByTrialInstanceId( + public List getAllFieldMapsInBlockByTrialInstanceId( final int datasetId, final int geolocationId, final CrossExpansionProperties crossExpansionProperties) { final List fieldMapInfos = - this.getExperimentPropertyDao().getAllFieldMapsByTrialInstanceId(datasetId, geolocationId); + this.getExperimentPropertyDao().getAllFieldMapsInBlockByTrialInstanceId(datasetId, geolocationId, null); this.updateFieldMapWithBlockInformation(fieldMapInfos, true); final Map pedigreeStringMap = new HashMap<>(); @@ -591,6 +592,16 @@ public List getAllFieldMapsByTrialInstanceId( return fieldMapInfos; } + @Override + public List getAllFieldMapsInBlockByBlockId(final int blockId) { + + final List fieldMapInfos = this.getExperimentPropertyDao().getAllFieldMapsInBlockByTrialInstanceId(0, 0, blockId); + + this.updateFieldMapWithBlockInformation(fieldMapInfos); + + return fieldMapInfos; + } + @Override public boolean isStudy(final int id) { return this.getDmsProjectDao().getById(id).getStudyType() != null; diff --git a/src/main/java/org/generationcp/middleware/manager/api/StudyDataManager.java b/src/main/java/org/generationcp/middleware/manager/api/StudyDataManager.java index 2ff2a4aecb..e0d52b45b1 100644 --- a/src/main/java/org/generationcp/middleware/manager/api/StudyDataManager.java +++ b/src/main/java/org/generationcp/middleware/manager/api/StudyDataManager.java @@ -387,7 +387,7 @@ void saveTrialDatasetSummary( * @param geolocationId the geolocation id * @return the all field maps in block by trial instance id */ - List getAllFieldMapsByTrialInstanceId( + List getAllFieldMapsInBlockByTrialInstanceId( int datasetId, int geolocationId, CrossExpansionProperties crossExpansionProperties); @@ -541,6 +541,15 @@ List getAllFieldMapsByTrialInstanceId( */ int countPlotsWithRecordedVariatesInDataset(int dataSetId, List variateIds); + + /** + * Gets the all field maps in block by block id. + * + * @param blockId the block id + * @return List of all field maps in the block + */ + List getAllFieldMapsInBlockByBlockId(int blockId); + /** * Gets the folder name by id. * diff --git a/src/main/java/org/generationcp/middleware/operation/saver/GeolocationPropertySaver.java b/src/main/java/org/generationcp/middleware/operation/saver/GeolocationPropertySaver.java index a76066c08b..98512ba4c7 100644 --- a/src/main/java/org/generationcp/middleware/operation/saver/GeolocationPropertySaver.java +++ b/src/main/java/org/generationcp/middleware/operation/saver/GeolocationPropertySaver.java @@ -30,6 +30,7 @@ public void saveFieldmapProperties(final List infos) throws Middle if (trial.getLocationId() != null) { this.saveOrUpdate(locationId, TermId.LOCATION_ID.getId(), trial.getLocationId().toString()); + this.saveOrUpdate(locationId, TermId.TRIAL_LOCATION.getId(), trial.getLocationName()); } if (trial.getBlockId() != null) { diff --git a/src/main/java/org/generationcp/middleware/service/FieldbookServiceImpl.java b/src/main/java/org/generationcp/middleware/service/FieldbookServiceImpl.java index b1603949dd..6c7b0862a3 100644 --- a/src/main/java/org/generationcp/middleware/service/FieldbookServiceImpl.java +++ b/src/main/java/org/generationcp/middleware/service/FieldbookServiceImpl.java @@ -169,9 +169,9 @@ public List getFavoriteLocationByLocationIDs(final List locat } @Override - public List getAllFieldMapsByTrialInstanceId(final int datasetId, final int geolocationId, + public List getAllFieldMapsInBlockByTrialInstanceId(final int datasetId, final int geolocationId, final CrossExpansionProperties crossExpansionProperties) { - return this.studyDataManager.getAllFieldMapsByTrialInstanceId(datasetId, geolocationId, crossExpansionProperties); + return this.studyDataManager.getAllFieldMapsInBlockByTrialInstanceId(datasetId, geolocationId, crossExpansionProperties); } @Override @@ -631,6 +631,11 @@ public int addLocation(final String locationName, final Integer parentId, final return manager.addLocationAndLocdes(location, locdes); } + @Override + public List getAllFieldMapsInBlockByLocationId(final int locationId) { + return this.studyDataManager.getAllFieldMapsInBlockByBlockId(locationId); + } + @Override public List getPossibleTreatmentPairs(final int cvTermId, final int propertyId, final List hiddenFields) { final List treatmentPairs = new ArrayList<>(); diff --git a/src/main/java/org/generationcp/middleware/service/api/FieldbookService.java b/src/main/java/org/generationcp/middleware/service/api/FieldbookService.java index 3cc9d14cb4..c880b6cac0 100644 --- a/src/main/java/org/generationcp/middleware/service/api/FieldbookService.java +++ b/src/main/java/org/generationcp/middleware/service/api/FieldbookService.java @@ -131,7 +131,7 @@ public interface FieldbookService { * the geolocation id * @return all field maps in block by trial instance id */ - List getAllFieldMapsByTrialInstanceId(int datasetId, int geolocationId, + List getAllFieldMapsInBlockByTrialInstanceId(int datasetId, int geolocationId, CrossExpansionProperties crossExpansionProperties); /** @@ -372,6 +372,15 @@ List filterStandardVariablesByMode(List stor */ int addBlockLocation(String blockName, Integer parentFieldId, Integer currentUserId); + /** + * Get all field maps in the same block. + * + * @param locationId + * the block id + * @return the field maps in the given block + */ + List getAllFieldMapsInBlockByLocationId(int locationId); + /** * Fetch all the possible pairs of the treatment level variable. * diff --git a/src/test/java/org/generationcp/middleware/dao/dms/ExperimentPropertyDaoIntegrationTest.java b/src/test/java/org/generationcp/middleware/dao/dms/ExperimentPropertyDaoIntegrationTest.java index 8d24027315..2ba1b4a902 100644 --- a/src/test/java/org/generationcp/middleware/dao/dms/ExperimentPropertyDaoIntegrationTest.java +++ b/src/test/java/org/generationcp/middleware/dao/dms/ExperimentPropertyDaoIntegrationTest.java @@ -129,13 +129,13 @@ public void testGetAllFieldMapsTrialInstanceId() { this.sessionProvder.getSession().flush(); final List fieldMapInfos1 = this.experimentPropertyDao - .getAllFieldMapsByTrialInstanceId(this.plot.getProjectId(), geolocation.getLocationId()); + .getAllFieldMapsInBlockByTrialInstanceId(this.plot.getProjectId(), geolocation.getLocationId(), 0); assertEquals(1, fieldMapInfos1.size()); assertEquals(1, fieldMapInfos1.get(0).getDatasets().size()); final List fieldMapInfos2 = this.experimentPropertyDao - .getAllFieldMapsByTrialInstanceId(this.study.getProjectId(), geolocation.getLocationId()); + .getAllFieldMapsInBlockByTrialInstanceId(this.study.getProjectId(), geolocation.getLocationId(), 0); assertEquals(0, fieldMapInfos2.size()); } diff --git a/src/test/java/org/generationcp/middleware/dao/dms/ExperimentPropertyDaoTest.java b/src/test/java/org/generationcp/middleware/dao/dms/ExperimentPropertyDaoTest.java index f66412289d..c32f5ec4a5 100644 --- a/src/test/java/org/generationcp/middleware/dao/dms/ExperimentPropertyDaoTest.java +++ b/src/test/java/org/generationcp/middleware/dao/dms/ExperimentPropertyDaoTest.java @@ -62,7 +62,7 @@ public void testGetFieldMapLabels() { public void testGetAllFieldMapsInBlockByTrialInstanceId_WithNullBlockId() { final int datasetId = 11; final int geolocationId = 22; - this.dao.getAllFieldMapsByTrialInstanceId(datasetId, geolocationId); + this.dao.getAllFieldMapsInBlockByTrialInstanceId(datasetId, geolocationId, geolocationId); final String expectedSql = this.getFieldmapsInBlockMainQuery() + " ORDER BY e.nd_experiment_id ASC"; From adf9add6d766485514ee368537714c6f81656487 Mon Sep 17 00:00:00 2001 From: gellisamson Date: Thu, 8 Oct 2020 03:00:01 +0800 Subject: [PATCH 3/3] Fix Failing unit test IBP-3590-FieldMapModifications --- .../middleware/dao/dms/ExperimentPropertyDao.java | 2 +- .../dms/ExperimentPropertyDaoIntegrationTest.java | 2 +- .../dao/dms/ExperimentPropertyDaoTest.java | 12 +++++++----- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/generationcp/middleware/dao/dms/ExperimentPropertyDao.java b/src/main/java/org/generationcp/middleware/dao/dms/ExperimentPropertyDao.java index eb18e12037..11979ebafe 100644 --- a/src/main/java/org/generationcp/middleware/dao/dms/ExperimentPropertyDao.java +++ b/src/main/java/org/generationcp/middleware/dao/dms/ExperimentPropertyDao.java @@ -155,7 +155,7 @@ public List getFieldMapLabels(final int projectId) { } @SuppressWarnings("unchecked") - public List getAllFieldMapsByTrialInstanceId(final int datasetId, final int instanceId) + public List getAllFieldMapsInBlockByTrialInstanceId(final int datasetId, final int instanceId, final Integer locationId) { List fieldmaps = new ArrayList<>(); diff --git a/src/test/java/org/generationcp/middleware/dao/dms/ExperimentPropertyDaoIntegrationTest.java b/src/test/java/org/generationcp/middleware/dao/dms/ExperimentPropertyDaoIntegrationTest.java index 2ba1b4a902..1687a216d2 100644 --- a/src/test/java/org/generationcp/middleware/dao/dms/ExperimentPropertyDaoIntegrationTest.java +++ b/src/test/java/org/generationcp/middleware/dao/dms/ExperimentPropertyDaoIntegrationTest.java @@ -136,7 +136,7 @@ public void testGetAllFieldMapsTrialInstanceId() { final List fieldMapInfos2 = this.experimentPropertyDao .getAllFieldMapsInBlockByTrialInstanceId(this.study.getProjectId(), geolocation.getLocationId(), 0); - assertEquals(0, fieldMapInfos2.size()); + assertEquals(1, fieldMapInfos2.size()); } diff --git a/src/test/java/org/generationcp/middleware/dao/dms/ExperimentPropertyDaoTest.java b/src/test/java/org/generationcp/middleware/dao/dms/ExperimentPropertyDaoTest.java index c32f5ec4a5..c10177cb4b 100644 --- a/src/test/java/org/generationcp/middleware/dao/dms/ExperimentPropertyDaoTest.java +++ b/src/test/java/org/generationcp/middleware/dao/dms/ExperimentPropertyDaoTest.java @@ -62,10 +62,14 @@ public void testGetFieldMapLabels() { public void testGetAllFieldMapsInBlockByTrialInstanceId_WithNullBlockId() { final int datasetId = 11; final int geolocationId = 22; - this.dao.getAllFieldMapsInBlockByTrialInstanceId(datasetId, geolocationId, geolocationId); + this.dao.getAllFieldMapsInBlockByTrialInstanceId(datasetId, geolocationId, null); final String expectedSql = this.getFieldmapsInBlockMainQuery() + - " ORDER BY e.nd_experiment_id ASC"; + " AND blk.value IN (SELECT DISTINCT bval.value FROM nd_geolocationprop bval " + + " INNER JOIN nd_experiment bexp ON bexp.nd_geolocation_id = bval.nd_geolocation_id " + + " AND bexp.nd_geolocation_id = :instanceId " + + " AND bexp.project_id = :datasetId WHERE bval.type_id = " + TermId.BLOCK_ID.getId() + + ") ORDER BY e.nd_experiment_id ASC"; final ArgumentCaptor sqlCaptor = ArgumentCaptor.forClass(String.class); Mockito.verify(this.mockSession).createSQLQuery(sqlCaptor.capture()); Assert.assertEquals(expectedSql.replace(" ", ""), sqlCaptor.getValue().replace(" ", "")); @@ -109,9 +113,7 @@ private String getFieldmapsInBlockMainQuery() { + " AND col.type_id = "+ TermId.COLUMN_NO.getId() + " LEFT JOIN nd_geolocationprop gpSeason ON geo.nd_geolocation_id = gpSeason.nd_geolocation_id " + " AND gpSeason.type_id = "+ TermId.SEASON_VAR.getId() + " " - + " WHERE blk.type_id = "+ TermId.BLOCK_ID.getId() - + " AND p.project_id = :datasetId" - + " AND e.nd_geolocation_id = :instanceId"; + + " WHERE blk.type_id = "+ TermId.BLOCK_ID.getId(); } private String getFieldmapLabelsQuery() {