186: EventRestController logic moved to EventService

[IgorMaksymov]

connect to #186

[vaa25]

@IgorMaksymov

1. MyBatisUserService.getUserByOtp(): broken intends
2. MyBatisEventService.redirectToEventSite(): missed Override annotation
3. ControllerHandlerTest.shouldNotAssignUserToEventIfAssigned(): has unused variable
4. To be continued...

[vaa25]

@IgorMaksymov

4. Why do you have redundant rules for uncalled service methods and don't verify method calls in controller tests?
5. The same is in: MyBatisUserServiceTest.shouldReturnUsersByEvent()
6. MyBatisEventServiceTest.shouldAssignToEvent(): You should mock getFutureEvent() instead getEvent()
7. MyBatisEventServiceTest.shouldUnassignUserFromEvent(): You should mock getFutureEvent() instead getEvent()
8. Replace actual with returned anywhere in tests

P.S. Good work overall

@AndriyBaibak 30min

[IgorMaksymov]

@vaa25 4: What rules? 6: no, its failing without mocking getEvent. FutureEvent just checks date. I can mock them both, but this is redundant 7: idea was, to enable user unassign after event passed, so its correct to just getEvent(). fixed that in EventService

[vaa25]

@IgorMaksymov

4. when(...).thenReturn(...)
5. yes, fails, because you shouldn't mock service itself at all. eventService is InjectMocks. eventDao is Mock.

[IgorMaksymov]

@vaa25 4: in sources its called Stubbing EventRestControllerTest refactored, review pls

[vaa25]

• PASSED @AndriyBaibak 10 min

[romach]

@IgorMaksymov you must merge assignAuthorizedUserToEvent() and assignUserToEvent() methods because assignUserToEvent isn't used anywhere else.

[IgorMaksymov]

@romach its private method of assignAuthorizedUserToEvent for more readability of code. @vaa25 do you agree, that this methods should be merged?

[romach]

@AndriyBaibak 30 minutes spent