OLMIS-3667 First look on the expanded REST api pattern [early feedback]

Activity

FEOLMIS-2210 32

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 2h 10m 17 Thanks for reviewing, everyone. I'm closing this review, ...
    Reviewer - 75% reviewed 9m 1 I think it would be a good idea to handle a cast error he...
    Reviewer - Complete 36m 4 there is alays value when more bugs can be detected durin...
    Reviewer - Complete 48m 2 Quick question: what about catalogItem? should it be also...
    Reviewer - 38% reviewed 14m 4 It'd be useful to see that example of some fields expande...
    Reviewer - Complete 20m 2 It seems a bit strange for this to be a subclass for Base...
    Reviewer - Complete 17m 2 Oh, OK I see now, we're actually retrieving the ObjectRef...
    Total   4h 34m 32  
    #permalink

    Objectives

    This is not ready for a final review yet, but I wanted to get early feedback on the approach we are taking. If you see any issues or problems, please let me know. I'll also mark key places directly in the code.

    Branches in review

    #permalink

    Issues Raised From Comments

    Key Summary State Assignee
    #permalink

    General Comments

    Josh Zamor

    This is interesting and it's got me thinking. Since this is just early feedba...

    This is interesting and it's got me thinking. Since this is just early feedback mine is that the level of indirection seems high. Less indirection might be more verbose, but perhaps better for maintainability. Without diving into it that's my first impression. I'll give it some more thought tonight. Thanks for sending this over!

    Sebastian Brudziński

    Thanks Josh Zamor - I'm definitely looking forward to any additional feedback...

    Thanks Josh Zamor - I'm definitely looking forward to any additional feedback. My main goal was to make the solution as re-usable as possible, and it currently is. All that is required is the expanded DTO and a call to the expander. The downside is obviously a need to use reflections.

    Sebastian Brudziński

    Thanks for reviewing, everyone. I'm closing this review, since the final one ...

    Thanks for reviewing, everyone. I'm closing this review, since the final one has been created - FEOLMIS-2241; feel free to take a look there.

    /src/.../web/InventoryItemControllerIntegrationTest.java Changed
    Open in IDE #permalink
    /src/main/.../cce/domain/InventoryItem.java Changed
    /src/main/.../cce/dto/InventoryItemDto.java Changed
    /src/main/.../dto/UserObjectReferenceDto.java Changed 6
    /src/main/.../dto/UserObjectReferenceDto.java Changed
    Open in IDE #permalink
    /src/main/java/.../cce/i18n/MessageKeys.java Changed
    Open in IDE #permalink
    /src/.../service/ObjReferenceExpander.java Added 16
    Open in IDE #permalink
    /src/main/.../cce/service/UuidConverter.java Added 5
    Open in IDE #permalink
    /src/main/.../web/InventoryItemController.java Changed 2
    /src/main/.../web/InventoryItemController.java Changed
    Open in IDE #permalink
    /src/main/resources/messages_en.properties Changed
    Open in IDE #permalink
    /src/.../web/InventoryItemDtoBuilderTest.java Changed
    /src/.../cce/InventoryItemDataBuilder.java Changed

    Review updated: Reload | Ignore | Collapse

    You cannot reload the review while writing a comment.

    Log time against