Add project service module with key generation and existence checks#7
Add project service module with key generation and existence checks#7
Conversation
📊 Static Analysis Summary🔍 Code Quality Checks
📦 Download detailed reports from the workflow artifacts. |
1 similar comment
📊 Static Analysis Summary🔍 Code Quality Checks
📦 Download detailed reports from the workflow artifacts. |
angelmp01
force-pushed
the
chore/project-service
branch
from
78c4242 to
a7eaadf
Compare
📊 Static Analysis Summary🔍 Code Quality Checks
📦 Download detailed reports from the workflow artifacts. |
| name: ODS Team | ||
| version: v0.0.1 | ||
| servers: | ||
| - url: http://{baseurl}/api/v0 |
There was a problem hiding this comment.
The old API was /api/pub/v0, but maybe we can keep it in this way
| @PostMapping | ||
| @Override | ||
| public ResponseEntity<CreateProjectResponse> createProject(@Valid @RequestBody CreateProjectRequest createProjectRequest) { | ||
| try { |
There was a problem hiding this comment.
Add logs when the call is innitiate to start with the "auditory"
As an example:
log.info("Triggering membership request for account '{}' of user '{}' to project '{}' with role '{}'",
addUserToProjectRequest.getAccount(), addUserToProjectRequest.getUser(), projectKey, addUserToProjectRequest.getRole());
There was a problem hiding this comment.
Do you want to add a info message saying start in every method in all the controllers? In this case we can implement something with AOP. What do you think about it?
| notFoundResponse.setError("NOT_FOUND"); | ||
| notFoundResponse.setErrorKey("PROJECT_NOT_FOUND"); | ||
| notFoundResponse.setMessage(String.format("Project with key '%s' not found", projectKey)); | ||
| return ResponseEntity.status(HttpStatus.NOT_FOUND).body(notFoundResponse); |
There was a problem hiding this comment.
ResponseEntity.notFound().build()
Check if this make sense
There was a problem hiding this comment.
But I need to add a body content, I cannot use this mehtod.
| import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; | ||
| import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; | ||
|
|
||
| @SpringBootTest(classes = ProjectControllerIntegrationTest.TestConfig.class) |
There was a problem hiding this comment.
this is not exactly an integration tests.
Here it is been tested that a post goes from the controller to the service without problems, but even the service is being mocked
|
|
||
| @Service | ||
| @Slf4j | ||
| public class ProjectServiceImpl implements ProjectService { |
There was a problem hiding this comment.
use:
@AllArgsConstructor instead of having the current constructor like you have in the controller.
| import org.opendevstack.apiservice.externalservice.ocp.service.OpenshiftService; | ||
| import org.opendevstack.apiservice.serviceproject.service.GenerateProjectKeyService; | ||
|
|
||
| @ExtendWith(MockitoExtension.class) |
There was a problem hiding this comment.
And the tests???????
| <include>**/*Test.java</include> | ||
| </includes> | ||
| <systemPropertyVariables> | ||
| <spring.config.location>${SPRING_CONFIG_DIR}</spring.config.location> |
There was a problem hiding this comment.
why is needed, defining the variable is not enough?
There was a problem hiding this comment.
Do you need this config if you try to execute an isolate test within the IDE to debug it.
jorge-romero
left a comment
jorge-romero
left a comment
There was a problem hiding this comment.
Pleas have a look at my comments
2 participants
No description provided.