feat: P4ADEV-1636 WF ClassifyIuvActivity: define classification labels #72
+721
−22
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
…ation-define-labels
|
macacia
requested review from
LarissaASLeite,
antocalo,
Benedetta-fabbri,
RiccardoGiuliani,
ElisKina-dev and
a team
as code owners
antonioT90
requested changes
Jan 8, 2025
| @Lazy | ||
| @Slf4j | ||
| @Service | ||
| public class ClassificationService { |
There was a problem hiding this comment.
Suggested change
| public class ClassificationService { | |
| public class TransferClassificationService { |
| .toList(); | ||
|
|
||
| if (labels.isEmpty()) { | ||
| throw new ClassificationException("Cannot define classification"); |
There was a problem hiding this comment.
instead of throwing an exception, could we apply a default one (eg: UNKNOWN)
| @@ -0,0 +1,21 @@ | |||
| package it.gov.pagopa.payhub.activities.service.classifications; | |||
There was a problem hiding this comment.
use nested package to group them
Suggested change
| package it.gov.pagopa.payhub.activities.service.classifications; | |
| package it.gov.pagopa.payhub.activities.service.classifications.trclassifiers; |
| * and determine a classification label ({@link ClassificationsEnum}). Each implementation | ||
| * should handle a particular condition or set of conditions to decide the appropriate label. | ||
| */ | ||
| public interface LabelClassifier { |
There was a problem hiding this comment.
Suggested change
| public interface LabelClassifier { | |
| public interface TransferClassifier { |
| public class IufNoTesClassifier implements LabelClassifier { | ||
|
|
||
| @Override | ||
| public ClassificationsEnum define(TransferDTO transferDTO, PaymentsReportingDTO paymentsReportingDTO, TreasuryDTO treasuryDTO) { |
There was a problem hiding this comment.
can we rename the enum into TransferClassificationEnum?
There was a problem hiding this comment.
This Enum is not only used for Transfers
| LabelClassifier classifier = new RtIufTesClassifier(); | ||
|
|
||
| @Test | ||
| void whenDefineThenSuccess() { |
There was a problem hiding this comment.
Suggested change
| void whenDefineThenSuccess() { | |
| void givenMatchedConditionWhenDefineThenSuccess() { |
| } | ||
|
|
||
| @Test | ||
| void whenDefineThenReturnNull() { |
There was a problem hiding this comment.
Suggested change
| void whenDefineThenReturnNull() { | |
| void givenUnmatchedConditionWhenDefineThenReturnNull() { |
| } | ||
|
|
||
| @Test | ||
| void whenDefineThenReturnNull() { |
There was a problem hiding this comment.
you should test also unmatched amounts
Suggested change
| void whenDefineThenReturnNull() { | |
| void givenUnmatchedConditionWhenDefineThenReturnNull() { |
| LabelClassifier classifier = new RtTesClassifier(); | ||
|
|
||
| @Test | ||
| void whenDefineThenSuccess() { |
There was a problem hiding this comment.
Suggested change
| void whenDefineThenSuccess() { | |
| void givenMatchedConditionWhenDefineThenSuccess() { |
| } | ||
|
|
||
| @Test | ||
| void whenDefineThenReturnNull() { |
There was a problem hiding this comment.
Suggested change
| void whenDefineThenReturnNull() { | |
| void givenUnmatchedConditionWhenDefineThenReturnNull() { |
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.
Description
List of Changes
Motivation and Context
How Has This Been Tested?
Types of changes
Checklist: