OLMIS-1162 More changes based on previous review's comments

Activity

FEOLMIS-462 12

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 32m 5 Sebastian Brudziński True. I wasn't planning on updating ...
    Reviewer - 61% reviewed 13m 2 Chongsun Ahn Do you plan to update IntegerResultDto as we...
    Reviewer - Complete 44m 5 Looks good, thanks.
    Total   1h 28m 12  
    #permalink

    Objectives

    RoleAssignment.assignTo(user) is not necessary anymore, but User transient properties still need to be loaded.
    Removed homeFacility from SupervisionRoleAssignment and hasRight method has been fixed to not use it anymore.
    The hasRight endpoint should return a JSON object on 200 OK.

    Branches in review

    #permalink

    Issues Raised From Comments

    Key Summary State Assignee
    #permalink

    General Comments

    Sebastian Brudziński

    Looks OK

    Looks OK

    Chongsun Ahn

    I have updated the review with changes of BooleanResultDto to ResultDto. Unfo...

    I have updated the review with changes of BooleanResultDto to ResultDto. Unfortunately it included a bunch of other unrelated commits. Just look at ResultDto, UserController, UserControllerTest and UserControllerIntegrationTest. I think in the future, I'm just going to create new reviews to avoid these kinds of problems.

    Sebastian Brudziński

    Chongsun Ahn Do you plan to update IntegerResultDto as well? If not, do we ha...

    Chongsun Ahn Do you plan to update IntegerResultDto as well? If not, do we have a ticket for this? I think we should do it asap so people do not follow two different approaches on returning primitives.

    Chongsun Ahn

    Sebastian Brudziński True. I wasn't planning on updating IntegerResultDto; I ...

    Sebastian Brudziński True. I wasn't planning on updating IntegerResultDto; I was going to let whoever implemented it in the first place to refactor it.

    Josh Zamor

    Looks good, thanks.

    Looks good, thanks.

    /STYLE-GUIDE.md Changed
    Open in IDE #permalink
    Repository Service_Template does not exist
    /src/.../web/UserControllerIntegrationTest.java Changed
    /src/main/.../domain/DirectRoleAssignment.java Changed
    /src/.../domain/FulfillmentRoleAssignment.java Changed
    /src/main/.../domain/RoleAssignment.java Changed
    /src/.../domain/SupervisionRoleAssignment.java Changed 1
    /src/main/.../referencedata/domain/User.java Changed
    /src/main/.../web/BooleanResultDto.java Deleted 2
    Open in IDE #permalink
    /src/main/.../referencedata/web/ResultDto.java Added
    Open in IDE #permalink
    /src/main/.../web/UserController.java Changed
    /src/main/resources/api-definition.yaml Changed 4
    /src/.../domain/SupervisionRoleAssignmentTest.java Changed
    /src/test/.../domain/UserTest.java Changed
    /src/test/.../web/UserControllerTest.java Changed

    Review updated: Reload | Ignore | Collapse

    You cannot reload the review while writing a comment.

    Log time against