Fix #968: validate Last Name mandatory field in CSV bulk upload#969
Open
areychana wants to merge 2 commits intoavniproject:masterfrom
Open
Fix #968: validate Last Name mandatory field in CSV bulk upload#969areychana wants to merge 2 commits intoavniproject:masterfrom
areychana wants to merge 2 commits intoavniproject:masterfrom
Conversation
📝 WalkthroughWalkthroughThe change conditions last-name assignment in SubjectWriter to only set last names for person-type subjects via a new private helper that validates non-empty values and records mandatory-field errors. Tests were updated to expect and exercise the new missing-last-name validation. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
avni-server-api/src/main/java/org/avni/server/importer/batch/csv/writer/SubjectWriter.java (1)
212-219: Fix indentation inside theifblock.The body of the
ifstatement (lines 215-217) is not indented correctly, making it inconsistent with thesetFirstNamemethod and harder to read.🔧 Proposed fix for indentation
private static void setLastName(Row row, Individual individual, List<String> allErrorMsgs) { String lastName = row.get(SubjectHeadersCreator.lastName); if (!StringUtils.hasText(lastName)) { - allErrorMsgs.add(String.format("Value required for mandatory field: '%s'", SubjectHeadersCreator.lastName)); - return; + allErrorMsgs.add(String.format("Value required for mandatory field: '%s'", SubjectHeadersCreator.lastName)); + return; } individual.setLastName(lastName); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@avni-server-api/src/main/java/org/avni/server/importer/batch/csv/writer/SubjectWriter.java` around lines 212 - 219, The if-block in setLastName is misindented; reformat the block so the statements inside the if are indented consistently with setFirstName: the lines that add the error to allErrorMsgs and the return should be indented one level inside the if (i.e. align with the opening brace), leaving individual.setLastName(lastName) at the method level; locate the method setLastName and adjust indentation for the lines referencing SubjectHeadersCreator.lastName, allErrorMsgs.add(...), and return.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@avni-server-api/src/main/java/org/avni/server/importer/batch/csv/writer/SubjectWriter.java`:
- Around line 212-219: The if-block in setLastName is misindented; reformat the
block so the statements inside the if are indented consistently with
setFirstName: the lines that add the error to allErrorMsgs and the return should
be indented one level inside the if (i.e. align with the opening brace), leaving
individual.setLastName(lastName) at the method level; locate the method
setLastName and adjust indentation for the lines referencing
SubjectHeadersCreator.lastName, allErrorMsgs.add(...), and return.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5b1e8a63-af94-4fcb-97b0-553f96c272a8
📒 Files selected for processing (2)
avni-server-api/src/main/java/org/avni/server/importer/batch/csv/writer/SubjectWriter.javaavni-server-api/src/test/java/org/avni/server/importer/batch/csv/writer/SubjectWriterIntegrationTest.java
There was a problem hiding this comment.
🧹 Nitpick comments (1)
avni-server-api/src/main/java/org/avni/server/importer/batch/csv/writer/SubjectWriter.java (1)
212-219: Implementation correctly mirrorssetFirstNamevalidation pattern.The validation logic is consistent with other mandatory field checks in this class. The early return on empty value and error message format are appropriate.
Minor style nit: Consider adding a blank line before this method to match the spacing convention between other methods in this file.
,
🧹 Optional: Add blank line for consistency
individual.setGender(gender); } + private static void setLastName(Row row, Individual individual, List<String> allErrorMsgs) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@avni-server-api/src/main/java/org/avni/server/importer/batch/csv/writer/SubjectWriter.java` around lines 212 - 219, Add a blank line before the private static method setLastName(Row row, Individual individual, List<String> allErrorMsgs) so its spacing matches the other methods in the class (maintain existing implementation and validation using SubjectHeadersCreator.lastName and allErrorMsgs).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@avni-server-api/src/main/java/org/avni/server/importer/batch/csv/writer/SubjectWriter.java`:
- Around line 212-219: Add a blank line before the private static method
setLastName(Row row, Individual individual, List<String> allErrorMsgs) so its
spacing matches the other methods in the class (maintain existing implementation
and validation using SubjectHeadersCreator.lastName and allErrorMsgs).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fda43d63-8c36-488d-9e56-3771f0a2211c
📒 Files selected for processing (1)
avni-server-api/src/main/java/org/avni/server/importer/batch/csv/writer/SubjectWriter.java
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
problem
when uploading a CSV file with the Last Name field empty (marked as mandatory), no validation error was shown and the record was created with an empty Last Name.
#968 - fixes
root cause
in SubjectWriter.java, first name and gender had explicit mandatory value checks but last name was being set directly without any validation.
##fix
Added a
setLastName()method following the same pattern assetFirstName(), which validates the field is non-empty before setting it on the individual.tests
missingMandatoryCoreValuesto include Last Name errormissingLastNameto verify the fixnote
avni-server-data:test is failing but this is a pre-existing issue unrelated to these changes.
Summary by CodeRabbit