OLMIS-5415: Added extra data field to geo zone

Activity

FEOLMIS-3342 19

Keyboard shortcuts  
  • Summarize the review outcomes (optional)
     
    #permalink

    Details

    Warning: no files are visible, they have all been filtered.
    Participant Role Time Spent Comments Latest Comment
    Author 44m 10 I resolved this comment because changes are in another re...
    Reviewer - 62% reviewed 19m 4 +1
    Reviewer - Complete 22m 1 This extraData field is continuing to get used but the lo...
    Reviewer - Complete 22m 3 then we use builders for regular classes and then exporti...
    Reviewer - Complete 15m 1 I'm not sure this pattern (hasSameFields) is more readabl...
    Total   2h 3m 19  
    #permalink

    Objectives

    There are no specific objectives for this review.

    Branches in review

    #permalink

    Issues Raised From Comments

    Key Summary State Assignee
    #permalink

    General Comments

    Wes Brown

    This extraData field is continuing to get used but the logic (limited as it i...

    This extraData field is continuing to get used but the logic (limited as it is) is being copied by each domain/dto object. I think that a BaseEntity and BaseDto subclass should be created with the extraData field and associated logic (import, export, etc) that domain/dto objects could inherit from instead of BaseEntity/BaseDto.

    Łukasz Lewczyński

    done in https://review.openlmis.org/cru/FEOLMIS-3349

    done in https://review.openlmis.org/cru/FEOLMIS-3349

    Łukasz Lewczyński

    I resolved this comment because changes are in another review

    I resolved this comment because changes are in another review

    /src/.../repository/ProgramRepositoryIntegrationTest.java Changed
    Open in IDE #permalink
    /src/.../repository/RightAssignmentRepositoryIntegrationTest.java Changed
    Open in IDE #permalink
    /src/.../web/BaseWebIntegrationTest.java Changed 2
    Open in IDE #permalink
    /src/.../web/FacilityControllerIntegrationTest.java Changed
    Open in IDE #permalink
    /src/.../web/GeographicZoneControllerIntegrationTest.java Changed 2
    Open in IDE #permalink
    /src/.../web/IdealStockAmountControllerIntegrationTest.java Changed
    Open in IDE #permalink
    /src/.../web/RequisitionGroupControllerIntegrationTest.java Changed
    Open in IDE #permalink
    /src/.../web/SupervisoryNodeControllerIntegrationTest.java Changed
    Open in IDE #permalink
    /src/main/.../domain/GeographicZone.java Changed 6
    /src/main/.../referencedata/dto/BaseDto.java Changed
    Open in IDE #permalink
    /src/main/.../dto/GeographicZoneDto.java Changed
    Open in IDE #permalink
    /src/main/.../dto/GeographicZoneSimpleDto.java Changed
    /src/.../web/GeographicZoneController.java Changed
    /src/.../migration/20181001094512748__add_extradata_to_geo_zone.sql Added
    Open in IDE #permalink
    /src/main/.../schemas/geographicZone.json Changed
    Open in IDE #permalink
    /src/test/.../domain/GeographicZoneTest.java Added
    Open in IDE #permalink
    /src/test/.../dto/BaseDtoTest.java Added
    Open in IDE #permalink
    /src/test/.../dto/BasicFacilityDtoTest.java Changed 5
    Open in IDE #permalink
    /src/test/.../dto/FacilityDtoTest.java Changed
    Open in IDE #permalink
    /src/test/.../dto/GeographicZoneDtoTest.java Added
    Open in IDE #permalink
    /src/.../dto/GeographicZoneSimpleDtoTest.java Added
    Open in IDE #permalink
    /src/.../testbuilder/GeographicLevelDataBuilder.java Changed
    Open in IDE #permalink
    /src/.../testbuilder/GeographicZoneDataBuilder.java Changed
    /CHANGELOG.md Changed

    Review updated: Reload | Ignore | Collapse

    You cannot reload the review while writing a comment.

    Log time against