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

feat: add clubs + subscriptions #19

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

feat: add clubs + subscriptions #19

wants to merge 13 commits into from

Conversation

sedub01
Copy link
Collaborator

@sedub01 sedub01 commented Jun 9, 2024

@sedub01 sedub01 requested a review from vladimirshefer June 9, 2024 10:32
vladimirshefer
vladimirshefer reviewed Jun 9, 2024
View reviewed changes
src/main/java/com/chessgrinder/chessgrinder/controller/ClubController.java Outdated

@Secured(RoleEntity.Roles.ADMIN)
@PostMapping
public void createClub() {
Copy link
Owner

Choose a reason for hiding this comment

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

здесь тоже надо сделать чтобы принималось dto
не дефолты.
плз

Copy link
Owner

Choose a reason for hiding this comment

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

нам не нуден эндпоинт для создания дефолтного клуба. он будет создан и так черещ миграции. этот метод не нужен. нужен просто "сощдать клуб"

Copy link
Collaborator Author

@sedub01 sedub01 Jun 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Не понял, но да ладно: просто удалю метод

В таком случае константы тоже не нужны, поэтому и их удаляю

Copy link
Owner

Choose a reason for hiding this comment

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

константа нужна только одна - ID дефолтного клуба

src/main/java/com/chessgrinder/chessgrinder/controller/UserController.java Outdated
@@ -238,4 +239,26 @@ public void signUp(
.build()
);
}

@Secured(RoleEntity.Roles.ADMIN)
@PatchMapping("/{userId}/subscription")
Copy link
Owner

Choose a reason for hiding this comment

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

PostMapping plz

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ну да, там уже стоит POST

Copy link
Owner

Choose a reason for hiding this comment

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

а, ой, put

src/main/java/com/chessgrinder/chessgrinder/entities/SubscriptionEntity.java Show resolved Hide resolved
src/main/resources/db/migration/V202406080000__create_table__clubs_table.sql Outdated Show resolved Hide resolved
@sedub01 sedub01 force-pushed the feature/create-clubs branch from 028cefc to 9d7e552 Compare June 9, 2024 15:41
vladimirshefer
vladimirshefer requested changes Jun 10, 2024
View reviewed changes
src/main/java/com/chessgrinder/chessgrinder/controller/ClubController.java Outdated

@Secured(RoleEntity.Roles.ADMIN)
@PostMapping
public void createClub() {
Copy link
Owner

Choose a reason for hiding this comment

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

нам не нуден эндпоинт для создания дефолтного клуба. он будет создан и так черещ миграции. этот метод не нужен. нужен просто "сощдать клуб"

vladimirshefer
vladimirshefer requested changes Jun 10, 2024
View reviewed changes
src/main/java/com/chessgrinder/chessgrinder/controller/ClubController.java Outdated
}

@Secured(RoleEntity.Roles.ADMIN)
@PatchMapping
Copy link
Owner

Choose a reason for hiding this comment

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

PutMapping
и путь /clubs/:id

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ну как бы этот айдишник можно передавать в ДТО, ну да ладно

Copy link
Owner

Choose a reason for hiding this comment

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

да, можно, но по restful спецификации айдишник должен быть в пути. айдишник в дто мы либо проверяем на равенство либо тупо игнорируем

src/main/java/com/chessgrinder/chessgrinder/controller/ClubController.java Show resolved Hide resolved
vladimirshefer
vladimirshefer reviewed Jun 10, 2024
View reviewed changes
src/main/java/com/chessgrinder/chessgrinder/dto/ClubDto.java Outdated
private String description;
private String location;

@JsonFormat(shape = JsonFormat.Shape.STRING, pattern = "yyyy-MM-dd'T'HH:mm")
Copy link
Owner

Choose a reason for hiding this comment

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

у нас кажжется в других дто использовалась константа для формата. можешь ее найти м заиспользовать?

vladimirshefer
vladimirshefer reviewed Jun 11, 2024
View reviewed changes
Copy link
Owner

@vladimirshefer vladimirshefer left a comment

Choose a reason for hiding this comment

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

  • add clubDto to tournamentDto

