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

[backend] Adding swagger documentation on Tags and TagRules endpoints #2257

Open
wants to merge 2 commits into
base: improvment/add-team-api-doc
Choose a base branch
from

Conversation

Dimfacion
Copy link
Member

@Dimfacion Dimfacion commented Jan 20, 2025

Proposed changes

  • Adding swagger documentation on Tags and TagRules endpoints

Related issues

Checklist

  • I consider the submitted work as finished
  • I tested the code for its functionality
  • I wrote test cases for the relevant uses case
  • I added/update the relevant documentation (either on github or on notion)
  • Where necessary I refactored code to improve the overall quality
  • For bug fix -> I implemented a test that covers the bug

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...

Copy link

codecov bot commented Jan 20, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 32.79%. Comparing base (a1a2bb2) to head (fe7feac).

Additional details and impacted files
@@                        Coverage Diff                        @@
##             improvment/add-team-api-doc    #2257      +/-   ##
=================================================================
+ Coverage                          32.77%   32.79%   +0.01%     
  Complexity                          1500     1500              
=================================================================
  Files                                580      579       -1     
  Lines                              17938    17931       -7     
  Branches                            1169     1169              
=================================================================
  Hits                                5880     5880              
+ Misses                             11794    11787       -7     
  Partials                             264      264              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@impolitepanda impolitepanda self-requested a review January 22, 2025 08:25
@@ -26,6 +31,10 @@
import org.springframework.web.bind.annotation.*;

@RestController
@io.swagger.v3.oas.annotations.tags.Tag(
Copy link
Member

Choose a reason for hiding this comment

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

question: tags in @operation create new sections in the swagger. According to the doc, the behavior for this @tag should be the same. Is that the case and if yes, is that what we want?

value = {
@ApiResponse(
responseCode = "200",
description = "All the existing corresponding to the search criteria")
Copy link
Member

Choose a reason for hiding this comment

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

All the existing -> All the existing tags

@@ -26,6 +31,10 @@
import org.springframework.web.bind.annotation.*;

@RestController
@io.swagger.v3.oas.annotations.tags.Tag(
name = "Tags management",
description = "Endpoints to manage tags")
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should describe here what tags are or put a link to the public doc as well ?

@@ -72,6 +96,8 @@ public Tag createTag(@Valid @RequestBody TagCreateInput input) {
@Secured(ROLE_ADMIN)
@PostMapping("/api/tags/upsert")
@Transactional(rollbackOn = Exception.class)
@ApiResponses(value = {@ApiResponse(responseCode = "200", description = "The created tag")})
Copy link
Member

Choose a reason for hiding this comment

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

"the created tag" => "the upserted tag" ?

@@ -85,15 +111,23 @@ public Tag upsertTag(@Valid @RequestBody TagCreateInput input) {

@Secured(ROLE_ADMIN)
@DeleteMapping("/api/tags/{tagId}")
public void deleteTag(@PathVariable String tagId) {
@ApiResponses(value = {@ApiResponse(responseCode = "200")})
Copy link
Member

Choose a reason for hiding this comment

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

If we return void, should this be a 204 ?

@Operation(summary = "Get TagRule by Id")
public TagRuleOutput findTagRule(@PathVariable @NotBlank final String tagRuleId) {
@Operation(description = "Get TagRule by Id", summary = "Get TagRule")
@ApiResponses(value = {@ApiResponse(responseCode = "200", description = "The tag rule")})
Copy link
Member

Choose a reason for hiding this comment

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

Sometime, we talk about TagRule and other "tag rule". Should we uniformize this?

@@ -58,7 +70,8 @@ public List<TagRuleOutput> tags() {
@ApiResponse(responseCode = "200", description = "TagRule deleted"),
Copy link
Member

Choose a reason for hiding this comment

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

204 instead of 200 ?

@Valid @RequestBody final TagRuleInput input) {
return tagRuleMapper.toTagRuleOutput(
tagRuleService.updateTagRule(tagRuleId, input.getTagName(), input.getAssetGroups()));
}

@LogExecutionTime
@PostMapping(TagRuleApi.TAG_RULE_URI + "/search")
@Operation(summary = "Search TagRule")
@Operation(
description = "Search TagRule corresponding to search criteria",
Copy link
Member

Choose a reason for hiding this comment

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

TagRule -> TagRules

@Operation(summary = "Search TagRule")
@Operation(
description = "Search TagRule corresponding to search criteria",
summary = "Search TagRule")
Copy link
Member

Choose a reason for hiding this comment

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

TagRule -> TagRules

@@ -18,8 +19,10 @@
public class TagRuleInput {
@NotBlank(message = MANDATORY_MESSAGE)
@JsonProperty("tag_name")
@Schema(description = "Name of the tag rule")
Copy link
Member

Choose a reason for hiding this comment

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

Name of the tag rule or name of the tag ? (variable name vs doc is misleading, I think)

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.

3 participants