FLW-713 Remove All File Upload Options #350
Merged
Conversation
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
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
📝 WalkthroughWalkthroughThis pull request introduces JWT token-based authentication for structured form retrieval, enabling state-specific field filtering. It adds persistent fields to track form field editability and state assignment, expands translation support with Assamese language, and creates a new JPA entity class for user-service-role mappings with corresponding repository access. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Controller as DynamicFormController
participant Service as FormMasterServiceImpl
participant JwtUtil
participant UserRepo as UserServiceRoleRepo
participant DB as Database
Client->>Controller: GET /form/{formId}?lang=en<br/>(Header: jwttoken)
activate Controller
Controller->>Service: getStructuredFormByFormId(formId, lang, token)
activate Service
Service->>JwtUtil: Extract stateId from token
activate JwtUtil
JwtUtil-->>Service: stateId
deactivate JwtUtil
Service->>UserRepo: findByUserName(userName from token)
activate UserRepo
UserRepo->>DB: Query user roles
DB-->>UserRepo: UserServiceRole list
deactivate UserRepo
UserRepo-->>Service: User roles
Service->>DB: Retrieve form by formId
DB-->>Service: Form & Fields
Service->>Service: Filter fields by stateId<br/>Apply language translations<br/>Parse options/validations/conditionals
Service-->>Controller: FormResponseDTO
deactivate Service
Controller-->>Client: ApiResponse with form data
deactivate Controller
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In `@src/main/java/com/iemr/common/data/users/UserServiceRole.java`:
- Line 10: Change the primary key field usrMappingId in UserServiceRole from
primitive int to the wrapper type Integer and update its accessor methods
(getUsrMappingId and setUsrMappingId) accordingly so the ID can be null for
unsaved entities; locate the usrMappingId declaration and the corresponding
getter/setter in class UserServiceRole and replace the type usages (including
any annotations or JPA mappings tied to usrMappingId) from int to Integer.
- Line 30: Rename the misspelled field userServciceRoleDeleted to
userServiceRoleDeleted in the UserServiceRole class and update its getter/setter
names accordingly (e.g., getUserServiceRoleDeleted/setUserServiceRoleDeleted);
also update all usages/references (constructors, equals/hashCode, JSON mappings,
DTOs, tests) to the new name and, if a `@Column` annotation or other persistence
mapping relies on the old name, add or update
`@Column`(name="userServcice_role_deleted") (or the exact DB column name) to
preserve the existing database mapping.
- Around line 364-368: The class UserServiceRole overrides hashCode() but has no
corresponding equals(), and hashCode() currently uses many mutable fields; add
an equals(Object o) implementation in UserServiceRole that compares the same
identity field(s) used for equality (e.g., usrMappingId or a stable identifier
such as userId/usrMappingId) and update hashCode() to compute only from that
stable identifier; ensure equals handles nulls and type checks (instanceof), and
keep both methods consistent so JPA entity identity is based on the chosen
identifier field(s) only.
- Around line 6-8: Update UserServiceRole to use Integer for the primary key
field (replace primitive int id) and implement equals(Object) to match the
existing hashCode contract; refactor hashCode() so it relies only on the
identifier (id) rather than all mutable fields, and make equals(Object) compare
instances by id (handling null/unsaved ids properly). Rename the misspelled
field userServciceRoleDeleted → userServiceRoleDeleted and convert blockid and
villageid to camelCase (blockId, villageId) throughout the class. Also review
the `@GeneratedValue`(strategy = GenerationType.IDENTITY) usage on the id field in
light of the v_userservicerolemapping view (ensure the DB supports generated ids
or remove the generator if the entity is read-only). Optionally consider
converting to Lombok (`@Data`) with field-based JPA annotations for consistency
with Translation and FormField after these fixes.
In
`@src/main/java/com/iemr/common/service/dynamicForm/FormMasterServiceImpl.java`:
- Around line 215-216: The line in FormMasterServiceImpl that does
singleSection.setSectionTitle(singleSection.getSectionTitle()) is a no-op and
leaves the DTO title null; replace it to assign a real title (for example use a
local sectionTitle variable if one exists, or derive it from the grouped fields
like the first field's getSectionTitle(), or a form/getSectionTitle method) so
GroupedFieldResponseDTO.singleSection receives a meaningful section title rather
than resetting itself.
- Around line 119-127: The code risks NPE and IOBE when accessing token and the
userServiceRole list: ensure token is not null before calling token.isEmpty()
(use token != null && !token.isEmpty()), obtain username via
jwtUtil.getUsernameFromToken only when token is valid, then check the returned
list from userServiceRoleRepo.findByUserName is not null and not empty before
accessing userServiceRole.get(0); only then assign stateId and call logger.info
(use clear message like "StateId: "+stateId). Also consider handling the else
cases (leave stateId as 0 or log a warning) in the methods/classes: token,
jwtUtil.getUsernameFromToken, userServiceRoleRepo.findByUserName,
userServiceRole.get(0).
- Around line 226-230: The catch in FormMasterServiceImpl currently logs the
exception and returns null, which hides failures; instead modify the catch in
the method containing that try/catch to either re-throw a runtime (or service)
exception (e.g., throw new RuntimeException("Failed to <operation> in
FormMasterServiceImpl", e)) or return a well-defined error DTO/result object so
callers can detect failure; ensure you reference the original exception (e) when
re-throwing or wrapping so the stack trace is preserved and update any
callers/controllers to handle the thrown exception or error result accordingly.
- Around line 137-138: In FormMasterServiceImpl where fieldDtos is built,
replace the unsafe Integer comparison formField.getStateCode()==0 with a
null-safe check using either unboxing (formField.getStateCode() != null &&
formField.getStateCode().intValue() == 0) or equals against an Integer constant
(Integer.valueOf(0).equals(formField.getStateCode())), and keep the existing
comparison to finalStateId similarly null-safe (finalStateId ==
formField.getStateCode().intValue() or
formField.getStateCode().equals(finalStateId)); ensure you reference the
fields.stream(...) filter that uses formField.getStateCode to locate and update
the check.
🧹 Nitpick comments (7)
src/main/java/com/iemr/common/service/users/UserServiceRoleRepo.java (1)
1-17: Package placement and unused imports.
- Unused imports:
@Queryand@Param(lines 5-6) are imported but never used.- Package naming: This repository is placed in
com.iemr.common.service.usersbut repositories are typically placed in arepositorypackage (e.g.,com.iemr.common.repository.users).♻️ Proposed fix
package com.iemr.common.service.users; import com.iemr.common.data.users.UserServiceRole; import org.springframework.data.jpa.repository.JpaRepository; -import org.springframework.data.jpa.repository.Query; -import org.springframework.data.repository.query.Param; import org.springframework.stereotype.Repository; import java.util.List;src/main/java/com/iemr/common/controller/dynamicForm/DynamicFormController.java (2)
10-10: Remove unused import.The
org.springframework.data.jpa.repository.Queryimport is not used in this controller.
86-89: Consider token handling and header naming.
- Breaking change: The
jwttokenheader is now required (norequired = false). This will break existing clients that don't provide this header.- Header naming convention: Consider using standard header naming like
Authorizationwith aBearerprefix, which is the OAuth 2.0 convention.If backward compatibility is needed, consider making the token optional with graceful fallback:
♻️ Example with optional token
`@GetMapping`(value = "form/{formId}/fields") -public ResponseEntity<ApiResponse<?>> getStructuredForm(`@PathVariable` String formId, `@RequestParam`(name = "lang", defaultValue = "en") String lang,`@RequestHeader`(value = "jwttoken") String token) { +public ResponseEntity<ApiResponse<?>> getStructuredForm(`@PathVariable` String formId, `@RequestParam`(name = "lang", defaultValue = "en") String lang, `@RequestHeader`(value = "Authorization", required = false) String token) {src/main/java/com/iemr/common/service/dynamicForm/FormMasterServiceImpl.java (2)
26-26: Remove unused import.
javax.persistence.criteria.CriteriaBuilderis imported but not used.
203-206: Use logger instead ofSystem.err.println.Line 205 uses
System.err.printlnfor error logging. The class already has a logger configured—use it consistently.♻️ Proposed fix
} catch (Exception e) { - - System.err.println("JSON Parsing Error in field: " + field.getFieldId()); + logger.error("JSON Parsing Error in field: {}", field.getFieldId(), e); throw new RuntimeException("Failed to parse JSON for field: " + field.getFieldId(), e); }src/main/java/com/iemr/common/data/users/UserServiceRole.java (2)
37-40: Inconsistent field naming convention.Fields
blockid,blockname,villageid,villagenameuse lowercase while other fields likeworkingDistrictId,workingLocationIduse camelCase. For consistency and Java naming conventions, consider renaming these fields toblockId,blockName,villageId,villageName.If the database column names require lowercase, keep the
@Column(name = "...")annotation to preserve the mapping.
1-41: Consider using Lombok to reduce boilerplate.This entity has ~32 fields with manual getters/setters spanning 300+ lines. Other entities in this codebase (e.g.,
Translation.java,FormField.java) use Lombok's@Dataannotation with field-based JPA annotations.Adopting Lombok would significantly reduce boilerplate and improve maintainability.
♻️ Example refactored structure using Lombok
package com.iemr.common.data.users; import jakarta.persistence.*; import lombok.Data; import java.util.Objects; `@Entity` `@Table`(name = "v_userservicerolemapping", schema = "db_iemr") `@Data` public class UserServiceRole { `@Id` `@GeneratedValue`(strategy = GenerationType.IDENTITY) `@Column`(name = "USRMappingID") private Integer usrMappingId; `@Column`(name = "UserID") private Integer userId; `@Column`(name = "Name") private String name; `@Column`(name = "UserName") private String userName; // ... remaining fields with `@Column` annotations ... // Override equals/hashCode for proper JPA entity identity `@Override` public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; UserServiceRole that = (UserServiceRole) o; return usrMappingId != null && Objects.equals(usrMappingId, that.usrMappingId); } `@Override` public int hashCode() { return Objects.hash(usrMappingId); } }
Comment on lines
+6
to
+8
| @Entity | ||
| @Table(name = "v_userservicerolemapping", schema = "db_iemr") | ||
| public class UserServiceRole { |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, read the complete UserServiceRole.java file
cat -n src/main/java/com/iemr/common/data/users/UserServiceRole.javaRepository: PSMRI/Common-API
Length of output: 11714
🏁 Script executed:
# Check if Translation.java exists and uses Lombok
fd Translation.java --type fRepository: PSMRI/Common-API
Length of output: 123
🏁 Script executed:
# Check if FormField.java exists and uses Lombok
fd FormField.java --type fRepository: PSMRI/Common-API
Length of output: 122
🏁 Script executed:
# Search for database migration files or schema definitions that mention v_userservicerolemapping
find . -type f \( -name "*.sql" -o -name "*.yml" -o -name "*.yaml" \) -exec grep -l "v_userservicerolemapping" {} \;Repository: PSMRI/Common-API
Length of output: 42
🏁 Script executed:
# Check Translation.java for Lombok annotations
head -30 src/main/java/com/iemr/common/data/translation/Translation.javaRepository: PSMRI/Common-API
Length of output: 679
🏁 Script executed:
# Check FormField.java for Lombok annotations
head -30 src/main/java/com/iemr/common/data/dynamic_from/FormField.javaRepository: PSMRI/Common-API
Length of output: 743
🏁 Script executed:
# Search for any database schema files that might define v_userservicerolemapping
find . -type f \( -name "*.sql" -o -name "*.yml" -o -name "*.yaml" \) | head -20Repository: PSMRI/Common-API
Length of output: 279
Implement equals() method to match overridden hashCode(). The class overrides hashCode() (line 365) but lacks an equals() method, violating the Java Object contract. This will cause incorrect behavior in collections and equality comparisons.
Additionally:
- Use
Integerinstead of primitiveintfor the ID field (lines 10, 56). Primitive types cannot distinguish unsaved/null entities in JPA. - Fix
hashCode()to use only the ID, not all 32 mutable fields. JPA entities should basehashCode()andequals()solely on the identifier to comply with best practices. - Verify
@GeneratedValue(IDENTITY)compatibility: The table namev_userservicerolemapping(withv_prefix) suggests a database view, which is typically read-only and may not support auto-generated IDs. - Fix typo in field name
userServciceRoleDeleted(lines 30, 256) →userServiceRoleDeleted. - Standardize field naming:
blockid,villageidshould follow camelCase convention used elsewhere (blockId,villageId). - Consider using Lombok
@Dataand field-based annotations for consistency withTranslation.javaandFormField.java.
🤖 Prompt for AI Agents
In `@src/main/java/com/iemr/common/data/users/UserServiceRole.java` around lines 6
- 8, Update UserServiceRole to use Integer for the primary key field (replace
primitive int id) and implement equals(Object) to match the existing hashCode
contract; refactor hashCode() so it relies only on the identifier (id) rather
than all mutable fields, and make equals(Object) compare instances by id
(handling null/unsaved ids properly). Rename the misspelled field
userServciceRoleDeleted → userServiceRoleDeleted and convert blockid and
villageid to camelCase (blockId, villageId) throughout the class. Also review
the `@GeneratedValue`(strategy = GenerationType.IDENTITY) usage on the id field in
light of the v_userservicerolemapping view (ensure the DB supports generated ids
or remove the generator if the entity is read-only). Optionally consider
converting to Lombok (`@Data`) with field-based JPA annotations for consistency
with Translation and FormField after these fixes.
| @Table(name = "v_userservicerolemapping", schema = "db_iemr") | ||
| public class UserServiceRole { | ||
| private Integer userId; | ||
| private int usrMappingId; |
Contributor
There was a problem hiding this comment.
Use Integer wrapper type instead of primitive int for the primary key.
The primary key field usrMappingId is declared as primitive int (Line 10), which defaults to 0. This makes it impossible to distinguish between an unsaved entity and one with ID=0. JPA best practice is to use wrapper types (Integer) for ID fields so that null can indicate an unpersisted entity.
🔧 Proposed fix
- private int usrMappingId;
+ private Integer usrMappingId;And update the getter/setter:
- public int getUsrMappingId() {
+ public Integer getUsrMappingId() {
return usrMappingId;
}
- public void setUsrMappingId(int usrMappingId) {
+ public void setUsrMappingId(Integer usrMappingId) {
this.usrMappingId = usrMappingId;
}Also applies to: 52-62
🤖 Prompt for AI Agents
In `@src/main/java/com/iemr/common/data/users/UserServiceRole.java` at line 10,
Change the primary key field usrMappingId in UserServiceRole from primitive int
to the wrapper type Integer and update its accessor methods (getUsrMappingId and
setUsrMappingId) accordingly so the ID can be null for unsaved entities; locate
the usrMappingId declaration and the corresponding getter/setter in class
UserServiceRole and replace the type usages (including any annotations or JPA
mappings tied to usrMappingId) from int to Integer.
Comment on lines
119
to
127
| int stateId =0 ; | ||
| try { | ||
| if(!token.isEmpty()){ | ||
| List<UserServiceRole> userServiceRole= userServiceRoleRepo.findByUserName(jwtUtil.getUsernameFromToken(token)); | ||
| if(userServiceRole!=null){ | ||
| stateId = userServiceRole.get(0).getStateId(); | ||
| logger.info("State:Id"+stateId); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Potential NullPointerException and IndexOutOfBoundsException.
Several null/empty checks are missing:
- Line 121:
token.isEmpty()will throwNullPointerExceptioniftokenisnull. - Line 124:
userServiceRole.get(0)will throwIndexOutOfBoundsExceptionif the list is empty.
🐛 Proposed fix with proper null and empty checks
- int stateId =0 ;
+ int stateId = 0;
try {
- if(!token.isEmpty()){
- List<UserServiceRole> userServiceRole= userServiceRoleRepo.findByUserName(jwtUtil.getUsernameFromToken(token));
- if(userServiceRole!=null){
+ if (token != null && !token.isEmpty()) {
+ String username = jwtUtil.getUsernameFromToken(token);
+ List<UserServiceRole> userServiceRole = userServiceRoleRepo.findByUserName(username);
+ if (userServiceRole != null && !userServiceRole.isEmpty()) {
stateId = userServiceRole.get(0).getStateId();
- logger.info("State:Id"+stateId);
+ logger.info("State:Id {}", stateId);
}
}📝 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
| int stateId =0 ; | |
| try { | |
| if(!token.isEmpty()){ | |
| List<UserServiceRole> userServiceRole= userServiceRoleRepo.findByUserName(jwtUtil.getUsernameFromToken(token)); | |
| if(userServiceRole!=null){ | |
| stateId = userServiceRole.get(0).getStateId(); | |
| logger.info("State:Id"+stateId); | |
| } | |
| } | |
| int stateId = 0; | |
| try { | |
| if (token != null && !token.isEmpty()) { | |
| String username = jwtUtil.getUsernameFromToken(token); | |
| List<UserServiceRole> userServiceRole = userServiceRoleRepo.findByUserName(username); | |
| if (userServiceRole != null && !userServiceRole.isEmpty()) { | |
| stateId = userServiceRole.get(0).getStateId(); | |
| logger.info("State:Id {}", stateId); | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@src/main/java/com/iemr/common/service/dynamicForm/FormMasterServiceImpl.java`
around lines 119 - 127, The code risks NPE and IOBE when accessing token and the
userServiceRole list: ensure token is not null before calling token.isEmpty()
(use token != null && !token.isEmpty()), obtain username via
jwtUtil.getUsernameFromToken only when token is valid, then check the returned
list from userServiceRoleRepo.findByUserName is not null and not empty before
accessing userServiceRole.get(0); only then assign stateId and call logger.info
(use clear message like "StateId: "+stateId). Also consider handling the else
cases (leave stateId as 0 or log a warning) in the methods/classes: token,
jwtUtil.getUsernameFromToken, userServiceRoleRepo.findByUserName,
userServiceRole.get(0).
src/main/java/com/iemr/common/service/dynamicForm/FormMasterServiceImpl.java
Outdated
Show resolved
Hide resolved
src/main/java/com/iemr/common/service/dynamicForm/FormMasterServiceImpl.java
Outdated
Show resolved
Hide resolved
src/main/java/com/iemr/common/service/dynamicForm/FormMasterServiceImpl.java
Outdated
Show resolved
Hide resolved
vanitha1822
requested changes
Jan 29, 2026
Member
vanitha1822
left a comment
vanitha1822
left a comment
There was a problem hiding this comment.
Check the coderabbit comments please.
|
drtechie
approved these changes
Jan 30, 2026
vanitha1822
approved these changes
Jan 30, 2026
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
JIRA ID: FLW-713
Remove All File Upload Options
✅ Type of Change
ℹ️ Additional Information
Please describe how the changes were tested, and include any relevant screenshots, logs, or other information that provides additional context.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.