Added teams functionality by maximAtanasov · Pull Request #380 · adessoSE/coderadar

maximAtanasov · 2020-06-16T14:38:12Z

Coderadar now provides functionality for adding and managing teams. Each team can be assigned to many projects and with different roles. Users now also have a role for any project that they have access to. Basic authentication and UI elements have also been added. Lots of works still needs to be done in the UI (and the Back-End will likely have to be adjusted), but this can be done later after we have discussed some of the details about the authentication concept and the UI elements.

AddTeamToProjectAdapter CreateTeamAdapter DeleteTeamAdapter ListProjectsForTeamAdapter ListTeamsForUserAdapter ListUsersAdapter TeamMapper and applied spotless.

…fix minor issues

…ent ListTeamsService

maximAtanasov · 2020-08-06T09:19:26Z

Please apply the tslint fixes

Done

…d-negative-tests

Add negative tests

jo2 · 2020-08-07T12:24:16Z

...java/io/reflectoring/coderadar/rest/authentication/AuthenticationServiceIntegrationTest.java

+
+  @BeforeEach
+  void setUp() {
+    coderadarConfigurationProperties.setAuthentication(


As far as I know, til now, this is the only test using such a configuration. if there are more configurations like this, we should use a spring profile for tests to handle these configurations for us.

I'm not sure what you mean by configuration. The credentials are basically mocked in these tests.

I thought this line would basically just set the property coderadar.authentication.enabled=true of he local.application.properties file. This was a misunderstanding by me.

That is correct, however I did not undestand the issue.

jo2 · 2020-08-07T12:28:00Z

...adar/rest/useradministration/permissions/RemoveUserFromProjectControllerIntegrationTest.java

+    testUser.setProjects(Collections.singletonList(testProject));
+    userRepository.save(testUser);
+  }
+


What happens if there is no user with this id for this project? What happens if user and project do not exist?

I'm not sure what you mean. The cases you describe are tested on lines 77 and 94.

The first means the user exists (has id 1) but he is not a user registered for this project. The test case you've used in line 77 is for a user which does not exist on our platform. For the second part, this file only does have 87 lines, at least in the GitHub view. What test is written in line 94?

Sorry, the lines I mentioned are for the SetUserRoleForProjectIntegrationTest. The first test checks if a user assigned to a project gets their role removed successfuly. The other two tests check that 404 is returned when a user/project does not exists. Is there something I'm missing?

jo2 · 2020-08-07T12:28:37Z

...adar/rest/useradministration/permissions/SetUserRoleForProjectControllerIntegrationTest.java

+    testUser.setPassword(PasswordUtil.hash("password1"));
+    userRepository.save(testUser);
+  }
+


What happens if there is no user with this id for this project? What happens if user and project do not exist?

jo2 · 2020-08-07T13:40:37Z

coderadar-ui/src/app/app.module.ts

+import {ContributorMergeDialogComponent} from './components/contributor-merge-dialog/contributor-merge-dialog.component';
+import {DeleteProjectDialogComponent} from './components/delete-project-dialog/delete-project-dialog.component';
+import {AddProjectToTeamDialogComponent} from './components/add-project-to-team-dialog/add-project-to-team-dialog.component';
+import {ContributorDialogComponent} from "./components/contributor-card/contributor-dialog.component";


Please execute TSLint. It should emphasize doulbe quotes in ts files and more. TSLint issues should be fixed.

Ts lint returns no errors for me. In fact running tslint --fix will remove all double quotes as those are counted as errors.

EDIT: I see the double quote now, I don't know why it was left out as I ran the linter last time. I'll fix it again.
EDIT2: Should we maybe make the linting a part of the build like it is for spotless?

I've run the linter again

Thanks, I think it would be a good idea to make it part of the build process!

jo2 · 2020-08-07T13:48:54Z

coderadar-ui/src/app/service/project.service.ts

