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.

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

feat: improve contact endpoints for management front #82

Merged
merged 28 commits into from
Sep 20, 2024

Conversation

BettyB979
Copy link
Contributor

No description provided.

@BettyB979 BettyB979 requested a review from davdarras September 6, 2024 10:38
@BettyB979 BettyB979 marked this pull request as ready for review September 6, 2024 10:38
Copy link
Contributor

@davdarras davdarras left a comment

Choose a reason for hiding this comment

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

Waiting for the 🍺

HttpHeaders responseHeaders = new HttpHeaders();
responseHeaders.set(HttpHeaders.LOCATION,
ServletUriComponentsBuilder.fromCurrentRequest().toUriString());
return ResponseEntity.status(HttpStatus.CREATED).headers(responseHeaders)
Copy link
Contributor

Choose a reason for hiding this comment

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

why setting response headers here ? The method could return void, and set the created http status in an annotation

})
public ResponseEntity postSurveyUnitComment(@PathVariable String id, @Valid @RequestBody SurveyUnitCommentInputDto surveyUnitCommentDto) {

SurveyUnit surveyUnit = surveyUnitService.findbyId(id);
Copy link
Contributor

Choose a reason for hiding this comment

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

setting entity in the controller, why setting comments in the survey unit object ?


String getIdSu();
String getIdentificationCode();
String getIdentificationName();
Copy link
Contributor

Choose a reason for hiding this comment

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

why an interface for this ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Passing through an interface acts as a converter between my SearchSurveyUnitDto and the result of my query.

@RequestParam(required = false) String identificationName,
public Page<SearchSurveyUnitDto> searchSurveyUnits(
@RequestParam(required = true) String searchParam,
@RequestParam(required = true) @Valid @ValidSurveyUnitParam @Schema(description = "id or code or name")String searchType,
Copy link
Contributor

Choose a reason for hiding this comment

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

does it work without @validated on the controller class ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the @validated annotation is present on the controller. And it does not work if I remove it

@RequestParam(required = false) String identificationName,
public Page<SearchSurveyUnitDto> searchSurveyUnits(
@RequestParam(required = true) String searchParam,
@RequestParam(required = true) @Valid @ValidSurveyUnitParam @Schema(description = "id or code or name")String searchType,
Copy link
Contributor

Choose a reason for hiding this comment

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

It should be working without custom annotations here by giving directly the enum as parameter

Copy link
Contributor

Choose a reason for hiding this comment

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

Same goes for the other custom validator annotation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The annotation allows to customize the error message and also ignore the case passed in the parameter. I don't know if this is possible without the custom annotation?

Copy link

@BettyB979 BettyB979 merged commit f7393e1 into develop Sep 20, 2024
7 checks passed
@BettyB979 BettyB979 deleted the management-front-endpoints branch September 20, 2024 10:02
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