Skip to content

bugfix/comm-4#136

Merged
priagungs merged 15 commits intorelease/2019-07-08from
bugfix/comm-4
Jul 1, 2019
Merged

bugfix/comm-4#136
priagungs merged 15 commits intorelease/2019-07-08from
bugfix/comm-4

Conversation

@priagungs
Copy link
Copy Markdown
Collaborator

bug fix on chatroom sorting

@priagungs priagungs added the bug Something isn't working label Jun 30, 2019

@Configuration
public class SwaggerConfiguration {
public class SwaggerConfiguration extends WebMvcConfigurerAdapter {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why is this added?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Just forget to remove it, i added swagger ui config in my local. Swagger ui doesn't work in release branch, and i forget to raise issue about it. Will be removed and issue will be raised soon

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yes, in release branch swagger was not working, but it is fixed in PR for CORE-39, so after that PR is merged, swagger should be working

@WithAnyRole(roles = {
Role.STUDENT, Role.MENTOR, Role.JUDGE, Role.ADMIN
})
@WithAnyRole
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This will allow guest to access, is this the expected behavior?

import org.springframework.http.HttpStatus;

import java.util.ArrayList;
import java.util.Comparator;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is this used? If not please remove

.map(user -> chatroomRepository.findAllByTitleContainingIgnoreCaseAndMembersOrderByCreatedAtDesc(
.map(user -> chatroomRepository.findAllByTitleContainingIgnoreCaseAndMembersOrderByUpdatedAtDesc(
keyword, user, pageable))
.orElse(PageHelper.empty(pageable));
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Use orElseGet so that empty page is not created on method execution

@jonathan016 jonathan016 self-requested a review June 30, 2019 10:55
Session session
@RequestParam(required = false, defaultValue = "10") int size
) {
if (chatroomId.equalsIgnoreCase("public")) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

remember to refactor this to be moved in service layer later

@priagungs priagungs merged commit b9c2a03 into release/2019-07-08 Jul 1, 2019
@priagungs priagungs deleted the bugfix/comm-4 branch August 17, 2019 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants