Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#790 members einladen #793

Merged
merged 23 commits into from
May 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
87ca431
#790 create dialog to invite members
janikEndtner Feb 2, 2024
13cbb67
[FM] Automated formating frontend
actions-user Feb 2, 2024
4e03df3
#790 format
janikEndtner Feb 2, 2024
6927b65
#790 create backend methods to create users
janikEndtner Feb 2, 2024
b31995c
#790 call backend when creating users
janikEndtner Feb 2, 2024
540b106
Merge remote-tracking branch 'origin/feature/790_members_einladen' in…
janikEndtner Feb 2, 2024
e411790
[FM] Automated formating backend
actions-user Feb 2, 2024
74a46a3
#790 add cypress tests
janikEndtner Feb 2, 2024
3f0019b
Merge remote-tracking branch 'origin/feature/790_members_einladen' in…
janikEndtner Feb 2, 2024
edcbf1c
#790 fix frontend tests
janikEndtner Feb 2, 2024
e3acf21
#790 fix column width of new-user columns
janikEndtner Feb 2, 2024
cd4beac
#790 some minor changes from review
janikEndtner Apr 9, 2024
e281ca8
#790 change form to reactiveForm for better validation
janikEndtner Apr 12, 2024
57fcf1a
[FM] Automated formating frontend
actions-user Apr 12, 2024
b42d808
#790 fix frontend unit tests
janikEndtner Apr 12, 2024
e04fea1
Merge remote-tracking branch 'origin/feature/790_members_einladen' in…
janikEndtner Apr 12, 2024
15c1024
[FM] Automated formating frontend
actions-user Apr 12, 2024
b081bfe
#790 improve email validation
janikEndtner Apr 12, 2024
b609505
#790 fix padding in input fields
janikEndtner Apr 12, 2024
ed8655f
Merge remote-tracking branch 'origin/feature/790_members_einladen' in…
janikEndtner Apr 12, 2024
088f785
Merge remote-tracking branch 'origin/multitenancy_main' into feature/…
janikEndtner Apr 26, 2024
7f86c70
Merge branch 'multitenancy_main' into feature/790_members_einladen
janikEndtner May 17, 2024
419b424
#790 fix test error messages
janikEndtner May 17, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package ch.puzzle.okr.controller;

import ch.puzzle.okr.dto.NewUserDto;
import ch.puzzle.okr.dto.UserDto;
import ch.puzzle.okr.mapper.UserMapper;
import ch.puzzle.okr.service.authorization.AuthorizationService;
Expand All @@ -10,7 +11,13 @@
import io.swagger.v3.oas.annotations.media.Schema;
import io.swagger.v3.oas.annotations.responses.ApiResponse;
import io.swagger.v3.oas.annotations.responses.ApiResponses;
import org.springframework.web.bind.annotation.*;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.PutMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;

import java.util.List;

Expand Down Expand Up @@ -68,4 +75,14 @@ public UserDto setOkrChampion(
return userMapper.toDto(user);
}

@Operation(summary = "Create users", description = "Creates a user entity for every user in the method body")
@ApiResponses(value = { @ApiResponse(responseCode = "200", description = "Returned users", content = {
@Content(mediaType = "application/json", schema = @Schema(implementation = UserDto.class)) }), })
@PostMapping(path = "/createall")
public List<UserDto> createUsers(
@io.swagger.v3.oas.annotations.parameters.RequestBody(description = "The users to create", required = true) @RequestBody List<NewUserDto> newUserDtoList) {
var createdUsers = this.userAuthorizationService.createUsers(userMapper.toUserList(newUserDtoList));
return userMapper.toDtos(createdUsers);
}

}
4 changes: 4 additions & 0 deletions backend/src/main/java/ch/puzzle/okr/dto/NewUserDto.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
package ch.puzzle.okr.dto;

public record NewUserDto(String firstname, String lastname, String email) {
}
18 changes: 18 additions & 0 deletions backend/src/main/java/ch/puzzle/okr/mapper/UserMapper.java
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package ch.puzzle.okr.mapper;

import ch.puzzle.okr.dto.NewUserDto;
import ch.puzzle.okr.dto.UserDto;
import ch.puzzle.okr.dto.UserTeamDto;
import ch.puzzle.okr.models.User;
import org.springframework.stereotype.Component;

