EDIT

task #8414

Updated by Andreas Kohlbecker over 4 years ago

eu.etaxonomy.cdm.remote.controller.OccurrenceController.doGetOccurencesDTO(@PathVariable(value="uuid") UUID uuid, HttpServletRequest request, HttpServletResponse response) throws IOException 

 * rename to *FieldUnitDTO as it returns `FieldUnitDTO`s 

 * In `doGetFieldUnitDTO` maybe also in other methods, the publish flag is checked only for the derivative for which the uuid is passed to the method. Originals are not checked though! --> #8424 **FIXME** 

 * `FieldUnitDTOOccurrenceServiceImpl.findFieldUnitDTO(DerivateDTO derivedUnitDTO, Collection<FieldUnitDTO> fieldUnits, HashMap<UUID, DerivateDTO> alreadyCollectedSpecimen)` misses documentation regarding the optional params which are not being used in most cases (fieldUnits, alreadyCollectedSpecimen) 
     * I think this method should be transactional (readonly) 
     * My first impression is that this method should be split in two methods, one method to get the FieldUnitDTO for a given    DerivedUnit (instead of PreservedSpecimenDTO) and another method which does the more fancy stuff. 
     * The method only adds the derived unit which is passed as parameter, what about all other derived units? These seem to be missing! This **needs to be FIXED** 
         * This is because this FieldUnitDTO should only contain the subtree of the passed derivedUnitDTO, the whole list of derivates can be seen when clicking on the details button. Therefore the method should be renamed to findFieldUnitDtoSubTree or something like this. 


 * FieldUnitDTO: 
     * has the field    `taxonRelatedDerivedUnits`. What is the purpose of this field? Is this field meant to hold derived units associated to taxa via `IndividualsAssociation`s?  
         * This can be removed, this is a relict of a first approach to get only the derived units belonging to a taxon 
     * why is `FieldUnitDTO` providing a `newInstance` method? As far as I know the only reason for doing so    is a situation in which the newInstanceMethod returns difference types, which the new operator can not do. The actual implementation of the newInstance method differs from the constructor: 

 ~~~java 
     /** 
      * @param fieldUnit 
      */ 
     public FieldUnitDTO(FieldUnit fieldUnit) { 
         super(fieldUnit); 
     } 


     public static FieldUnitDTO newInstance(FieldUnit fieldUnit){ 
	     FieldUnitDTO fieldUnitDto = new FieldUnitDTO(fieldUnit); 
	     if (fieldUnit.getGatheringEvent() != null){ 
	         fieldUnitDto.gatheringEvent = GatheringEventDTO.newInstance(fieldUnit.getGatheringEvent()); 
	     } 
	     fieldUnitDto.setRecordBase(fieldUnit.getRecordBasis().getMessage()); 
	     fieldUnitDto.setListLabel(fieldUnit.getTitleCache()); 

	     return fieldUnitDto; 

	 } 
 ~~~  

 is this by purpose? To me this looks like an error.... 

    * maybe this can be included into the constructor. 

 `OccurrenceServiceImpl.assembleFieldUnitDTO(..)` is using the constructor and adds the gathering event to the dto which is done in the newInstance method.    --> **TODO**: needs review and harmonization** (I think the newInstance Method should be removed completely as it causes confusion) 

Back