Skip to content

Commit

Permalink
#705 fix all unit tests
Browse files Browse the repository at this point in the history
+ remove isWritableInterface from user, since it's not needed anymore.
  • Loading branch information
janikEndtner committed Dec 28, 2023
1 parent a4cbd82 commit e687228
Show file tree
Hide file tree
Showing 27 changed files with 171 additions and 232 deletions.
Original file line number Diff line number Diff line change
@@ -1,33 +1,33 @@
package ch.puzzle.okr;
package ch.puzzle.okr.applicationlistener;

import ch.puzzle.okr.repository.UserRepository;
import ch.puzzle.okr.service.persistence.UserPersistenceService;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.boot.context.event.ApplicationReadyEvent;
import org.springframework.context.ApplicationListener;
import org.springframework.stereotype.Component;

@Component
public class OkrChampionListener implements ApplicationListener<ApplicationReadyEvent> {
public class OkrChampionApplicationListener implements ApplicationListener<ApplicationReadyEvent> {

private static final String DELIMITER = ",";

@Value("${okr.user.champion.emails}")
private String okrChampionEmails;

private final UserRepository userRepository;
private final UserPersistenceService userPersistenceService;

public OkrChampionListener(UserRepository userRepository) {
this.userRepository = userRepository;
public OkrChampionApplicationListener(UserPersistenceService userPersistenceService) {
this.userPersistenceService = userPersistenceService;
}

@Override
public void onApplicationEvent(ApplicationReadyEvent event) {
String[] championMails = okrChampionEmails.split(DELIMITER);
for (var mail : championMails) {
var user = userRepository.findByEmail(mail)
var user = userPersistenceService.findByEmail(mail.trim())
.orElseThrow(() -> new RuntimeException("User not found by champion e-mail: " + mail));
user.setOkrChampion(true);
userRepository.save(user);
userPersistenceService.save(user);
}
}
}
2 changes: 1 addition & 1 deletion backend/src/main/java/ch/puzzle/okr/dto/UserDto.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@
import java.util.List;

public record UserDto(Long id, int version, String firstname, String lastname, String email,
List<UserTeamDto> userTeamList, boolean isOkrChampion, boolean isWriteable) {
List<UserTeamDto> userTeamList, boolean isOkrChampion) {
}
2 changes: 1 addition & 1 deletion backend/src/main/java/ch/puzzle/okr/mapper/UserMapper.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,6 @@ public UserDto toDto(User user) {
teamMapper.toDto(ut.getTeam()), ut.isTeamAdmin())).collect(Collectors.toList());

return new UserDto(user.getId(), user.getVersion(), user.getFirstname(), user.getLastname(), user.getEmail(),
userTeams, user.isOkrChampion(), user.isWriteable());
userTeams, user.isOkrChampion());
}
}
20 changes: 4 additions & 16 deletions backend/src/main/java/ch/puzzle/okr/models/User.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
@Entity
// table cannot be named "user" since it is a reserved keyword of Postgres
@Table(name = "person")
public class User implements WriteableInterface {
public class User {
@Id
@GeneratedValue(strategy = GenerationType.AUTO, generator = "sequence_person")
@SequenceGenerator(name = "sequence_person", allocationSize = 1)
Expand Down Expand Up @@ -45,8 +45,6 @@ public class User implements WriteableInterface {
@Column(nullable = false, columnDefinition = "boolean default false")
private boolean isOkrChampion = false;

private transient boolean writeable;

public User() {
}

Expand Down Expand Up @@ -108,20 +106,10 @@ public void setOkrChampion(boolean okrChampion) {
isOkrChampion = okrChampion;
}

@Override
public boolean isWriteable() {
return writeable;
}

@Override
public void setWriteable(boolean writeable) {
this.writeable = writeable;
}

@Override
public String toString() {
return "User{" + "id=" + id + ", version=" + version + ", firstname='" + firstname + '\'' + ", lastname='"
+ lastname + '\'' + ", email='" + email + '\'' + ", writeable=" + writeable + '}';
+ lastname + '\'' + ", email='" + email + '\'' + '}';
}

@Override
Expand All @@ -133,12 +121,12 @@ public boolean equals(Object o) {
User user = (User) o;
return Objects.equals(id, user.id) && Objects.equals(version, user.version)
&& Objects.equals(firstname, user.firstname) && Objects.equals(lastname, user.lastname)
&& Objects.equals(email, user.email) && Objects.equals(writeable, user.writeable);
&& Objects.equals(email, user.email);
}

@Override
public int hashCode() {
return Objects.hash(id, version, firstname, lastname, email, writeable);
return Objects.hash(id, version, firstname, lastname, email);
}

public static final class Builder {
Expand Down
14 changes: 9 additions & 5 deletions backend/src/main/java/ch/puzzle/okr/models/UserTeam.java
Original file line number Diff line number Diff line change
Expand Up @@ -119,27 +119,31 @@ public static class Builder {
private Team team;
private boolean isTeamAdmin;

public Builder setId(Long id) {
public static Builder builder() {
return new Builder();
}

public Builder withId(Long id) {
this.id = id;
return this;
}

public Builder setVersion(int version) {
public Builder withVersion(int version) {
this.version = version;
return this;
}

public Builder setUser(User user) {
public Builder withUser(User user) {
this.user = user;
return this;
}

public Builder setTeam(Team team) {
public Builder withTeam(Team team) {
this.team = team;
return this;
}

public Builder setTeamAdmin(boolean isAdmin) {
public Builder withTeamAdmin(boolean isAdmin) {
this.isTeamAdmin = isAdmin;
return this;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@

import java.util.List;

import static ch.puzzle.okr.service.authorization.AuthorizationService.hasRoleWriteAndReadAll;

@Service
public class UserAuthorizationService {
private final UserBusinessService userBusinessService;
Expand All @@ -22,9 +20,6 @@ public UserAuthorizationService(UserBusinessService userBusinessService,

public List<User> getAllUsers() {
AuthorizationUser authorizationUser = authorizationService.getAuthorizationUser();
boolean isWritable = hasRoleWriteAndReadAll(authorizationUser);
List<User> allUsers = userBusinessService.getAllUsers();
allUsers.forEach(user -> user.setWriteable(isWritable));
return allUsers;
return userBusinessService.getAllUsers();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,12 @@ public synchronized User getOrCreateUser(User user) {
Optional<User> savedUser = getRepository().findByEmail(user.getEmail());
return savedUser.orElseGet(() -> getRepository().save(user));
}

public Optional<User> findByEmail(String email) {
return getRepository().findByEmail(email);
}

public User save(User user) {
return getRepository().save(user);
}
}
6 changes: 3 additions & 3 deletions backend/src/test/java/ch/puzzle/okr/TestHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ public static User defaultOkrChampion(Long id) {
return user;
}

public static User defaultUserWithTeams(Long id, List<Team> adminTeams, List<Team> memberTeams) {
var user = defaultUser(id);
public static User defaultUserWithTeams(Long userId, List<Team> adminTeams, List<Team> memberTeams) {
var user = defaultUser(userId);
var adminUserTeams = adminTeams.stream()
.map(t -> UserTeam.Builder.builder().withTeamAdmin(true).withTeam(t).withUser(user).build());
var memberUserTeams = memberTeams.stream()
Expand Down Expand Up @@ -73,7 +73,7 @@ public static Jwt defaultJwtToken() {
return mockJwtToken(FIRSTNAME, LASTNAME, EMAIL);
}

public static Jwt mockJwtToken(User user, List<String> roles) {
public static Jwt mockJwtToken(User user) {
return mockJwtToken(user.getFirstname(), user.getLastname(), user.getEmail());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,14 +141,16 @@ void serviceLayerCheck() {
.layer("PersistenceService").definedBy("..service.persistence..") //
.layer("Repository").definedBy("..repository..") //
.layer("Mapper").definedBy("..mapper..") //
.layer("ApplicationListener").definedBy("..applicationlistener..") //

.whereLayer("Controller").mayNotBeAccessedByAnyLayer() //
.whereLayer("AuthorizationService").mayOnlyBeAccessedByLayers("Controller") //
.whereLayer("BusinessService")
.mayOnlyBeAccessedByLayers("Controller", "AuthorizationService", "Mapper", "BusinessService") //
.whereLayer("ValidationService").mayOnlyBeAccessedByLayers("BusinessService") //
.whereLayer("PersistenceService")
.mayOnlyBeAccessedByLayers("BusinessService", "PersistenceService", "ValidationService") //
.mayOnlyBeAccessedByLayers("BusinessService", "PersistenceService", "ValidationService",
"ApplicationListener") //
.whereLayer("Repository").mayOnlyBeAccessedByLayers("PersistenceService"); //

layeredArchitecture.check(importedClasses);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import ch.puzzle.okr.models.overview.Overview;
import ch.puzzle.okr.models.overview.OverviewId;
import ch.puzzle.okr.service.authorization.OverviewAuthorizationService;
import ch.puzzle.okr.service.business.OrganisationBusinessService;
import org.hamcrest.Matchers;
import org.hamcrest.core.Is;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -40,8 +39,6 @@ class OverviewControllerIT {
private MockMvc mvc;
@MockBean
private OverviewAuthorizationService overviewAuthorizationService;
@MockBean
OrganisationBusinessService organisationBusinessService;
// Dashboard and OverviewMapper are required for testing
@SpyBean
private OverviewMapper overviewMapper;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import org.springframework.test.web.servlet.result.MockMvcResultMatchers;
import org.springframework.web.server.ResponseStatusException;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
Expand All @@ -44,25 +43,25 @@ class TeamControllerIT {
static Team teamPuzzle = Team.Builder.builder().withId(5L).withName(PUZZLE).build();
static Team teamOKR = Team.Builder.builder().withId(7L).withName("OKR").build();
static List<Team> teamList = Arrays.asList(teamPuzzle, teamOKR);
static TeamDto teamPuzzleDto = new TeamDto(5L, 3, PUZZLE, new ArrayList<>(), false);
static TeamDto teamOkrDto = new TeamDto(7L, 4, "OKR", new ArrayList<>(), false);
static TeamDto teamPuzzleDto = new TeamDto(5L, 3, PUZZLE);
static TeamDto teamOkrDto = new TeamDto(7L, 4, "OKR");

private static final String CREATE_NEW_TEAM = """
{
"id": null, "name": "OKR-Team", "organisations": []
"id": null, "name": "OKR-Team", "organisations": []
}
""";
private static final String CREATE_NEW_TEAM_WITH_NULL_VALUES = """
{
"id": null, "name": null, "organisations": null
"id": null, "name": null, "organisations": null
}
""";
private static final String RESPONSE_NEW_TEAM = """
{"id":7,"version":4,"name":"OKR","organisations":[],"filterIsActive":false}""";

private static final String UPDATE_TEAM = """
{
"id": 1, "name": "OKR-Team", "organisations": []
"id": 1, "name": "OKR-Team", "organisations": []
}
""";

Expand All @@ -75,8 +74,8 @@ class TeamControllerIT {

@BeforeEach
void setUp() {
BDDMockito.given(teamMapper.toDto(teamPuzzle, List.of())).willReturn(teamPuzzleDto);
BDDMockito.given(teamMapper.toDto(teamOKR, List.of())).willReturn(teamOkrDto);
BDDMockito.given(teamMapper.toDto(teamPuzzle)).willReturn(teamPuzzleDto);
BDDMockito.given(teamMapper.toDto(teamOKR)).willReturn(teamOkrDto);
}

@Test
Expand All @@ -93,8 +92,8 @@ void shouldGetAllTeams() throws Exception {
void shouldGetAllTeamsWhenNoQuarterParamIsPassed() throws Exception {
BDDMockito.given(teamAuthorizationService.getAllTeams()).willReturn(teamList);
mvc.perform(get(BASE_URL).contentType(MediaType.APPLICATION_JSON)).andExpectAll();
BDDMockito.verify(teamMapper).toDto(teamOKR, List.of());
BDDMockito.verify(teamMapper).toDto(teamPuzzle, List.of());
BDDMockito.verify(teamMapper).toDto(teamOKR);
BDDMockito.verify(teamMapper).toDto(teamPuzzle);
}

@Test
Expand Down Expand Up @@ -127,11 +126,10 @@ void shouldReturnResponseStatusExceptionWhenCreatingObjectiveWithNullValues() th

@Test
void shouldReturnUpdatedTeam() throws Exception {
TeamDto teamDto = new TeamDto(1L, 0, "OKR-Team", new ArrayList<>(), false);
Team team = Team.Builder.builder().withId(1L).withName("OKR-Team")
.withAuthorizationOrganisation(new ArrayList<>()).build();
TeamDto teamDto = new TeamDto(1L, 0, "OKR-Team");
Team team = Team.Builder.builder().withId(1L).withName("OKR-Team").build();

BDDMockito.given(teamMapper.toDto(any(), any())).willReturn(teamDto);
BDDMockito.given(teamMapper.toDto(any())).willReturn(teamDto);
BDDMockito.given(teamAuthorizationService.updateEntity(any(), anyLong())).willReturn(team);

mvc.perform(put(URL_TEAM_1).contentType(MediaType.APPLICATION_JSON).content(UPDATE_TEAM)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.springframework.test.web.servlet.MockMvc;
import org.springframework.test.web.servlet.result.MockMvcResultMatchers;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
Expand All @@ -41,8 +42,8 @@ class UserControllerIT {
static User userBob = User.Builder.builder().withId(9L).withFirstname(FIRSTNAME_2).withLastname(LASTNAME_2)
.withEmail(EMAIL_2).build();
static List<User> userList = Arrays.asList(userAlice, userBob);
static UserDto userAliceDto = new UserDto(2L, 3, FIRSTNAME_1, LASTNAME_1, EMAIL_1, true);
static UserDto userBobDto = new UserDto(9L, 4, FIRSTNAME_2, LASTNAME_2, EMAIL_2, false);
static UserDto userAliceDto = new UserDto(2L, 3, FIRSTNAME_1, LASTNAME_1, EMAIL_1, new ArrayList<>(), false);
static UserDto userBobDto = new UserDto(9L, 4, FIRSTNAME_2, LASTNAME_2, EMAIL_2, new ArrayList<>(), false);
@Autowired
private MockMvc mvc;
@MockBean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@
import org.springframework.core.convert.converter.Converter;
import org.springframework.security.oauth2.jwt.Jwt;

import java.util.List;

import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertSame;

Expand All @@ -18,13 +16,6 @@ public class JwtConverterFactoryIT {
@Autowired
private JwtConverterFactory jwtConverterFactory;

@Test
void getJwtOrganisationConverter() {
Converter<Jwt, List<String>> converter = jwtConverterFactory.getJwtOrganisationConverter();
assertNotNull(converter);
assertSame(converter, jwtConverterFactory.getJwtOrganisationConverter());
}

@Test
void getJwtUserConverter() {
Converter<Jwt, User> converter = jwtConverterFactory.getJwtUserConverter();
Expand Down
Loading

0 comments on commit e687228

Please sign in to comment.