@@ -214,7 +203,7 @@ export class ProjectService {
   * @param branch The branch to use for getting the commits.
   */
  public getCommitsForContributor(id: number, branch: string, email: string): Promise<HttpResponse<Commit[]>> {
-    return this.httpClient.get<Commit[]>(this.apiURL + 'projects/' + id + '/' + branch + '/commits?email='+email,
+    return this.httpClient.get<Commit[]>(this.apiURL + 'projects/' + id + '/' + branch + '/commits?email=' + email.replace('+', '%2B'),


Why do you replace all '+' in generated url with '%2B'? '+' has to be encoded, but I don't see any place where a '+' could be added to the url.

Also, if the urls have multiple parameter, we may consider using string literals to increase readability. That would be the scope of a new issue though.

The + sign is an edge case here. Some emails (those generate by GitHub for example) contain a plus sign. I agree about the string literals.

jo2

Some things are stil left to do but it is coming to an end.

maximAtanasov · 2020-09-02T07:13:37Z

@jo2 Do you want any more changes to be made in this PR?
I have worked on further fixes and improvements in another branch so as to not bloat this PR any more.

jo2 · 2020-09-02T14:23:47Z

What I'm stil missing for the RemoveUserFromProjectControllerIntegrationTest is this case: user and projects both exist but the projects of the user do not contain the project. In that case userRepository.removeUserRoleFromProject is called and it tries to remove a non-existing relationship. Can this cause issues? In my opinion it should be tested. Otherwise this is ready for merge.

… to remove the role and write a test for this case

maximAtanasov · 2020-09-02T17:10:44Z

Yes, you were correct in that this case wasn't handled. I've added a check for it and wrote a test.

sonarqubecloud · 2020-09-02T17:20:32Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities (and 0 Security Hotspots to review)
1 Code Smell

82.8% Coverage
0.0% Duplication

maximAtanasov and others added 30 commits May 5, 2020 15:25

add use-cases, services, ports for managing teams

d08d4ca

create controllers for team configuration

8bbff09

implemented services for team management

528d064

Merge branch 'teams' of https://github.com/adessoAG/coderadar into teams

99f5f74

implement AddUsersToTeamController

2bd9cf5

add create and delete team use-cases/ports/service

5a34eed

Merge branch 'teams' of https://github.com/adessoAG/coderadar into teams

f7ea11d

Implemented:

f44bb38

AddTeamToProjectAdapter CreateTeamAdapter DeleteTeamAdapter ListProjectsForTeamAdapter ListTeamsForUserAdapter ListUsersAdapter TeamMapper and applied spotless.

implement controllers for teams/users configuration

f9cb589

apply spotless

877268a

Merge branch 'teams' of https://github.com/adessoAG/coderadar into teams

3b26fd1

create adapters

a52b8ad

work on useradministration adapters

3336421

Merge branch 'teams' of https://github.com/adessoAG/coderadar into teams

d800d30

implement useradministration adapters

08905ea

adjust controllers

cc322c1

fix CreateTeamService

9e5bcb1

Merge branch 'teams' of https://github.com/adessoAG/coderadar into teams

d545907

fix queries

672d95e

fix typo in ProjectRepository

e7511b0

added ListTeams use-case/port/controller/adapter

162b37d

create json wrapper for ProjectRole

3dc9487

do not create multiple relationships between teams and projects

dbcb0b1

Merge branch 'teams' of https://github.com/adessoAG/coderadar into teams

89df620

minor fixes

e7dc5de

add ListUsersForProject use-case/port/adapter/service/controller and …

b8a35a4

…fix minor issues

Merge branch 'teams' of https://github.com/adessoAG/coderadar into teams

751c65e

fix the addUsersToTeam Query in TeamRepository

f7c91f1

rename DeleteUserRoleForProject* to RemoveUserFromProject* and implem…

70d38a8

…ent ListTeamsService

change method parameter of controller

411b234

maximAtanasov force-pushed the teams branch from ee7522e to 38648d2 Compare August 6, 2020 09:18

maximAtanasov requested a review from jo2 August 6, 2020 11:32

maximAtanasov and others added 5 commits August 6, 2020 16:06

Merge branch 'master' into teams

f7f378e

Merge branch 'master' into teams

dc2b28f

Merge branch 'teams' of https://github.com/adessoAG/coderadar into ad…

6e802f4

…d-negative-tests

Merge pull request #400 from adessoAG/add-negative-tests

c0790b6

Add negative tests

add tests for the AuthenticationService

7cf85aa

maximAtanasov force-pushed the teams branch from 089bf23 to 7cf85aa Compare August 7, 2020 11:30

maximAtanasov and others added 3 commits August 7, 2020 13:37

Merge branch 'master' into teams

cf75d44

fix code smells

6e69300

Merge branch 'teams' of https://github.com/adessoAG/coderadar into teams

1f6d26c

jo2 reviewed Aug 7, 2020

View reviewed changes

jo2 requested changes Aug 7, 2020

View reviewed changes

maximAtanasov and others added 2 commits August 7, 2020 17:43

apply tslint

0f5578b

Merge branch 'master' into teams

c4b5804

maximAtanasov mentioned this pull request Aug 11, 2020

Add tslint to the build process #405

Merged

Merge branch 'master' into teams

9d65cc9

throw an exception if a user is not assigned to a project when trying…

b586771

… to remove the role and write a test for this case

jo2 approved these changes Sep 3, 2020

View reviewed changes

maximAtanasov merged commit 6baf7c6 into master Sep 3, 2020

maximAtanasov deleted the teams branch September 3, 2020 07:27

Conversation

maximAtanasov commented Jun 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maximAtanasov commented Aug 6, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maximAtanasov Aug 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jo2 Aug 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jo2 left a comment

Choose a reason for hiding this comment

Uh oh!

maximAtanasov commented Sep 2, 2020

Uh oh!

jo2 commented Sep 2, 2020

Uh oh!

maximAtanasov commented Sep 2, 2020

Uh oh!

sonarqubecloud bot commented Sep 2, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

maximAtanasov commented Jun 16, 2020 •

edited

Loading

maximAtanasov Aug 7, 2020 •

edited

Loading

jo2 Aug 7, 2020 •

edited

Loading