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

CU-86b1g1mye_SRU2024_Generar-un-enum-de-FieldNames-para-validar-que-e… #194

Conversation

ptorres-prowide
Copy link
Contributor

@ptorres-prowide ptorres-prowide commented Aug 22, 2024

…l-Field-exista

Summary by CodeRabbit

  • New Features

    • Introduced the FieldEnum, providing a centralized reference for field names in the SWIFT messaging standard.
    • Added the SRU2024FieldEnum, encompassing a comprehensive list of field identifiers relevant to the 2024 SWIFT messaging standard.
    • New methods for enhanced code usability: fieldName() for retrieving human-readable field names and fromFieldName(String fieldName) for converting string representations to enum types.
  • Tests

    • Updated unit tests for FieldEnum to validate functionality and ensure robust handling of valid, invalid, and edge case inputs.

Copy link
Contributor

coderabbitai bot commented Aug 22, 2024

Caution

Review failed

The pull request is closed.

📝 Walkthrough
📝 Walkthrough

Walkthrough

The recent updates introduce the FieldEnum and SRU2024FieldEnum, new enumerations for field identifiers in the SWIFT messaging standard. These enums include constants for various field codes and provide methods for retrieving field names and converting string representations to enum instances. Additionally, a test suite for FieldEnum has been implemented to validate its functionality.

Changes

File Change Summary
CHANGELOG.md Added version entry for 9.5.2, highlighting the introduction of FieldEnum.
src/generated/java/com/prowidesoftware/.../FieldEnum.java Introduced FieldEnum enum with constants for field names, including methods fieldName() and fromFieldName(String fieldName).
src/generated/java/com/prowidesoftware/.../SRU2024FieldEnum.java Introduced SRU2024FieldEnum enum with constants for field codes, including methods getCode() and fromCode(String code).
src/test/java/com/prowidesoftware/.../FieldEnumTest.java Added unit tests for FieldEnum, validating methods related to field names and handling various input scenarios.

Possibly related PRs


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

CHANGELOG.md Outdated
@@ -1,5 +1,8 @@
# Prowide Core - CHANGELOG

#### 9.5.2 - SNAPSHOT
Copy link
Contributor

Choose a reason for hiding this comment

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

Adjust the heading level for version entry.

The heading level for version 9.5.2 should be consistent with the rest of the document. Change it to level 2.

Apply this diff to fix the heading level:

-#### 9.5.2 - SNAPSHOT
+## 9.5.2 - SNAPSHOT
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#### 9.5.2 - SNAPSHOT
## 9.5.2 - SNAPSHOT
Tools
Markdownlint

3-3: Expected: h2; Actual: h4
Heading levels should only increment by one level at a time

(MD001, heading-increment)


3-3: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)

CHANGELOG.md Outdated
@@ -1,5 +1,8 @@
# Prowide Core - CHANGELOG

#### 9.5.2 - SNAPSHOT
* Added new SRU2023FieldEnum with all the available field names
Copy link
Contributor

Choose a reason for hiding this comment

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

Ensure proper indentation and blank lines for list items.

The bullet point should be indented correctly and surrounded by blank lines for consistency.

Apply this diff to fix the indentation and add blank lines:

 ## 9.5.2 - SNAPSHOT

-  * Added new SRU2023FieldEnum with all the available field names
+* Added new SRU2023FieldEnum with all the available field names

 #### 9.5.1 - June 2024

Committable suggestion was skipped due to low confidence.

Tools
Markdownlint

4-4: Expected: 0; Actual: 2
Unordered list indentation

(MD007, ul-indent)


4-4: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)

@ptorres-prowide ptorres-prowide force-pushed the CU-86b1g1mye_SRU2024_Generar-un-enum-de-FieldNames-para-validar-que-el-Field-exista branch from cff5c0b to 902cfa5 Compare August 24, 2024 00:08
@zubri zubri force-pushed the CU-86b1g1mye_SRU2024_Generar-un-enum-de-FieldNames-para-validar-que-el-Field-exista branch from 3d38ada to b6c99c0 Compare September 30, 2024 21:46
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 7

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE

📥 Commits

Files that changed from the base of the PR and between 3d38ada and b6c99c0.

