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.

Sign up for GitHub

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

Jump to bottom

CHE-1078: UserService structural refactorings #1393

Merged
merged 1 commit into from
Jul 5, 2016
Merged

CHE-1078: UserService structural refactorings #1393

merged 1 commit into from
Jul 5, 2016

Conversation

voievodin
Copy link
Contributor

I separated changes on different commits which can be reviewed separately:

  • Profile & User models
  • Components separation
    • Extract preferences API from the ProfileService
    • Add Manager layer for profile/user/preferences
    • Rewrite REST services tests using RestAssured
  • Dashboard refactorings
  • Infrastructure adaptation to the refactorings

@skabashnyuk, @benoitf, @akorneta please review the pull request.
Please pay attention on classes User, Profile, UserManager, ProfileManager, PreferencesManager, UserService, ProfileService, PreferencesService

@voievodin voievodin mentioned this pull request May 31, 2016
Merged
benoitf
benoitf reviewed May 31, 2016
View reviewed changes
...ster/che-core-api-user/src/main/java/org/eclipse/che/api/user/server/PreferencesService.java
}
return preferencesManager.update(userId(), preferences);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

what about a update(String key, String value) and remove(String key) (no bulk mode)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it may be useful

@codenvy-ci
Copy link

Build # 760 - FAILED

Please check console output at http://ci.codenvy-dev.com/jenkins/job/che-pullrequests-build/760/ to view the results.

benoitf
benoitf reviewed May 31, 2016
View reviewed changes
wsmaster/che-core-api-user/src/main/java/org/eclipse/che/api/user/server/UserService.java
* when password violates any rule
*/
private void checkPassword(String password) throws BadRequestException {
private static void checkPassword(String password) throws BadRequestException {
if (password == null) {
throw new BadRequestException("Password required");
Copy link
Contributor

Choose a reason for hiding this comment

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

not covered by test

@codenvy-ci
Copy link

Build # 774 - FAILED

Please check console output at http://ci.codenvy-dev.com/jenkins/job/che-pullrequests-build/774/ to view the results.

@voievodin
Copy link
Contributor Author

voievodin commented Jun 1, 2016 •
edited
Loading

I added tests and fixes.
As created:created preference moving to attributes requires migration i decided to create a separate issue for that

@skabashnyuk, @benoitf are you okay with the changes?

@benoitf
Copy link
Contributor

benoitf commented Jun 1, 2016

@evoevodin ok for the changes, thx!
reviewing code, it seems also that now ServiceClient methods should use Promise instead of AsyncRequestCallback

void createUser(@NotNull String token, boolean isTemporary, AsyncRequestCallback callback);
--> Promise createuser(@NotNull String token, boolean isTemporary)

@benoitf benoitf added this to the 4.4.0 milestone Jun 1, 2016
@codenvy-ci
Copy link

Build # 785 - FAILED

Please check console output at http://ci.codenvy-dev.com/jenkins/job/che-pullrequests-build/785/ to view the results.

@voievodin voievodin force-pushed the CHE-1078 branch 2 times, most recently from 1c6a463 to 1148b22 Compare June 7, 2016 16:08
@codenvy-ci
Copy link

Build # 831 - FAILED

Please check console output at http://ci.codenvy-dev.com/jenkins/job/che-pullrequests-build/831/ to view the results.

@voievodin voievodin mentioned this pull request Jun 8, 2016
Merged
@riuvshin riuvshin removed this from the 4.4.0 milestone Jun 23, 2016
@codenvy-ci
Copy link

Build # 1015 - FAILED

Please check console output at http://ci.codenvy-dev.com/jenkins/job/che-pullrequests-build/1015/ to view the results.

@codenvy-ci
Copy link

Build # 1026 - FAILED

Please check console output at http://ci.codenvy-dev.com/jenkins/job/che-pullrequests-build/1026/ to view the results.

@codenvy-ci
Copy link

Build # 1044 - FAILED

Please check console output at http://ci.codenvy-dev.com/jenkins/job/che-pullrequests-build/1044/ to view the results.

@voievodin voievodin force-pushed the CHE-1078 branch 2 times, most recently from df487cd to a9732dc Compare June 29, 2016 07:36
@codenvy-ci
Copy link

Build success. http://ci.codenvy-dev.com/jenkins/job/che-pullrequests-build/1049/

@codenvy-ci
Copy link

Build # 1082 - FAILED

Please check console output at http://ci.codenvy-dev.com/jenkins/job/che-pullrequests-build/1082/ to view the results.

@codenvy-ci
Copy link

Build success. http://ci.codenvy-dev.com/jenkins/job/che-pullrequests-build/1084/

@codenvy-ci
Copy link

Build success. http://ci.codenvy-dev.com/jenkins/job/che-pullrequests-build/1116/

@skabashnyuk
Copy link
Contributor

ok

@voievodin voievodin merged commit 2929688 into master Jul 5, 2016
@voievodin voievodin deleted the CHE-1078 branch July 5, 2016 11:34
@codenvy-ci
Copy link

Build # 1130 - FAILED

Please check console output at http://ci.codenvy-dev.com/jenkins/job/che-pullrequests-build/1130/ to view the results.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@voievodin @codenvy-ci @benoitf @skabashnyuk @gazarenkov @sleshchenko @riuvshin