Skip to content

task ready#31

Open
sergeywhite86 wants to merge 4 commits into
EffectiveMobile:mainfrom
sergeywhite86:dev
Open

task ready#31
sergeywhite86 wants to merge 4 commits into
EffectiveMobile:mainfrom
sergeywhite86:dev

Conversation

@sergeywhite86

Copy link
Copy Markdown

файл .env добавил для запуска приложения


@Override
@PostMapping("/login")
public Map<String, String> login(@Valid @RequestBody AuthRequest request) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Вместо Map.of("token", token) лучше вернуть DTO (AuthResponse)
Причины:
Явный контракт API (видно, какие поля есть в ответе)
Легче расширять (например, добавить refreshToken, expiresIn)

@PreAuthorize("hasRole('ADMIN')")
@GetMapping
public List<TodoDto> getAllTodos(@RequestParam(defaultValue = "10")
int limit, @RequestParam(defaultValue = "0") int offset) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

limit/offset создают впечатление пагинации, но на самом деле мы не возвращаем total, hasNextPage и т.п. Лучше использовать DTO с метаинформацией (PageResponse).

Naming: getMyTodos не очень REST-friendly. Стоит переименовать в getCurrentUserTodos или getUserTodos

Ниже так же, и поправь naming RESTful api

- changeSet:
id: 1
author: sergey_white
changes:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

preconditions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants