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

MOSIP-23551: Added validators for DocType and DocCat code in valid document entity #937

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

balaji-alluru
Copy link
Contributor

@AttributeOverride(name = "docCategoryCode", column = @Column(name = "doccat_code", nullable = false, length = 36)) })

@Column(name = "doctyp_code", nullable = false, length = 36)
@DocTypeCode(message = "docType is Invalid")
Copy link
Contributor

Choose a reason for hiding this comment

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

It is always better to set error_code in the message instead of message directly.

private String docTypeCode;

@Id
@Column(name = "doccat_code", nullable = false, length = 36)
@DocCatCode(message = "docCategory is Invalid")
Copy link
Contributor

Choose a reason for hiding this comment

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

It is always better to set error_code in the message instead of message directly.

@Retention(RetentionPolicy.RUNTIME)
public @interface DocCatCode {

String message() default "docCategory is Invalid";
Copy link
Contributor

Choose a reason for hiding this comment

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

It is always better to set error_code in the message instead of message directly.


public class DocCatCodeValidator implements ConstraintValidator<DocCatCode, String> {

private List<String> docCatCode;
Copy link
Contributor

Choose a reason for hiding this comment

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

Its a list then why is the variable name singular?

}

if(null != value && !value.isEmpty()) {
return docCatCode.contains(value);
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens when a new document category is added? how and when the docCatCode list is refreshed?

@Retention(RetentionPolicy.RUNTIME)
public @interface DocTypeCode {

String message() default "docType is Invalid";
Copy link
Contributor

Choose a reason for hiding this comment

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

It is always better to set error_code in the message instead of message directly.


public class DocTypeCodeValidator implements ConstraintValidator<DocTypeCode, String> {

private List<String> docTypeCode;
Copy link
Contributor

Choose a reason for hiding this comment

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

Its a list then why is the variable name singular?

}

if(null != value && !value.isEmpty()) {
return docTypeCode.contains(value);
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens when a new document category is added? how and when the docCatCode list is refreshed?

@@ -113,7 +113,7 @@ public void setUp() throws Exception {
@Test
@WithUserDetails(value = "zonal-admin")
public void t002lostRidTest() throws Exception {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's rename the test method name based on the agreed convention
methodNameUnderTest_scenarioName_ Result

Copy link
Contributor

@ase-101 ase-101 left a comment

Choose a reason for hiding this comment

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

Please check comments

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.

2 participants