-
Notifications
You must be signed in to change notification settings - Fork 90
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
Adding Swagger doc on teams endpoints #2256
base: improvment/add-swagger-doc
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## improvment/add-swagger-doc #2256 +/- ##
================================================================
- Coverage 32.87% 32.77% -0.10%
+ Complexity 1509 1500 -9
================================================================
Files 582 580 -2
Lines 17930 17938 +8
Branches 1158 1169 +11
================================================================
- Hits 5894 5880 -14
- Misses 11772 11794 +22
Partials 264 264 ☔ View full report in Codecov by Sentry. |
if (preAuthorizeAnnotation != null) { | ||
specialRequirement += "<br>**Special Requirement :** "; | ||
String preAuthorize = preAuthorizeAnnotation.value(); | ||
if (preAuthorize.startsWith("isExercisePlanner") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For all the tests, isn't startsWith a bit risky, instead of equals ? (especially for the tests on isPlanner, isObserver, etc.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, code could probably be factorized a bit using regex ? With a Pattern like Pattern.compile("is(.?)Planner") ?
If group 0 is empty, give the isPlanner message, else if group 0 = Scenario give the scenarioPlanner message, else give the simulationPlanner message ?
But not sure if it's worth doing or not
@@ -165,13 +192,19 @@ public Team upsertTeam(@Valid @RequestBody TeamCreateInput input) { | |||
|
|||
@DeleteMapping("/api/teams/{teamId}") | |||
@PreAuthorize("isPlanner()") | |||
public void deleteTeam(@PathVariable String teamId) { | |||
@ApiResponses(value = {@ApiResponse(responseCode = "200")}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
204 instead of 200 ?
teamRepository.deleteById(teamId); | ||
} | ||
|
||
@PutMapping("/api/teams/{teamId}") | ||
@PreAuthorize("isPlanner()") | ||
public Team updateTeam(@PathVariable String teamId, @Valid @RequestBody TeamUpdateInput input) { | ||
@ApiResponses(value = {@ApiResponse(responseCode = "200", description = "The updated team")}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't think about it before this one, but shouldn't we also describe errors ? (like updating a team taht deosn't exist ?)
private String organizationId; | ||
|
||
@JsonProperty("team_tags") | ||
@Schema(description = "Id of the tags linked to the team") | ||
private List<String> tagIds = new ArrayList<>(); | ||
|
||
@JsonProperty("team_exercises") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we use this opportunity to rename it to team_simulation ?
public class TeamUpdateInput { | ||
|
||
@NotBlank(message = MANDATORY_MESSAGE) | ||
@JsonProperty("team_name") | ||
@Schema(description = "Name of the team") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably create a super class here to mutualize code between the various team input types
|
||
@JsonProperty("team_exercises") | ||
private Set<String> exercises = new HashSet<>(); | ||
@Schema(description = "List of exercises of the team") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
List of simulation IDs
|
||
@JsonProperty("team_scenarios") | ||
private Set<String> scenarios = new HashSet<>(); | ||
@Schema(description = "List of scenarios of the team") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
List of scenario IDs
public long getUsersNumber() { | ||
return this.users.size(); | ||
} | ||
|
||
// region transient | ||
@JsonProperty("team_exercise_injects") | ||
private Set<String> exercisesInjects = new HashSet<>(); | ||
@Schema(description = "List of injects from all simulations of the team") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
List of inject IDs
public long getExercisesInjectsNumber() { | ||
return this.exercisesInjects.size(); | ||
} | ||
|
||
@JsonProperty("team_scenario_injects") | ||
@Schema(description = "List of injects from all scenarios of the team") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
List of inject IDs
public long getScenariosInjectsNumber() { | ||
return this.scenariosInjects.size(); | ||
} | ||
|
||
@JsonIgnore private List<InjectExpectation> injectExpectations = new ArrayList<>(); | ||
|
||
@JsonProperty("team_inject_expectations") | ||
@Schema(description = "List of expectations id linked to this team") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
expectations id -> expectation ids
Proposed changes
Related issues
Checklist
Further comments
If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...