import java.util.List;
import java.util.stream.Collectors;

@Component
Expand All @@ -16,6 +18,10 @@ public UserMapper(TeamMapper teamMapper) {
this.teamMapper = teamMapper;
}

public List<UserDto> toDtos(List<User> userList) {
return userList.stream().map(this::toDto).toList();
}

public UserDto toDto(User user) {
var userTeams = user.getUserTeamList().stream().map(
ut -> new UserTeamDto(ut.getId(), user.getVersion(), teamMapper.toDto(ut.getTeam()), ut.isTeamAdmin()))
Expand All @@ -24,4 +30,16 @@ public UserDto toDto(User user) {
return new UserDto(user.getId(), user.getVersion(), user.getFirstname(), user.getLastname(), user.getEmail(),
userTeams, user.isOkrChampion());
}

public List<User> toUserList(List<NewUserDto> newUserList) {
return newUserList.stream().map(this::toUser).toList();
}

public User toUser(NewUserDto newUserDto) {
var user = new User();
user.setFirstname(newUserDto.firstname());
user.setLastname(newUserDto.lastname());
user.setEmail(newUserDto.email());
return user;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,10 @@ public User setIsOkrChampion(long id, boolean isOkrChampion) {
OkrResponseStatusException.of(ErrorKey.NOT_AUTHORIZED_TO_WRITE, USER));
return userBusinessService.setIsOkrChampion(user, isOkrChampion);
}

public List<User> createUsers(List<User> userList) {
AuthorizationService.checkRoleWriteAndReadAll(authorizationService.updateOrAddAuthorizationUser(),
OkrResponseStatusException.of(ErrorKey.NOT_AUTHORIZED_TO_WRITE, USER));
return userBusinessService.createUsers(userList);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@
import ch.puzzle.okr.service.CacheService;
import ch.puzzle.okr.service.persistence.UserPersistenceService;
import ch.puzzle.okr.service.validation.UserValidationService;
import jakarta.transaction.Transactional;
import org.springframework.http.HttpStatus;
import org.springframework.stereotype.Service;

import java.util.List;
import java.util.Objects;
import java.util.stream.StreamSupport;

@Service
public class UserBusinessService {
Expand Down Expand Up @@ -61,4 +63,10 @@ private void checkAtLeastOneOkrChampionExists(User user) {
public User saveUser(User user) {
return userPersistenceService.save(user);
}

@Transactional
public List<User> createUsers(List<User> userList) {
var userIter = userPersistenceService.saveAll(userList);
return StreamSupport.stream(userIter.spliterator(), false).toList();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,8 @@ public User save(User user) {
public List<User> findAllOkrChampions() {
return getRepository().findByIsOkrChampion(true);
}

public Iterable<User> saveAll(List<User> userList) {
return getRepository().saveAll(userList);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -81,4 +81,29 @@ void setOkrChampion_shouldThrowErrorIfLoggedInUserIsNotOkrChampion() {
assertThrows(OkrResponseStatusException.class,
() -> userAuthorizationService.setIsOkrChampion(user.getId(), true));
}

@Test
void createUsers_shouldCallBusinessService() {
var loggedInUser = defaultUser(1L);
loggedInUser.setOkrChampion(true);

List<User> users = List.of(user, user2);
when(userBusinessService.createUsers(users)).thenReturn(users);
when(authorizationService.updateOrAddAuthorizationUser()).thenReturn(new AuthorizationUser(loggedInUser));

userAuthorizationService.createUsers(users);

verify(userBusinessService, times(1)).createUsers(users);
}

@Test
void createUsers_shouldThrowErrorIfLoggedInUserIsNotOkrChampion() {
var loggedInUser = defaultUser(1L);
loggedInUser.setOkrChampion(false);

when(authorizationService.updateOrAddAuthorizationUser()).thenReturn(new AuthorizationUser(loggedInUser));

assertThrows(OkrResponseStatusException.class,
() -> userAuthorizationService.createUsers(List.of(user, user2)));
}
}
54 changes: 53 additions & 1 deletion frontend/cypress/e2e/teammanagement.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,50 @@ describe('Team management tests', () => {
});
});

describe('invite members', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to add a test for the new user to log-in after registering, to verify the first or last name got updated and the assignment via email works? (Debatable, might be much effort to do)

it('invite two members', () => {
const mailUserClaudia = uniqueSuffix('claudia.meier@test') + '.ch';
const mailUserStefan = uniqueSuffix('stefan.schmidt@test') + '.ch';
const firstNameClaudia = uniqueSuffix('Claudia');
const firstNameStefan = uniqueSuffix('Stefan');

cy.getByTestId('invite-member').click();
fillOutNewUser(firstNameClaudia, 'Meier', mailUserClaudia);
cy.tabForward();
cy.tabForward();
cy.realPress('Enter');
fillOutNewUser(firstNameStefan, 'Schmidt', mailUserStefan);
cy.tabForward();
cy.tabForward();
cy.realPress('Enter');

// test error messages
fillOutNewUser('Robin', '', 'papierer');
cy.getByTestId('invite').click();
cy.contains('Angabe benötigt');
cy.contains('E-Mail ungültig');
cy.getByTestId('email-col_2').focus();
cy.realType('@puzzle.ch');
cy.contains('E-Mail ungültig').should('not.exist');
cy.contains('E-Mail existiert bereits');
cy.tabBackward();
cy.realType('Papirer');
cy.contains('Angabe benötigt').should('not.exist');

// delete last entry
cy.tabForward();
cy.tabForward();
cy.realPress('Enter');
cy.contains('[email protected]').should('not.exist');

// save
cy.getByTestId('invite').click();
cy.contains('Die Members wurden erfolgreich registriert');
cy.contains(firstNameClaudia);
cy.contains(firstNameStefan);
});
});

it('Navigate to Bobs profile and add him to BBT and LoremIpsum', () => {
cy.intercept('PUT', '**/updateaddteammembership/*').as('updateEsha');

Expand Down Expand Up @@ -202,7 +246,7 @@ describe('Team management tests', () => {
cy.getByTestId('edit-okr-champion-checkbox').click();
cy.getByTestId('edit-okr-champion-readonly').contains('OKR Champion:');
cy.getByTestId('edit-okr-champion-readonly').contains('Ja');
cy.contains('Der User wurde erfolgreich aktualisiert.');
cy.contains('Der Member wurde erfolgreich aktualisiert.');

// reset okr champion to false
cy.getByTestId('edit-okr-champion-edit').click();
Expand Down Expand Up @@ -362,3 +406,11 @@ function navigateToUser(userName: string) {
cy.get('td').contains(userName).click();
cy.wait('@getUser');
}

function fillOutNewUser(firstname: string, lastname: string, email: string) {
cy.realType(firstname, { delay: 1 });
cy.tabForward();
cy.realType(lastname, { delay: 1 });
cy.tabForward();
cy.realType(email, { delay: 1 });
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ const configServiceMock = {
config$: of({}),
};

describe('ApplicationHeaderComponent', () => {
describe('ApplicationTopBarComponent', () => {
let component: ApplicationTopBarComponent;
let fixture: ComponentFixture<ApplicationTopBarComponent>;
let loader: HarnessLoader;
Expand Down
36 changes: 30 additions & 6 deletions frontend/src/app/services/user.service.spec.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { getTestBed, TestBed } from '@angular/core/testing';
import { fakeAsync, getTestBed, TestBed, tick } from '@angular/core/testing';
import { HttpClientTestingModule, HttpTestingController } from '@angular/common/http/testing';
import { UserService } from './user.service';
import { users } from '../shared/testData';
import { testUser, users } from '../shared/testData';

describe('UserService', () => {
let service: UserService;
Expand All @@ -21,11 +21,11 @@ describe('UserService', () => {
httpMock.verify();
});

test('should be created', () => {
it('should be created', () => {
expect(service).toBeTruthy();
});

test('getUsers should only reload users when they are not loaded yet', (done) => {
it('getUsers should only reload users when they are not loaded yet', (done) => {
const spy = jest.spyOn(service, 'reloadUsers');
service.getUsers().subscribe(() => {
expect(spy).toBeCalledTimes(1);
Expand All @@ -38,11 +38,11 @@ describe('UserService', () => {
});
});

test('get current user should throw error, when not loaded', () => {
it('get current user should throw error, when not loaded', () => {
expect(() => service.getCurrentUser()).toThrowError('user should not be undefined here');
});

test('init current user should load user', (done) => {
it('init current user should load user', (done) => {
expect(() => service.getCurrentUser()).toThrowError('user should not be undefined here');
service.getOrInitCurrentUser().subscribe(() => {
expect(service.getCurrentUser()).toBe(users[0]);
Expand All @@ -51,4 +51,28 @@ describe('UserService', () => {
const req = httpMock.expectOne('api/v1/users/current');
req.flush(users[0]);
});

it('setIsOkrChampion should call put operation, reloadUsers and reloadCurrentUser', fakeAsync(() => {
service.setIsOkrChampion(testUser, true).subscribe();
const req = httpMock.expectOne(`api/v1/users/${testUser.id}/isokrchampion/true`);
req.flush(users[0]);

tick();

const req2 = httpMock.expectOne(`api/v1/users`);
const req3 = httpMock.expectOne(`api/v1/users/current`);
req2.flush({});
req3.flush({});
}));

it('createUsers should call createAll and reloadUsers', fakeAsync(() => {
service.createUsers(users).subscribe();
const req = httpMock.expectOne(`api/v1/users/createall`);
req.flush(users);

tick();

const req2 = httpMock.expectOne(`api/v1/users`);
req2.flush({});
}));
});
12 changes: 11 additions & 1 deletion frontend/src/app/services/user.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Injectable } from '@angular/core';
import { BehaviorSubject, Observable, of, tap } from 'rxjs';
import { HttpClient } from '@angular/common/http';
import { User } from '../shared/types/model/User';
import { NewUser } from '../shared/types/model/NewUser';

@Injectable({
providedIn: 'root',
Expand Down Expand Up @@ -50,6 +51,15 @@ export class UserService {
}

setIsOkrChampion(user: User, isOkrChampion: boolean) {
return this.httpClient.put(`${this.API_URL}/${user.id}/isokrchampion/${isOkrChampion}`, {});
return this.httpClient.put(`${this.API_URL}/${user.id}/isokrchampion/${isOkrChampion}`, {}).pipe(
tap(() => {
this.reloadUsers();
this.reloadCurrentUser().subscribe();
}),
);
}

createUsers(userList: NewUser[]) {
return this.httpClient.post<User>(`${this.API_URL}/createall`, userList).pipe(tap(() => this.reloadUsers()));
janikEndtner marked this conversation as resolved.
Show resolved Hide resolved
janikEndtner marked this conversation as resolved.
Show resolved Hide resolved
}
}
5 changes: 5 additions & 0 deletions frontend/src/app/shared/types/model/NewUser.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export interface NewUser {
firstname: string;
lastname: string;
email: string;
}
5 changes: 5 additions & 0 deletions frontend/src/app/shared/types/model/NewUserForm.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export interface NewUserForm<T> {
firstname: T;
email: T;
lastname: T;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<section mat-dialog-title>
<div class="d-flex justify-content-between align-items-center position-relative dialog-header mt-1">
<span class="title w-100 mb-2">
<span>Members registrieren</span>
</span>
</div>
</section>
<mat-dialog-content>
<div class="dialog-content" tabindex="-1">
<form>
@for (userFormGroup of form.controls; track $index) {
<div class="w-100 d-flex justify-content-between align-items-center pe-4">
<app-new-user
(removeUser)="removeUser($index)"
[index]="$index"
[userFormGroup]="userFormGroup"
[triedToSubmit]="triedToSubmit"
></app-new-user>
</div>
}
<div class="w-100 d-flex justify-content-start">
<button (click)="addUser()" class="px-0 mb-3 pe-2 align-new-icon" color="primary" mat-button>
<span class="d-flex align-items-center fw-bold add-text">
<img alt="Add User button" class="add-cross-button" src="/assets/icons/new-icon.svg" />
Weiterer Member hinzufügen
janikEndtner marked this conversation as resolved.
Show resolved Hide resolved
</span>
</button>
</div>
</form>
</div>
</mat-dialog-content>
<mat-dialog-actions>
<div class="d-flex justify-content-between p-0 mt-2" mat-dialog-actions>
<div class="d-flex p-0">
<button color="primary" [attr.data-testId]="'invite'" mat-flat-button type="submit" (click)="registerUsers()">
Einladen
</button>
<button color="primary" mat-button mat-dialog-close class="ms-2">Abbrechen</button>
</div>
</div>
</mat-dialog-actions>
Loading
Loading