OLMIS-5479: Refactor orderFileTemplate to csvFileTemplate to accomodate shipment...

Activity

FEOLMIS-3363 14

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 34m 8 Thank you Nikodem Graczewski and Łukasz Lewczyński for re...
    Reviewer - Complete 9m 3 A good idea when renaming and changing the file content i...
    Reviewer - Complete 4m 3 and what about contract tests and ref-distro?
    Reviewer - 0% reviewed      
    Reviewer - 0% reviewed      
    Reviewer - 0% reviewed      
    Total   47m 14  
    #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

    Łukasz Lewczyński

    It seems like there are some issues from sonar: http://sonar.openlmis.org/das...

    It seems like there are some issues from sonar: http://sonar.openlmis.org/dashboard?id=org.sonarqube%3Aopenlmis-fulfillment

      Elias marked as Resolved 11 Oct

    Elias

    Working on this.

    Working on this.

    Nikodem Graczewski

    A good idea when renaming and changing the file content is to split them into...

    A good idea when renaming and changing the file content is to split them into different commits and reviews (first file renames, then separately file changes). Trying to check what was changed is painful (I assume there were some changes as the line counts changed).

    Elias

    Point taken. I have done another round of refactoring to implement the feedba...

    Point taken. I have done another round of refactoring to implement the feedback I received on the sprint demo. Instead of adding those changes to this review, I have created a new review. Hope that makes it easier.
    The new review will have a few more additional tests so the commit could pass sonar analysis. The tests are not for new code. Those are for old code just written to satisfy sonar's 65% branch coverage quality gate. Hope it is okay to keep those in the same commit.

    Elias

    Thank you Nikodem Graczewski and Łukasz Lewczyński for reviewing this. I cre...

    Thank you Nikodem Graczewski and Łukasz Lewczyński for reviewing this.

    I created a new review for a commit that affects the same files. The new review is for another refactoring that I did to apply the feedback I received on the sprint demo. By making it a new review, my hope is that it will be easier to review. I have marked the unresolved issues to resolved because those were resolved in a subsequent commit that went into that next review. I did not want to add that commit to this review, as that would double the effort it takes to review the same work twice.

    /performance/tests/orderFileTemplate.yml Changed
    /src/.../web/BaseWebIntegrationTest.java Changed
    /src/.../web/CsvFileTemplateControllerIntegrationTest.java Added
    Open in IDE #permalink
    /src/.../web/OrderFileTemplateControllerIntegrationTest.java Deleted
    Open in IDE #permalink
    /src/main/.../domain/CsvFileColumn.java Added
    Open in IDE #permalink
    /src/main/.../domain/CsvFileTemplate.java Added
    Open in IDE #permalink
    /src/main/.../domain/CsvTemplateType.java Added
    Open in IDE #permalink
    /src/main/.../domain/OrderFileColumn.java Deleted
    Open in IDE #permalink
    /src/main/.../domain/OrderFileTemplate.java Deleted
    Open in IDE #permalink
    /src/main/.../i18n/MessageKeys.java Changed
    /src/.../repository/CsvFileColumnRepository.java Added
    Open in IDE #permalink
    /src/.../repository/CsvFileTemplateRepository.java Added
    Open in IDE #permalink
    /src/.../repository/OrderFileColumnRepository.java Deleted
    Open in IDE #permalink
    /src/.../repository/OrderFileTemplateRepository.java Deleted
    Open in IDE #permalink
    /src/.../service/CsvFileTemplateService.java Added 4
    Open in IDE #permalink
    /src/main/.../service/OrderCsvHelper.java Changed
    /src/main/.../service/OrderFileStorage.java Changed
    /src/.../service/OrderFileTemplateService.java Deleted
    Open in IDE #permalink
    /src/main/.../web/util/CsvFileColumnDto.java Added
    Open in IDE #permalink
    /src/main/.../web/util/CsvFileTemplateDto.java Added
    Open in IDE #permalink
    /src/main/.../web/util/OrderFileColumnDto.java Deleted
    Open in IDE #permalink
    /src/main/.../util/OrderFileTemplateDto.java Deleted
    Open in IDE #permalink
    /src/.../validator/CsvFileTemplateValidator.java Added
    Open in IDE #permalink
    /src/.../validator/OrderFileTemplateValidator.java Deleted
    Open in IDE #permalink
    /src/.../web/CsvFileTemplateController.java Added
    Open in IDE #permalink
    /src/main/.../web/OrderController.java Changed
    /src/.../web/OrderFileTemplateController.java Deleted
    Open in IDE #permalink
    /src/.../web/OrderNumberConfigurationController.java Changed
    /src/.../migration/20181005182750366__change_order_file_configs_to_csv_file_config.sql Added
    Open in IDE #permalink
    /src/main/.../schemas/csvFileColumnDto.json Added
    Open in IDE #permalink
    /src/main/.../schemas/csvFileTemplateDto.json Added
    Open in IDE #permalink
    /src/main/.../schemas/orderFileColumnDto.json Deleted
    Open in IDE #permalink
    /src/.../schemas/orderFileTemplateDto.json Deleted
    Open in IDE #permalink
    /src/main/resources/api-definition.yaml Changed
    /src/main/resources/messages_en.properties Changed
    /src/test/.../service/OrderCsvHelperTest.java Changed
    /src/test/.../service/OrderStorageTest.java Changed
    /CHANGELOG.md Changed
    /gradle.properties Changed 5
    Open in IDE #permalink

    Review updated: Reload | Ignore | Collapse

    You cannot reload the review while writing a comment.

    Log time against