[backend/frontend] Restarting an atomic testing keeps the same Inject UUID #1901
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
the
filigran team
isselparra
force-pushed
the
issue/1847
branch
2 times, most recently
from
e4a1047 to
cb41d05
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release/1.10.0 #1901 +/- ##
====================================================
+ Coverage 33.62% 34.10% +0.48%
- Complexity 1671 1700 +29
====================================================
Files 572 572
Lines 16697 16718 +21
Branches 970 969 -1
====================================================
+ Hits 5614 5702 +88
+ Misses 10829 10756 -73
- Partials 254 260 +6 ☔ View full report in Codecov by Sentry. |
isselparra
force-pushed
the
issue/1847
branch
6 times, most recently
from
f7e1608 to
2db9515
Compare
MarineLeM
reviewed
Nov 25, 2024
MarineLeM
reviewed
Nov 26, 2024
| public InjectResultOverviewOutput launchAtomicTesting( | ||
| @PathVariable @NotBlank final String atomicTestingId) { | ||
| return atomicTestingService.launch(atomicTestingId); | ||
| } |
There was a problem hiding this comment.
The API routes are much clearer now, thank you!
|
@isselparra When I try to relaunch an atomic test, I consistently encounter a 500 error. It seems like there might be an issue on the backend. |
MarineLeM
reviewed
Nov 26, 2024
antoinemzs
reviewed
Nov 26, 2024
Comment on lines
105
to
146
| @Transactional | ||
| public InjectResultOverviewOutput duplicate(String id) { | ||
| Inject injectOrigin = injectRepository.findById(id).orElseThrow(ElementNotFoundException::new); | ||
| Inject duplicatedInject = InjectUtils.duplicateInject(injectOrigin); | ||
| duplicatedInject.setTitle(duplicateString(duplicatedInject.getTitle())); | ||
| Inject savedInject = injectRepository.save(duplicatedInject); | ||
| return injectMapper.toInjectResultOverviewOutput(savedInject); | ||
| } | ||
|
|
||
| @Transactional | ||
| public InjectResultOverviewOutput launch(String id) { | ||
| Inject inject = injectRepository.findById(id).orElseThrow(ElementNotFoundException::new); | ||
| inject.clean(); | ||
| inject.setUpdatedAt(Instant.now()); | ||
|
|
||
| InjectStatus injectStatus = setInjectStatusAsQueuing(inject); | ||
| inject.setStatus(injectStatus); | ||
|
|
||
| return injectMapper.toInjectResultOverviewOutput(injectRepository.save(inject)); | ||
| } | ||
|
|
||
| @Transactional | ||
| public InjectResultOverviewOutput relaunch(String id) { | ||
| Inject injectOrigin = injectRepository.findById(id).orElseThrow(ElementNotFoundException::new); | ||
| Inject duplicatedInject = InjectUtils.duplicateInject(injectOrigin); | ||
|
|
||
| Inject savedInject = injectRepository.save(duplicatedInject); | ||
|
|
||
| InjectStatus injectStatus = setInjectStatusAsQueuing(savedInject); | ||
| savedInject.setStatus(injectStatus); | ||
|
|
||
| injectDocumentRepository.deleteDocumentsFromInject(id); | ||
| injectRepository.deleteById(id); | ||
|
|
||
| return injectMapper.toInjectResultOverviewOutput(injectRepository.save(savedInject)); | ||
| } |
There was a problem hiding this comment.
issue: we should avoid duplicating too much code, perhaps we could encapsulate some behaviour
There was a problem hiding this comment.
I mutualise as much as possible some behaviors
Let me know if you have any other suggestions
MarineLeM
reviewed
Nov 26, 2024
openbas-api/src/test/java/io/openbas/rest/AtomicTestingApiTest.java
Outdated
Show resolved
Hide resolved
isselparra
force-pushed
the
issue/1847
branch
from
cce443e to
e651b10
Compare
|
@MarineLeM I fixed the problem you were having with the relaunch of an Inject |
MarineLeM
reviewed
Nov 27, 2024
openbas-api/src/main/java/io/openbas/rest/atomic_testing/AtomicTestingApi.java
Show resolved
Hide resolved
MarineLeM
reviewed
Nov 27, 2024
openbas-front/src/admin/components/atomic_testings/atomic_testing/AtomicTestingHeader.tsx
Show resolved
Hide resolved
MarineLeM
approved these changes
Nov 27, 2024
isselparra
force-pushed
the
issue/1847
branch
from
f6dab08 to
36a7099
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed changes
Atomic TestingAPI changes:
Atomic testing duplication route
Before:
@PostMapping("/{atomicTestingId}")After:
@PostMapping("/{atomicTestingId}/duplicate")Atomic testing launching route
Before:
@GetMapping("/try/{injectId}")After:
@PostMapping("/{atomicTestingId}/launch")Atomic testing relaunching route (new)
@PostMapping("/{atomicTestingId}/relaunch")Inject launching (deleted)
Not used in the app
@GetMapping("/api/injects/try/{injectId}")Refactor:
Change
InjectDuplicateServiceto reduce the number of db callsRelated 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...