⛔ Files ignored due to path filters (1)
  • src/generated/java/com/prowidesoftware/swift/model/field/FieldEnum.java is excluded by !**/generated/**
📒 Files selected for processing (2)
  • CHANGELOG.md (1 hunks)
  • src/test/java/com/prowidesoftware/swift/model/field/FieldEnumTest.java (1 hunks)
🧰 Additional context used
🪛 Markdownlint
CHANGELOG.md

3-3: Expected: h2; Actual: h4
Heading levels should only increment by one level at a time

(MD001, heading-increment)


4-4: Expected: 0; Actual: 2
Unordered list indentation

(MD007, ul-indent)


3-3: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)


4-4: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)

🔇 Additional comments (1)
CHANGELOG.md (1)

4-4: LGTM! Clear and concise changelog entry.

The content of the changelog entry accurately describes the addition of the new FieldEnum feature. It provides sufficient information for users to understand the main change in this version.

🧰 Tools
🪛 Markdownlint

4-4: Expected: 0; Actual: 2
Unordered list indentation

(MD007, ul-indent)


4-4: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)

Comment on lines 9 to 15
@Test
void testfieldName() {
// Validate that fieldName returns the correct value
assertEquals("11A", FieldEnum.F11A.fieldName());
assertEquals("14P", FieldEnum.F14P.fieldName());
assertEquals("30K", FieldEnum.F30K.fieldName());
}
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Approve with suggestions for improvement

The testfieldName method provides basic coverage for the fieldName() method of FieldEnum. However, consider the following improvements:

  1. Rename the method to follow Java naming conventions: testFieldName().
  2. Consider adding more test cases to cover a wider range of enum values.
  3. You might want to add a test case for an enum value that doesn't follow the standard pattern (if any exist) to ensure robustness.

Comment on lines 17 to 23
@Test
void testfromFieldNameValid() {
// Validate that fromFieldName returns the correct enum when a valid code is provided
assertEquals(FieldEnum.F11A, FieldEnum.fromFieldName("11A"));
assertEquals(FieldEnum.F14R, FieldEnum.fromFieldName("14R"));
assertEquals(FieldEnum.F30K, FieldEnum.fromFieldName("30K"));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Approve with suggestions for improvement

The testfromFieldNameValid method provides basic coverage for the fromFieldName() method of FieldEnum with valid inputs. Consider the following improvements:

  1. Rename the method to follow Java naming conventions: testFromFieldNameValid().
  2. Consider adding more test cases to cover a wider range of valid field names.
  3. You might want to add a test case for a field name at the boundary of validity (e.g., the highest valid field number) to ensure comprehensive coverage.

Comment on lines 25 to 31
@Test
void testfromFieldNameInvalid() {
// Validate that fromFieldName returns null when an invalid code is provided
assertNull(FieldEnum.fromFieldName("930K"));
assertNull(FieldEnum.fromFieldName("ABC"));
assertNull(FieldEnum.fromFieldName(""));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Approve with suggestions for enhancement

The testfromFieldNameInvalid method provides good coverage for invalid inputs to the fromFieldName() method. Consider the following enhancements:

  1. Rename the method to follow Java naming conventions: testFromFieldNameInvalid().
  2. Add more test cases for invalid inputs, such as:
    • A null input
    • A very long string
    • A string with special characters
  3. Consider using a parameterized test to make it easier to add more invalid input scenarios.

Comment on lines 33 to 39
@Test
void testfromFieldNameEdgeCases() {
// Validate edge cases, such as lowercase inputs, to ensure the method is case-sensitive
assertNull(FieldEnum.fromFieldName("11a")); // should return null because "11a" is lowercase
assertNull(FieldEnum.fromFieldName(" 11A")); // should return null because of the leading space
assertNull(FieldEnum.fromFieldName("11A ")); // should return null because of the trailing space
}
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Approve with minor suggestions

The testfromFieldNameEdgeCases method provides excellent coverage for edge cases of the fromFieldName() method. The test cases and comments are clear and informative. Consider the following minor improvements:

  1. Rename the method to follow Java naming conventions: testFromFieldNameEdgeCases().
  2. Consider adding a test case for a mixed-case input (e.g., "11a") to explicitly verify case sensitivity.

Comment on lines 41 to 47
@Test
void testAllEnumValues() {
// Validate that all enum values have the correct field name
for (FieldEnum field : FieldEnum.values()) {
assertEquals(field.name().substring(1), field.fieldName());
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Approve with suggestions for robustness

The testAllEnumValues method provides excellent comprehensive coverage for all enum values. This ensures that the relationship between enum names and field names is consistent across the entire FieldEnum. Consider the following suggestions to enhance robustness:

  1. Add a check to ensure that FieldEnum.values() is not empty before the loop, to avoid a potential silent pass if the enum were empty.
  2. Consider adding an explicit assertion for the number of enum values, if it's expected to be stable.
  3. If there's a possibility of exceptions (e.g., if fieldName() could throw), consider adding appropriate exception handling or using assertDoesNotThrow.

Comment on lines 1 to 48
package com.prowidesoftware.swift.model.field;

import static org.junit.jupiter.api.Assertions.*;

import org.junit.jupiter.api.Test;

class FieldEnumTest {

@Test
void testfieldName() {
// Validate that fieldName returns the correct value
assertEquals("11A", FieldEnum.F11A.fieldName());
assertEquals("14P", FieldEnum.F14P.fieldName());
assertEquals("30K", FieldEnum.F30K.fieldName());
}

@Test
void testfromFieldNameValid() {
// Validate that fromFieldName returns the correct enum when a valid code is provided
assertEquals(FieldEnum.F11A, FieldEnum.fromFieldName("11A"));
assertEquals(FieldEnum.F14R, FieldEnum.fromFieldName("14R"));
assertEquals(FieldEnum.F30K, FieldEnum.fromFieldName("30K"));
}

@Test
void testfromFieldNameInvalid() {
// Validate that fromFieldName returns null when an invalid code is provided
assertNull(FieldEnum.fromFieldName("930K"));
assertNull(FieldEnum.fromFieldName("ABC"));
assertNull(FieldEnum.fromFieldName(""));
}

@Test
void testfromFieldNameEdgeCases() {
// Validate edge cases, such as lowercase inputs, to ensure the method is case-sensitive
assertNull(FieldEnum.fromFieldName("11a")); // should return null because "11a" is lowercase
assertNull(FieldEnum.fromFieldName(" 11A")); // should return null because of the leading space
assertNull(FieldEnum.fromFieldName("11A ")); // should return null because of the trailing space
}

@Test
void testAllEnumValues() {
// Validate that all enum values have the correct field name
for (FieldEnum field : FieldEnum.values()) {
assertEquals(field.name().substring(1), field.fieldName());
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Overall, well-structured and comprehensive test suite

This test class provides a robust set of unit tests for the FieldEnum class, covering the main functionalities, edge cases, and comprehensive enum value checking. The code is well-organized and easy to follow.

Some general suggestions for improvement:

  1. Consistently use camelCase for method names to follow Java naming conventions.
  2. Consider using parameterized tests for some methods to make it easier to add more test cases.
  3. Add comments explaining the purpose of each test method (similar to what's done in testfromFieldNameEdgeCases).
  4. If not already present elsewhere, consider adding integration tests that use these enums in the context of the larger system.

Great job on creating a thorough test suite!

CHANGELOG.md Outdated
Comment on lines 3 to 4
#### 9.5.2 - SNAPSHOT
* Added new FieldEnum with all the available field names
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Adjust formatting for consistency and readability.

Please make the following changes to improve the formatting of the changelog entry:

  1. Change the heading level from h4 to h2 for consistency with other version entries.
  2. Add a blank line before the heading.
  3. Remove the indentation for the bullet point.

Apply this diff to fix the formatting:

+
-#### 9.5.2 - SNAPSHOT
-  * Added new FieldEnum with all the available field names
+## 9.5.2 - SNAPSHOT
+* Added new FieldEnum with all the available field names
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#### 9.5.2 - SNAPSHOT
* Added new FieldEnum with all the available field names
## 9.5.2 - SNAPSHOT
* Added new FieldEnum with all the available field names
🧰 Tools
🪛 Markdownlint

3-3: Expected: h2; Actual: h4
Heading levels should only increment by one level at a time

(MD001, heading-increment)


4-4: Expected: 0; Actual: 2
Unordered list indentation

(MD007, ul-indent)


3-3: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)


4-4: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)

@zubri zubri merged commit bfe88e2 into SRU2024 Sep 30, 2024
1 check failed
@zubri zubri deleted the CU-86b1g1mye_SRU2024_Generar-un-enum-de-FieldNames-para-validar-que-el-Field-exista branch September 30, 2024 22:22
zubri added a commit that referenced this pull request Nov 19, 2024
* SRU2024: Updated MT messages and fields with SRU2024 definition
* yearly deprecation iteration
* fix 33Z components and javadoc typo
* added isPercentage helper method in field 37K + javadoc typo
* Restore deprecated method in MT210 class
* CU-86b1g1mye_SRU2024_Generar-un-enum-de-FieldNames-para-validar-que-e… (#194)
* CU-86b1g1mye_SRU2024_Generar-un-enum-de-FieldNames-para-validar-que-el-Field-exista
* CU-86b1g1mye_SRU2024_Generar-un-enum-de-FieldNames-para-validar-que-el-Field-exista
* CU-86b14j4e0_SRU2024_check-code-security-reports-at-GitHub-for-all-repos
* Fixed `getMUR` and `setMUR` in `SwiftMessage` to prioritize field 108 in block 4 over block 3 for system messages (category 0) (#211)
* CU-86b1uerqp_Generar-un-enum-de-MTs-para-validar-sus-secuencias-y-paths_SRU2024 (#209)
---------
Co-authored-by: zubri <[email protected]>
Co-authored-by: ecancrini <[email protected]>
Co-authored-by: ptorres-prowide <[email protected]>
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