on request
POST /tournament
create tournament with link to the tournamentDto.club
or if club==null, then club=clubRepo.findById(ApplicationConstants.DEFAULT_CLUB_ID)

src/main/java/com/chessgrinder/chessgrinder/controller/ClubController.java Outdated
}

@Secured(RoleEntity.Roles.ADMIN)
@PostMapping("/createClub")
Copy link
Owner

Choose a reason for hiding this comment

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

just /

src/main/java/com/chessgrinder/chessgrinder/controller/ClubController.java Outdated
@PathVariable UUID clubId,
@RequestBody ClubDto clubDto
) {
ClubEntity club = clubRepository.findById(clubId).orElseThrow(
Copy link
Owner

Choose a reason for hiding this comment

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

nitpick: optional: line separator before dot

src/main/java/com/chessgrinder/chessgrinder/controller/ClubController.java Outdated Show resolved Hide resolved
src/main/java/com/chessgrinder/chessgrinder/controller/UserController.java
.user(user)
.build();

subscriptionRepository.save(subscription);
Copy link
Owner

Choose a reason for hiding this comment

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

  • create method GET /user/{:id}/subscription to get all subscriptions of user

src/main/java/com/chessgrinder/chessgrinder/controller/UserController.java Outdated
Comment on lines 249 to 252
UserEntity user = userRepository.findById(userId).orElseThrow();
ClubEntity club = clubRepository.findById(UUID.fromString(data.getClub().getId())).orElseThrow();
SubscriptionLevelEntity subscriptionLevel = subscriptionLevelRepository.findById(
UUID.fromString(data.getSubscriptionLevel().getId())).orElseThrow();
Copy link
Owner

Choose a reason for hiding this comment

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

nitpick: return 404 not found by id :id

src/main/java/com/chessgrinder/chessgrinder/repositories/ClubRepository.java Outdated
Comment on lines 11 to 14
@Override
List<ClubEntity> findAll();
Copy link
Owner

Choose a reason for hiding this comment

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

there is another base interface which already returns list from find all
see other repositories

src/main/java/com/chessgrinder/chessgrinder/service/ClubService.java Outdated
List<ClubEntity> clubs = clubRepository.findAll();

return clubs.stream().map(clubMapper::toDto)
.sorted(Comparator.comparing(ClubDto::getRegistrationDate))
Copy link
Owner

Choose a reason for hiding this comment

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

remove

Copy link
Owner

Choose a reason for hiding this comment

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

or you can initially sort entities by createdAt, then map them all

src/main/resources/db/migration/V202406080000__create_table__clubs_table.sql Show resolved Hide resolved
src/main/resources/db/migration/V202406080000__create_table__clubs_table.sql Outdated
UNIQUE(name, location)
);

INSERT INTO clubs_table (id, name, description, location)
Copy link
Owner

Choose a reason for hiding this comment

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

  • createdBy = "DATABASE MIGRATION TOOL"

src/main/resources/db/migration/V202406080000__create_table__clubs_table.sql Outdated
);

INSERT INTO clubs_table (id, name, description, location)
VALUES('12345678-9abc-def0-1234-56789abcdef0', 'DEFAULT CLUB', 'DEFAULT DESCRIPTION', 'DEFAULT LOCATION');
Copy link
Owner

Choose a reason for hiding this comment

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

why dont u change to more random UUID?

Copy link
Owner

Choose a reason for hiding this comment

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

replace with random plz

Merge branch 'feature/create-clubs' of https://github.com/vladimirshe…
665bdd1 
…fer/ChessGrinder into feature/create-clubs

# Conflicts:
#	src/main/java/com/chessgrinder/chessgrinder/controller/ClubController.java
#	src/main/java/com/chessgrinder/chessgrinder/controller/UserController.java
#	src/main/java/com/chessgrinder/chessgrinder/service/TournamentService.java
@sedub01 sedub01 force-pushed the feature/create-clubs branch from 3826d65 to 665bdd1 Compare June 15, 2024 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vladimirshefer vladimirshefer vladimirshefer requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sedub01 @vladimirshefer