#1712 | [DMP 2025] Signature capture #915
Conversation
|
Hey @ShivangMishra , even web-app requires some additions. Please go through the webapp, do the needful and make a PR in the web app as well |
ShivangMishra
force-pushed
the
dmp-2025-signature-capture
branch
from
23a9a87 to
4a4acbd
Compare
|
Warning Rate limit exceeded@ShivangMishra has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 3 minutes and 42 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughAdds a new ConceptDataType enum value: Signature. Updates media-type handling to treat Signature like other media (Image/ImageV2/Video) in ObservationCreator and validation to route Signature through validateImageValue in EnhancedValidationService. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CSV as CSV Import
participant OC as ObservationCreator
participant Media as handleMediaValue
note over OC: getObservationValue(dataType, value)
CSV->>OC: Provide value with dataType = Signature
alt dataType is media (Image/ImageV2/Video/Signature)
OC->>Media: handleMediaValue(value)
Media-->>OC: MediaObservation
else other data types
OC-->>CSV: Other handling path
end
OC-->>CSV: Observation created
sequenceDiagram
autonumber
participant Client as Client
participant EVS as EnhancedValidationService
participant VImg as validateImageValue
note over EVS: validateAnswer(concept.dataType, value)
Client->>EVS: value with dataType = Signature
alt Image/ImageV2/Signature
EVS->>VImg: validateImageValue(value)
VImg-->>EVS: Validation result
else other data types
EVS-->>Client: Other validation path
end
EVS-->>Client: Validation outcome
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
avni-server-api/src/main/java/org/avni/server/importer/batch/csv/creator/ObservationCreator.java (1)
224-226: Routing Signature through handleMediaValue is consistent.Good addition. Consider consolidating all S3-backed media (File/Audio) under the same handler for parity, if applicable.
If desired:
- case Image, ImageV2, Video, Signature: + case Image, ImageV2, Video, Signature, File, Audio: return handleMediaValue(formElement, answerValue, errorMsgs, oldValue);Does
s3Service.getObservationValueForUploadaccept signature payloads/keys identically to images?avni-server-api/src/main/java/org/avni/server/service/EnhancedValidationService.java (1)
185-187: Signature validated via image path: OK; consider broader media parity.If video/file/audio should be host-checked too, extend this case or introduce
validateMediaValue.- case Image, ImageV2, Signature: - validateImageValue(question, value, errorMessages); + case Image, ImageV2, Signature/*, Video, File, Audio */: + validateImageValue(question, value, errorMessages); // or validateMediaValue(...) break;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
avni-server-api/src/main/java/org/avni/server/importer/batch/csv/creator/ObservationCreator.java(1 hunks)avni-server-api/src/main/java/org/avni/server/service/EnhancedValidationService.java(8 hunks)avni-server-data/src/main/java/org/avni/server/domain/ConceptDataType.java(1 hunks)
🔇 Additional comments (6)
avni-server-data/src/main/java/org/avni/server/domain/ConceptDataType.java (2)
33-33: Adding Signature to mediaDataTypes aligns isMedia() behavior.LGTM. Also confirm we don’t need multi-select signatures (not included in multiSelectTypes).
If multi-signature capture is a requirement, we’ll need to add
SignaturetomultiSelectTypesand ensure UI/importers support it. Do you expect multiple signatures per question?
28-29: No @Enumerated(EnumType.ORDINAL) usage found; addingSignatureis safe.
The search didn’t uncover anyEnumType.ORDINALpersistence forConceptDataType, so introducing a new constant won’t shift stored values.avni-server-api/src/main/java/org/avni/server/service/EnhancedValidationService.java (4)
327-334: Safer URL parsing; host/protocol check is good—validate endpoint assumptions.Using
new URI(...).toURL()is safer. Ensure the comparison against an upload URL (generateMediaUploadUrl) matches the domain used for stored media (e.g., CloudFront vs S3) to avoid false negatives.If downloads use a CDN host, compare against that instead of the upload host, or relax validation to allow configured media hosts.
281-291: Typed, null-safe location validation improvements LGTM.Use of instanceof with safe cast and stream pipeline reads well.
302-308: Clearer error messages for location key-values.Improves debuggability without behavior changes.
388-390: Explicit error for invalid qGroup item types.Good guardrail for malformed payloads.
ShivangMishra
force-pushed
the
dmp-2025-signature-capture
branch
from
ae94c9c to
7f5c31c
Compare
|
This closes avniproject/avni-product#1712 |
ombhardwajj
force-pushed
the
master
branch
3 times, most recently
from
a3cb55d to
d1d6215
Compare
2 participants
related to avniproject/avni-client#1678
Summary by CodeRabbit