feat: enhance coupon system and implement HTML-to-Docx export#181
feat: enhance coupon system and implement HTML-to-Docx export#181aniebietafia merged 1 commit intomainfrom
Conversation
- Coupon System: - Add support for duration-based expiry (validDurationValue/Unit). - Truncate validity dates to UTC seconds. - Update custom code length constraints (3-40 characters). - Docx Export: - Add 'type' parameter to support downloading original vs. edited text. - Implement HTML parsing in DocxExportServiceImpl to preserve formatting (headers, lists, bold/italic). - Auth: - Change logoutAllDevices return type to void. - Docs: Update Coupon System documentation formatting and links. Signed-off-by: aniebietafia <aniebietafia87@gmail.com>
📝 WalkthroughWalkthroughThis PR introduces duration-based coupon validity fields (validDurationValue and validDurationUnit), relaxes coupon code validation constraints (3–40 characters), implements HTML-to-Docx rendering with proper tag and formatting support, adds type-based OCR content export selection (original vs. edited), changes AuthService.logoutAllDevices return type from UnravelDocsResponse to void, and updates coupon documentation and tests accordingly. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Controller as DownloadOcrCResultController
participant Service as DownloadOcrServiceImpl
participant DocxService as DocxExportServiceImpl
participant HtmlParser as Jsoup
participant PoiLib as POI XWPFDocument
User->>Controller: GET /download?type=edited
Controller->>Service: downloadAsDocx(collectionId, documentId, "edited", userId)
Service->>Service: Fetch OCR data
alt type == "edited"
Service->>Service: Use editedContent
else
Service->>Service: Use extractedText
end
Service->>DocxService: generateDocxFromText(text)
alt isHtml(text)
DocxService->>HtmlParser: parse HTML
HtmlParser-->>DocxService: Document root element
DocxService->>DocxService: processElement (h1-h6, p, ul/ol)
DocxService->>DocxService: renderChildren & processInlineElement
else
DocxService->>DocxService: newline-based paragraph creation
end
DocxService->>PoiLib: Write paragraphs/headings/lists to XWPFDocument
PoiLib-->>DocxService: Generated DOCX bytes
DocxService-->>Service: DownloadableFile (DOCX bytes)
Service-->>Controller: ResponseEntity<InputStreamResource>
Controller-->>User: DOCX file download
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~55 minutes Possibly related PRs
Suggested Labels
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 |
github-actions
bot
added
documentation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| // Simplified numbering helpers - in a real app these would use a numbering.xml | ||
| // template | ||
| private java.math.BigInteger getBulletNumId(XWPFDocument document) { |
Check notice
Code scanning / CodeQL
Useless parameter Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 8 days ago
In general, a useless parameter should either be used in the method body (if it is logically needed but was omitted from the implementation) or removed from the method signature (and from all its call sites) if it truly serves no purpose. For private helper methods where we control all usages, the safest and cleanest fix is to remove the unused parameter.
Here, getBulletNumId does not use its XWPFDocument document parameter at all and simply returns a constant BigInteger. The most direct fix is to change its signature to private java.math.BigInteger getBulletNumId() and remove the parameter from the declaration. To preserve existing functionality, the body can remain unchanged aside from no longer accepting the parameter. However, we must also ensure that any calls to getBulletNumId(document) within DocxExportServiceImpl are updated to call getBulletNumId() with no arguments. The same applies to getDecimalNumId, which also has an unused document parameter and follows the same pattern; for consistency and to avoid a second similar issue, we should remove the parameter there as well and update its call sites.
Concretely, within src/main/java/com/extractor/unraveldocs/wordexport/impl/DocxExportServiceImpl.java, adjust the method declarations around lines 163–170 to remove the XWPFDocument document parameter from both getBulletNumId and getDecimalNumId. Then, in this same file, locate any invocations of these methods (for example, getBulletNumId(document) or getDecimalNumId(document) if they exist elsewhere in this class) and remove the argument so that they call the new parameterless versions. No new imports or additional method definitions are required; we are only modifying signatures and their usages.
| @@ -160,12 +160,12 @@ | ||
|
|
||
| // Simplified numbering helpers - in a real app these would use a numbering.xml | ||
| // template | ||
| private java.math.BigInteger getBulletNumId(XWPFDocument document) { | ||
| private java.math.BigInteger getBulletNumId() { | ||
| // This is a simplified approach. Ideally, you define numbering styles. | ||
| return java.math.BigInteger.valueOf(1); | ||
| } | ||
|
|
||
| private java.math.BigInteger getDecimalNumId(XWPFDocument document) { | ||
| private java.math.BigInteger getDecimalNumId() { | ||
| return java.math.BigInteger.valueOf(2); | ||
| } | ||
| } |
| return java.math.BigInteger.valueOf(1); | ||
| } | ||
|
|
||
| private java.math.BigInteger getDecimalNumId(XWPFDocument document) { |
Check notice
Code scanning / CodeQL
Useless parameter Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 8 days ago
In general, to fix a useless parameter you either (a) start using it meaningfully inside the method body, or (b) remove it from the method signature and from all call sites. Since getDecimalNumId does not use document at all and simply returns a constant BigInteger value, the minimal, behavior-preserving fix is to remove the XWPFDocument document parameter from the method signature and update any invocations to call the no-arg version. This keeps the functionality (always returning 2) unchanged while simplifying the API.
Concretely in src/main/java/com/extractor/unraveldocs/wordexport/impl/DocxExportServiceImpl.java:
- Change the method declaration at lines 168–170 from
private java.math.BigInteger getDecimalNumId(XWPFDocument document)toprivate java.math.BigInteger getDecimalNumId(), leaving the body as-is. - Update any calls within the shown class from
getDecimalNumId(document)(or any argument) togetDecimalNumId(). The call sites are not shown in the snippet; they must exist somewhere in this same file if the method is used. Because we are constrained to only edit shown snippets, we can safely adjust just the method signature here. If there are no call sites in the rest of the file, no further changes are needed; if there are, they should be similarly updated when that code is edited.
No new imports or additional methods are required; we are only simplifying an existing method’s parameter list.
| @@ -165,7 +165,7 @@ | ||
| return java.math.BigInteger.valueOf(1); | ||
| } | ||
|
|
||
| private java.math.BigInteger getDecimalNumId(XWPFDocument document) { | ||
| private java.math.BigInteger getDecimalNumId() { | ||
| return java.math.BigInteger.valueOf(2); | ||
| } | ||
| } |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| return java.math.BigInteger.valueOf(1); | ||
| } | ||
|
|
||
| private java.math.BigInteger getDecimalNumId(XWPFDocument document) { |
|
|
||
| // Simplified numbering helpers - in a real app these would use a numbering.xml | ||
| // template | ||
| private java.math.BigInteger getBulletNumId(XWPFDocument document) { |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (2)
src/main/java/com/extractor/unraveldocs/auth/service/AuthService.java (1)
53-55: Inconsistent return type and discarded response information.The
logout()method (line 49-51) returnsUnravelDocsResponse<Void>whilelogoutAllDevices()now returnsvoid, creating an inconsistent pattern within the same service. Additionally, the response fromrefreshTokenService.logoutAllDevices(request)is being discarded—if the underlying implementation ever returns error/status information via the response wrapper (rather than throwing exceptions), that information would be silently lost.Consider either:
- Keeping the
UnravelDocsResponse<Void>return type for consistency withlogout()and other methods.- Or if
voidis intentional, also refactorlogout()to match.♻️ Option 1: Maintain consistency by returning the response
- public void logoutAllDevices(HttpServletRequest request) { - refreshTokenService.logoutAllDevices(request); + public UnravelDocsResponse<Void> logoutAllDevices(HttpServletRequest request) { + return refreshTokenService.logoutAllDevices(request); }Note: This would require updating the controller to handle the response appropriately.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/extractor/unraveldocs/auth/service/AuthService.java` around lines 53 - 55, The logoutAllDevices method currently discards the response and uses void, which is inconsistent with logout() and may drop status info from refreshTokenService.logoutAllDevices(request); change logoutAllDevices to return UnravelDocsResponse<Void> and return the result of refreshTokenService.logoutAllDevices(request) (update the method signature in AuthService and adjust any callers/controllers to handle the returned UnravelDocsResponse<Void>), ensuring you reference logoutAllDevices, logout, refreshTokenService.logoutAllDevices, and UnravelDocsResponse<Void> when making the change.src/test/java/com/extractor/unraveldocs/coupon/service/impl/CouponServiceImplTest.java (1)
175-200: Strengthen coverage for the new duration contract.This matcher only proves the saved expiry is more than four days out, so it still passes if the implementation adds six days or misses truncation. Please assert the exact derived value here, and add the missing BAD_REQUEST case for requests that omit both
validUntiland the duration pair.✅ Tighter assertion
verify(couponRepository).save(argThat(coupon -> { - return coupon.getValidUntil() != null && - coupon.getValidUntil().isAfter(coupon.getValidFrom().plusDays(4)); + return coupon.getValidFrom() != null && + coupon.getValidUntil() != null && + coupon.getValidUntil().isEqual(coupon.getValidFrom().plusDays(5)); }));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/extractor/unraveldocs/coupon/service/impl/CouponServiceImplTest.java` around lines 175 - 200, Update the test in CouponServiceImplTest so the matcher asserts the exact computed expiry instead of a loose >4-days check: for createCoupon_withDurationValue_savesCoupon build the requestWithDuration with known validFrom (or capture the saved Coupon.getValidFrom()) and assert coupon.getValidUntil().equals(coupon.getValidFrom().plus(5, requestWithDuration.getValidDurationUnit())) (or the equivalent ChronoUnit-based addition) and include equality/truncation checks; additionally add a new test that calls couponService.createCoupon with a request missing both validUntil and the duration pair (validDurationValue and validDurationUnit) and assert the service responds/throws a BAD_REQUEST outcome (verify responseBuilderService.buildUserResponse called with HttpStatus.BAD_REQUEST or the expected exception), referencing methods couponService.createCoupon, couponRepository.save, and responseBuilderService.buildUserResponse to locate the logic to exercise.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/com/extractor/unraveldocs/coupon/api_docs/api_docs.md`:
- Around line 39-55: The API docs table is out of sync with the
CreateCouponRequest DTO: update the entries for `customCode` and
`maxUsagePerUser` in the api_docs.md table to match the DTO contract used by
CreateCouponRequest — change `customCode` from "Yes, 5-20 chars" to optional and
allow "3-40 chars", and change `maxUsagePerUser` default from "unlimited" to
default "1"; ensure the note about required validity fields and ISO 8601 format
remains accurate.
In
`@src/main/java/com/extractor/unraveldocs/coupon/api_docs/COUPON_SYSTEM_DOCUMENTATION.md`:
- Around line 117-127: The table in COUPON_SYSTEM_DOCUMENTATION.md lists the
bulk-generation endpoint as `POST /bulk` which conflicts with the canonical path
in api_docs.md; update the entry in the table (the row currently showing
`/bulk`) to `POST /admin/coupons/bulk-generate` so the `POST` + endpoint string
matches the documented route, and ensure any surrounding description text
referencing bulk generation uses the same `/admin/coupons/bulk-generate` path.
- Around line 409-411: The three Markdown list items with empty link targets
("FEATURE_COUPON_SYSTEM.md", "Email Templates", "API Documentation") are
rendering as broken links; update the entries in COUPON_SYSTEM_DOCUMENTATION.md
so each link points to a real file/URL or convert them to plain text: either
replace []() with the correct relative file paths or URLs (e.g.,
FEATURE_COUPON_SYSTEM.md -> ./FEATURE_COUPON_SYSTEM.md or a docs URL) or remove
the link syntax and leave the descriptive text ("FEATURE_COUPON_SYSTEM.md",
"Email Templates", "API Documentation") so markdownlint no longer flags them.
In
`@src/main/java/com/extractor/unraveldocs/coupon/dto/request/CreateCouponRequest.java`:
- Around line 81-84: Replace the exposed java.time.temporal.ChronoUnit on the
CreateCouponRequest DTO with a dedicated app enum (e.g., CouponDurationUnit)
containing only safe expiry units (SECONDS, MINUTES, HOURS, DAYS, WEEKS, MONTHS,
YEARS); change the CreateCouponRequest.validDurationUnit field type to this
enum, update any request deserialization annotations if needed, and in
CouponServiceImpl.createCoupon() map the new enum to the appropriate
TemporalUnit (ChronoUnit.*) before calling OffsetDateTime.plus(...), handling
nulls and providing a clear error/validation message for unsupported or missing
units.
In
`@src/main/java/com/extractor/unraveldocs/coupon/service/impl/CouponServiceImpl.java`:
- Around line 89-92: The date validation in CouponServiceImpl currently only
rejects when validUntil.isBefore(validFrom) and therefore allows validUntil ==
validFrom; update the check in the method using
responseBuilder.buildUserResponse so that it also rejects equal dates (e.g.,
change the condition to reject when validUntil.isBefore(validFrom) OR
validUntil.isEqual(validFrom), or equivalently require
validUntil.isAfter(validFrom)) and return the same BAD_REQUEST response when the
window is zero-length or negative.
- Around line 71-87: In CouponServiceImpl ensure both request.getValidFrom() and
the final validUntil are normalized to UTC with seconds precision: when reading
validFrom (variable validFrom) convert any provided OffsetDateTime to
ZoneOffset.UTC and truncateTo(ChronoUnit.SECONDS) (not only the fallback), use
that normalized validFrom when computing validUntil via plus(...), and after
determining validUntil (whether provided or computed) convert it to UTC and
truncateTo(ChronoUnit.SECONDS) before further use or storage so both timestamps
follow the “UTC seconds” contract.
- Around line 97-99: The current logic passes request.getCustomCode() into
generateUniqueCode(), which appends randomness and breaks exact custom codes;
change it so when request.getCustomCode() is present you normalize it
(toUpperCase().trim()) but do NOT call generateUniqueCode() — instead set code =
normalizedCustomCode and explicitly validate uniqueness (e.g., via the
repository check used elsewhere, like couponRepository.existsByCode or
findByCode) and return/throw the documented conflict if it already exists; only
call generateUniqueCode(...) when customCode is null/blank.
In
`@src/main/java/com/extractor/unraveldocs/wordexport/impl/DocxExportServiceImpl.java`:
- Around line 41-44: The isHtml(String text) method currently only checks for
four exact tag substrings and misses valid HTML/variants; update isHtml to
detect HTML more broadly by matching common tag patterns or attributes (e.g., a
regex like "<\\s*[a-zA-Z]+(\\s+[^>]*)?>" or checking for any "<...>" pairs) or,
better, use the stored content format flag if available; modify the isHtml
method (referenced by isHtml(String text)) to use a robust regex or consult the
content-format metadata so edited HTML with <h1>, <ol>, <strong>, <p
class="...">, self-closing tags, etc., are correctly detected and routed to the
HTML-preserving export path.
- Around line 79-85: The list items call getBulletNumId(document) and
getDecimalNumId(document) which return hardcoded IDs but never create numbering
definitions, so create proper numbering/abstract numbering before assigning
numIds: implement functions (or update getBulletNumId/getDecimalNumId) to add
AbstractNum definitions to the XWPFDocument numbering (using
XWPFDocument.createNumbering(), createAbstractNum/addAbstractNum/addNum or
similar) and return the resulting numId instead of hardcoded 1/2; update the
code paths that call getBulletNumId/getDecimalNumId in DocxExportServiceImpl
(the block handling "ul"/"ol" and the methods getBulletNumId/getDecimalNumId) to
ensure numbering entries exist on the document before calling
liPara.setNumID(...).
In
`@src/main/java/com/extractor/unraveldocs/wordexport/impl/DownloadOcrServiceImpl.java`:
- Around line 56-67: The current handling in DownloadOcrServiceImpl uses an if
("edited".equalsIgnoreCase(type)) / else that treats any other value (including
typos) as "original"; change the logic to explicitly accept only "edited" or
"original" (case-insensitive) and throw a BadRequestException for any other
value; adjust the branches to validate editedContent when type equals "edited"
and validate extractedText when type equals "original", setting textToExport
accordingly (use the existing variables ocrData, textToExport, documentId in
your checks).
---
Nitpick comments:
In `@src/main/java/com/extractor/unraveldocs/auth/service/AuthService.java`:
- Around line 53-55: The logoutAllDevices method currently discards the response
and uses void, which is inconsistent with logout() and may drop status info from
refreshTokenService.logoutAllDevices(request); change logoutAllDevices to return
UnravelDocsResponse<Void> and return the result of
refreshTokenService.logoutAllDevices(request) (update the method signature in
AuthService and adjust any callers/controllers to handle the returned
UnravelDocsResponse<Void>), ensuring you reference logoutAllDevices, logout,
refreshTokenService.logoutAllDevices, and UnravelDocsResponse<Void> when making
the change.
In
`@src/test/java/com/extractor/unraveldocs/coupon/service/impl/CouponServiceImplTest.java`:
- Around line 175-200: Update the test in CouponServiceImplTest so the matcher
asserts the exact computed expiry instead of a loose >4-days check: for
createCoupon_withDurationValue_savesCoupon build the requestWithDuration with
known validFrom (or capture the saved Coupon.getValidFrom()) and assert
coupon.getValidUntil().equals(coupon.getValidFrom().plus(5,
requestWithDuration.getValidDurationUnit())) (or the equivalent ChronoUnit-based
addition) and include equality/truncation checks; additionally add a new test
that calls couponService.createCoupon with a request missing both validUntil and
the duration pair (validDurationValue and validDurationUnit) and assert the
service responds/throws a BAD_REQUEST outcome (verify
responseBuilderService.buildUserResponse called with HttpStatus.BAD_REQUEST or
the expected exception), referencing methods couponService.createCoupon,
couponRepository.save, and responseBuilderService.buildUserResponse to locate
the logic to exercise.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9874dd24-1210-40fb-a4d3-53fe9a8ae34b
📒 Files selected for processing (12)
src/main/java/com/extractor/unraveldocs/auth/service/AuthService.javasrc/main/java/com/extractor/unraveldocs/coupon/api_docs/COUPON_SYSTEM_DOCUMENTATION.mdsrc/main/java/com/extractor/unraveldocs/coupon/api_docs/INTEGRATION_GUIDE.mdsrc/main/java/com/extractor/unraveldocs/coupon/api_docs/api_docs.mdsrc/main/java/com/extractor/unraveldocs/coupon/dto/request/CreateCouponRequest.javasrc/main/java/com/extractor/unraveldocs/coupon/helpers/CouponCodeGenerator.javasrc/main/java/com/extractor/unraveldocs/coupon/service/impl/CouponServiceImpl.javasrc/main/java/com/extractor/unraveldocs/wordexport/controller/DownloadOcrCResultController.javasrc/main/java/com/extractor/unraveldocs/wordexport/impl/DocxExportServiceImpl.javasrc/main/java/com/extractor/unraveldocs/wordexport/impl/DownloadOcrServiceImpl.javasrc/main/java/com/extractor/unraveldocs/wordexport/interfaces/DownloadOcrResultService.javasrc/test/java/com/extractor/unraveldocs/coupon/service/impl/CouponServiceImplTest.java
💤 Files with no reviewable changes (1)
- src/main/java/com/extractor/unraveldocs/coupon/api_docs/INTEGRATION_GUIDE.md
| | Field | Type | Required | Description | | ||
| |----------------------|------------|----------|------------------------------------------------------------------------------------------------------| | ||
| | `customCode` | `string` | Yes | Unique code for the coupon (alphanumeric, 5-20 chars) | | ||
| | `description` | `string` | Yes | Description of the coupon | | ||
| | `discountPercentage` | `decimal` | Yes | Discount percentage (e.g., 15.00 for 15%) | | ||
| | `minPurchaseAmount` | `decimal` | No | Minimum purchase amount to apply coupon (default: 0.00) | | ||
| | `validFrom` | `datetime` | No | Start date/time for coupon validity (ISO 8601 UTC format). Defaults to creation time if not provided | | ||
| | `validUntil` | `datetime` | No* | End date/time for coupon validity (ISO 8601 UTC format) | | ||
| | `validDurationValue` | `long` | No* | Duration value (e.g. 5, 13) for dynamic expiry matching validDurationUnit | | ||
| | `validDurationUnit` | `string` | No* | Unit for duration (e.g. Seconds, Minutes, Days) | | ||
| | `maxUsageCount` | `int` | No | Maximum total uses for the coupon (default: unlimited) | | ||
| | `maxUsagePerUser` | `int` | No | Maximum uses per user (default: unlimited) | | ||
| | `recipientCategory` | `string` | Yes | Target users: `ALL_USERS`, `NEW_USERS`, `ALL_PAID_USERS`, `SPECIFIC_USERS` | | ||
| | `specificUserIds` | `array` | No | List of user IDs (UUIDs) if `recipientCategory` is `SPECIFIC_USERS` | | ||
| | `templateId` | `string` | No | ID of coupon template to base this coupon on (if any) | | ||
|
|
||
| **Note:** You must provide either `validUntil` OR `validDurationValue` & `validDurationUnit` for the coupon to be successfully created. Dates must strictly conform to ISO 8601 UTC formats truncated to seconds (e.g. "2026-03-28T23:59:59Z"). |
There was a problem hiding this comment.
Sync this request table with the actual DTO contract.
customCode is optional in CreateCouponRequest and now allows 3-40 characters, not “required, 5-20 chars”. maxUsagePerUser also defaults to 1, not unlimited. Publishing the old contract here will cause avoidable client-side validation failures.
📝 Suggested doc update
-| `customCode` | `string` | Yes | Unique code for the coupon (alphanumeric, 5-20 chars) |
+| `customCode` | `string` | No | Optional custom code for the coupon (alphanumeric, 3-40 chars) |
@@
-| `maxUsagePerUser` | `int` | No | Maximum uses per user (default: unlimited) |
+| `maxUsagePerUser` | `int` | No | Maximum uses per user (default: 1) |📝 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.
| | Field | Type | Required | Description | | |
| |----------------------|------------|----------|------------------------------------------------------------------------------------------------------| | |
| | `customCode` | `string` | Yes | Unique code for the coupon (alphanumeric, 5-20 chars) | | |
| | `description` | `string` | Yes | Description of the coupon | | |
| | `discountPercentage` | `decimal` | Yes | Discount percentage (e.g., 15.00 for 15%) | | |
| | `minPurchaseAmount` | `decimal` | No | Minimum purchase amount to apply coupon (default: 0.00) | | |
| | `validFrom` | `datetime` | No | Start date/time for coupon validity (ISO 8601 UTC format). Defaults to creation time if not provided | | |
| | `validUntil` | `datetime` | No* | End date/time for coupon validity (ISO 8601 UTC format) | | |
| | `validDurationValue` | `long` | No* | Duration value (e.g. 5, 13) for dynamic expiry matching validDurationUnit | | |
| | `validDurationUnit` | `string` | No* | Unit for duration (e.g. Seconds, Minutes, Days) | | |
| | `maxUsageCount` | `int` | No | Maximum total uses for the coupon (default: unlimited) | | |
| | `maxUsagePerUser` | `int` | No | Maximum uses per user (default: unlimited) | | |
| | `recipientCategory` | `string` | Yes | Target users: `ALL_USERS`, `NEW_USERS`, `ALL_PAID_USERS`, `SPECIFIC_USERS` | | |
| | `specificUserIds` | `array` | No | List of user IDs (UUIDs) if `recipientCategory` is `SPECIFIC_USERS` | | |
| | `templateId` | `string` | No | ID of coupon template to base this coupon on (if any) | | |
| **Note:** You must provide either `validUntil` OR `validDurationValue` & `validDurationUnit` for the coupon to be successfully created. Dates must strictly conform to ISO 8601 UTC formats truncated to seconds (e.g. "2026-03-28T23:59:59Z"). | |
| | Field | Type | Required | Description | | |
| |----------------------|------------|----------|------------------------------------------------------------------------------------------------------| | |
| | `customCode` | `string` | No | Optional custom code for the coupon (alphanumeric, 3-40 chars) | | |
| | `description` | `string` | Yes | Description of the coupon | | |
| | `discountPercentage` | `decimal` | Yes | Discount percentage (e.g., 15.00 for 15%) | | |
| | `minPurchaseAmount` | `decimal` | No | Minimum purchase amount to apply coupon (default: 0.00) | | |
| | `validFrom` | `datetime` | No | Start date/time for coupon validity (ISO 8601 UTC format). Defaults to creation time if not provided | | |
| | `validUntil` | `datetime` | No* | End date/time for coupon validity (ISO 8601 UTC format) | | |
| | `validDurationValue` | `long` | No* | Duration value (e.g. 5, 13) for dynamic expiry matching validDurationUnit | | |
| | `validDurationUnit` | `string` | No* | Unit for duration (e.g. Seconds, Minutes, Days) | | |
| | `maxUsageCount` | `int` | No | Maximum total uses for the coupon (default: unlimited) | | |
| | `maxUsagePerUser` | `int` | No | Maximum uses per user (default: 1) | | |
| | `recipientCategory` | `string` | Yes | Target users: `ALL_USERS`, `NEW_USERS`, `ALL_PAID_USERS`, `SPECIFIC_USERS` | | |
| | `specificUserIds` | `array` | No | List of user IDs (UUIDs) if `recipientCategory` is `SPECIFIC_USERS` | | |
| | `templateId` | `string` | No | ID of coupon template to base this coupon on (if any) | | |
| **Note:** You must provide either `validUntil` OR `validDurationValue` & `validDurationUnit` for the coupon to be successfully created. Dates must strictly conform to ISO 8601 UTC formats truncated to seconds (e.g. "2026-03-28T23:59:59Z"). |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/extractor/unraveldocs/coupon/api_docs/api_docs.md` around
lines 39 - 55, The API docs table is out of sync with the CreateCouponRequest
DTO: update the entries for `customCode` and `maxUsagePerUser` in the
api_docs.md table to match the DTO contract used by CreateCouponRequest — change
`customCode` from "Yes, 5-20 chars" to optional and allow "3-40 chars", and
change `maxUsagePerUser` default from "unlimited" to default "1"; ensure the
note about required validity fields and ISO 8601 format remains accurate.
| | Method | Endpoint | Description | | ||
| |----------|-------------------------|------------------------------------------| | ||
| | `POST` | `/` | Create a new coupon | | ||
| | `PUT` | `/{couponId}` | Update coupon details | | ||
| | `DELETE` | `/{couponId}` | Deactivate a coupon | | ||
| | `GET` | `/{couponId}` | Get coupon by ID | | ||
| | `GET` | `/` | List all coupons (paginated, filterable) | | ||
| | `GET` | `/{couponId}/usage` | Get coupon usage history | | ||
| | `GET` | `/{couponId}/analytics` | Get coupon analytics | | ||
| | `POST` | `/bulk` | Generate bulk coupons | | ||
|
|
There was a problem hiding this comment.
Align the bulk-generation endpoint with the API reference.
This table documents bulk generation as /bulk, but src/main/java/com/extractor/unraveldocs/coupon/api_docs/api_docs.md documents it as POST /admin/coupons/bulk-generate. One of these is wrong today, which will send readers to the wrong route.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/extractor/unraveldocs/coupon/api_docs/COUPON_SYSTEM_DOCUMENTATION.md`
around lines 117 - 127, The table in COUPON_SYSTEM_DOCUMENTATION.md lists the
bulk-generation endpoint as `POST /bulk` which conflicts with the canonical path
in api_docs.md; update the entry in the table (the row currently showing
`/bulk`) to `POST /admin/coupons/bulk-generate` so the `POST` + endpoint string
matches the documented route, and ensure any surrounding description text
referencing bulk generation uses the same `/admin/coupons/bulk-generate` path.
| - [FEATURE_COUPON_SYSTEM.md]() - Feature overview and implementation status | ||
| - [Email Templates]() - Thymeleaf templates for notifications | ||
| - [API Documentation]() - Swagger/OpenAPI documentation |
There was a problem hiding this comment.
Replace the empty links with real targets or plain text.
These three entries render as broken links, and markdownlint is already flagging them.
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 409-409: No empty links
(MD042, no-empty-links)
[warning] 410-410: No empty links
(MD042, no-empty-links)
[warning] 411-411: No empty links
(MD042, no-empty-links)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/extractor/unraveldocs/coupon/api_docs/COUPON_SYSTEM_DOCUMENTATION.md`
around lines 409 - 411, The three Markdown list items with empty link targets
("FEATURE_COUPON_SYSTEM.md", "Email Templates", "API Documentation") are
rendering as broken links; update the entries in COUPON_SYSTEM_DOCUMENTATION.md
so each link points to a real file/URL or convert them to plain text: either
replace []() with the correct relative file paths or URLs (e.g.,
FEATURE_COUPON_SYSTEM.md -> ./FEATURE_COUPON_SYSTEM.md or a docs URL) or remove
the link syntax and leave the descriptive text ("FEATURE_COUPON_SYSTEM.md",
"Email Templates", "API Documentation") so markdownlint no longer flags them.
| /** | ||
| * Optional duration unit for coupon validity (e.g. SECONDS, MINUTES, DAYS). | ||
| */ | ||
| private java.time.temporal.ChronoUnit validDurationUnit; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Which java.time.temporal.ChronoUnit values are supported by java.time.OffsetDateTime.plus(long, TemporalUnit), and what happens if ChronoUnit.FOREVER is passed?
💡 Result:
OffsetDateTime.plus(long, TemporalUnit) treats ChronoUnit specially: if the unit is a ChronoUnit, the addition is delegated to LocalDateTime.plus(long, TemporalUnit) and the offset is left unchanged. [1]
Therefore, the supported ChronoUnit values are exactly those LocalDateTime reports as supported units:
NANOS, MICROS, MILLIS, SECONDS, MINUTES, HOURS, HALF_DAYS, DAYS, WEEKS, MONTHS, YEARS, DECADES, CENTURIES, MILLENNIA, ERAS. [2]
If you pass ChronoUnit.FOREVER, it is not in that supported list (so it’s unsupported), and OffsetDateTime.plus(...) throws UnsupportedTemporalTypeException. [1] [2]
🏁 Script executed:
# Locate and examine CouponServiceImpl to see how validDurationUnit is used
find . -name "CouponServiceImpl.java" -type f | head -5Repository: Brints/unraveldocs-api
Length of output: 150
🏁 Script executed:
# Read CouponServiceImpl.java to see how validDurationUnit is used
cat -n ./src/main/java/com/extractor/unraveldocs/coupon/service/impl/CouponServiceImpl.javaRepository: Brints/unraveldocs-api
Length of output: 22200
Replace raw ChronoUnit with an app-level enum for valid coupon expiry units.
Exposing java.time.temporal.ChronoUnit directly on the API request contract allows invalid values like FOREVER (which throws UnsupportedTemporalTypeException at runtime) and nonsensical units like ERAS, DECADES, CENTURIES, or MILLENNIA for coupon expiry. The value is passed directly to OffsetDateTime.plus(long, TemporalUnit) on line 78 of CouponServiceImpl.createCoupon() without any validation. Create a constrained enum of meaningful units (e.g., SECONDS, MINUTES, HOURS, DAYS, WEEKS, MONTHS, YEARS) to make the contract safer and self-documenting.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/extractor/unraveldocs/coupon/dto/request/CreateCouponRequest.java`
around lines 81 - 84, Replace the exposed java.time.temporal.ChronoUnit on the
CreateCouponRequest DTO with a dedicated app enum (e.g., CouponDurationUnit)
containing only safe expiry units (SECONDS, MINUTES, HOURS, DAYS, WEEKS, MONTHS,
YEARS); change the CreateCouponRequest.validDurationUnit field type to this
enum, update any request deserialization annotations if needed, and in
CouponServiceImpl.createCoupon() map the new enum to the appropriate
TemporalUnit (ChronoUnit.*) before calling OffsetDateTime.plus(...), handling
nulls and providing a clear error/validation message for unsupported or missing
units.
| OffsetDateTime validFrom = request.getValidFrom() != null | ||
| ? request.getValidFrom() | ||
| : OffsetDateTime.now(ZoneOffset.UTC).truncatedTo(ChronoUnit.SECONDS); | ||
|
|
||
| OffsetDateTime validUntil = request.getValidUntil(); | ||
| if (validUntil == null) { | ||
| if (request.getValidDurationValue() != null && request.getValidDurationUnit() != null) { | ||
| validUntil = validFrom.plus(request.getValidDurationValue(), request.getValidDurationUnit()); | ||
| } else { | ||
| return responseBuilder.buildUserResponse( | ||
| null, HttpStatus.BAD_REQUEST, | ||
| "Either validUntil or validDurationValue and validDurationUnit must be provided"); | ||
| } | ||
| } | ||
|
|
||
| // Ensure validUntil is properly truncated if calculated or set | ||
| validUntil = validUntil.truncatedTo(ChronoUnit.SECONDS); |
There was a problem hiding this comment.
Normalize both validity timestamps to UTC seconds before using them.
Right now only the fallback validFrom is normalized, and only validUntil is truncated. If a caller sends validFrom with a non-UTC offset or fractional seconds, the stored window no longer matches the documented “UTC seconds” contract, and the derived validUntil can end up slightly shorter than the requested duration.
🕒 Proposed fix
- OffsetDateTime validFrom = request.getValidFrom() != null
- ? request.getValidFrom()
- : OffsetDateTime.now(ZoneOffset.UTC).truncatedTo(ChronoUnit.SECONDS);
+ OffsetDateTime validFrom = request.getValidFrom() != null
+ ? request.getValidFrom().withOffsetSameInstant(ZoneOffset.UTC).truncatedTo(ChronoUnit.SECONDS)
+ : OffsetDateTime.now(ZoneOffset.UTC).truncatedTo(ChronoUnit.SECONDS);
OffsetDateTime validUntil = request.getValidUntil();
if (validUntil == null) {
if (request.getValidDurationValue() != null && request.getValidDurationUnit() != null) {
validUntil = validFrom.plus(request.getValidDurationValue(), request.getValidDurationUnit());
} else {
return responseBuilder.buildUserResponse(
null, HttpStatus.BAD_REQUEST,
"Either validUntil or validDurationValue and validDurationUnit must be provided");
}
}
- // Ensure validUntil is properly truncated if calculated or set
- validUntil = validUntil.truncatedTo(ChronoUnit.SECONDS);
+ validUntil = validUntil.withOffsetSameInstant(ZoneOffset.UTC).truncatedTo(ChronoUnit.SECONDS);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/extractor/unraveldocs/coupon/service/impl/CouponServiceImpl.java`
around lines 71 - 87, In CouponServiceImpl ensure both request.getValidFrom()
and the final validUntil are normalized to UTC with seconds precision: when
reading validFrom (variable validFrom) convert any provided OffsetDateTime to
ZoneOffset.UTC and truncateTo(ChronoUnit.SECONDS) (not only the fallback), use
that normalized validFrom when computing validUntil via plus(...), and after
determining validUntil (whether provided or computed) convert it to UTC and
truncateTo(ChronoUnit.SECONDS) before further use or storage so both timestamps
follow the “UTC seconds” contract.
| // Validate date range | ||
| if (request.getValidUntil().isBefore(request.getValidFrom())) { | ||
| if (validUntil.isBefore(validFrom)) { | ||
| return responseBuilder.buildUserResponse( | ||
| null, HttpStatus.BAD_REQUEST, "Valid until date must be after valid from date"); |
There was a problem hiding this comment.
Reject validUntil == validFrom as well.
Line 90 still accepts a zero-length validity window even though the error message says the end date must be after the start date.
🛠️ Proposed fix
- if (validUntil.isBefore(validFrom)) {
+ if (!validUntil.isAfter(validFrom)) {
return responseBuilder.buildUserResponse(
null, HttpStatus.BAD_REQUEST, "Valid until date must be after valid from date");
}📝 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.
| // Validate date range | |
| if (request.getValidUntil().isBefore(request.getValidFrom())) { | |
| if (validUntil.isBefore(validFrom)) { | |
| return responseBuilder.buildUserResponse( | |
| null, HttpStatus.BAD_REQUEST, "Valid until date must be after valid from date"); | |
| // Validate date range | |
| if (!validUntil.isAfter(validFrom)) { | |
| return responseBuilder.buildUserResponse( | |
| null, HttpStatus.BAD_REQUEST, "Valid until date must be after valid from date"); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/extractor/unraveldocs/coupon/service/impl/CouponServiceImpl.java`
around lines 89 - 92, The date validation in CouponServiceImpl currently only
rejects when validUntil.isBefore(validFrom) and therefore allows validUntil ==
validFrom; update the check in the method using
responseBuilder.buildUserResponse so that it also rejects equal dates (e.g.,
change the condition to reject when validUntil.isBefore(validFrom) OR
validUntil.isEqual(validFrom), or equivalently require
validUntil.isAfter(validFrom)) and return the same BAD_REQUEST response when the
window is zero-length or negative.
| if (request.getCustomCode() != null && !request.getCustomCode().isBlank()) { | ||
| code = request.getCustomCode().toUpperCase(); | ||
| if (couponRepository.existsByCode(code)) { | ||
| return responseBuilder.buildUserResponse( | ||
| null, HttpStatus.CONFLICT, "Coupon code already exists"); | ||
| } | ||
| code = generateUniqueCode(request.getCustomCode().toUpperCase().trim()); | ||
| } else { |
There was a problem hiding this comment.
Preserve the exact custom code instead of treating it as a prefix.
Passing customCode into generateUniqueCode() changes SAVE20 into something like SAVE20-AB12CD34. That breaks the admin-facing “custom code” feature and also stops duplicate custom codes from returning the documented conflict.
🎯 Proposed fix
String code;
if (request.getCustomCode() != null && !request.getCustomCode().isBlank()) {
- code = generateUniqueCode(request.getCustomCode().toUpperCase().trim());
+ code = request.getCustomCode().toUpperCase().trim();
+ if (couponRepository.existsByCode(code)) {
+ return responseBuilder.buildUserResponse(
+ null, HttpStatus.CONFLICT, "Coupon code already exists");
+ }
} else {
code = generateUniqueCode(null);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/extractor/unraveldocs/coupon/service/impl/CouponServiceImpl.java`
around lines 97 - 99, The current logic passes request.getCustomCode() into
generateUniqueCode(), which appends randomness and breaks exact custom codes;
change it so when request.getCustomCode() is present you normalize it
(toUpperCase().trim()) but do NOT call generateUniqueCode() — instead set code =
normalizedCustomCode and explicitly validate uniqueness (e.g., via the
repository check used elsewhere, like couponRepository.existsByCode or
findByCode) and return/throw the documented conflict if it already exists; only
call generateUniqueCode(...) when customCode is null/blank.
| private boolean isHtml(String text) { | ||
| return text != null | ||
| && (text.contains("<p>") || text.contains("<h3>") || text.contains("<ul>") || text.contains("<li>")); | ||
| } |
There was a problem hiding this comment.
Broaden HTML detection or key off the stored content format.
This heuristic only recognizes four exact tag strings, so valid HTML like <h1>, <ol>, <strong>, or <p class="..."> falls through to the plain-text path and gets exported with literal tags instead of formatting. That breaks the new HTML-preserving export behavior for edited content.
💡 Suggested fix
- private boolean isHtml(String text) {
- return text != null
- && (text.contains("<p>") || text.contains("<h3>") || text.contains("<ul>") || text.contains("<li>"));
- }
+ private boolean isHtml(String text) {
+ if (text == null || text.isBlank()) {
+ return false;
+ }
+
+ Document parsed = Jsoup.parseBodyFragment(text);
+ return !parsed.body().children().isEmpty();
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/extractor/unraveldocs/wordexport/impl/DocxExportServiceImpl.java`
around lines 41 - 44, The isHtml(String text) method currently only checks for
four exact tag substrings and misses valid HTML/variants; update isHtml to
detect HTML more broadly by matching common tag patterns or attributes (e.g., a
regex like "<\\s*[a-zA-Z]+(\\s+[^>]*)?>" or checking for any "<...>" pairs) or,
better, use the stored content format flag if available; modify the isHtml
method (referenced by isHtml(String text)) to use a robust regex or consult the
content-format metadata so edited HTML with <h1>, <ol>, <strong>, <p
class="...">, self-closing tags, etc., are correctly detected and routed to the
HTML-preserving export path.
| case "ul": | ||
| case "ol": | ||
| for (Element li : element.children()) { | ||
| if (li.tagName().equalsIgnoreCase("li")) { | ||
| XWPFParagraph liPara = document.createParagraph(); | ||
| liPara.setNumID(tagName.equals("ul") ? getBulletNumId(document) : getDecimalNumId(document)); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C2 --type=java 'createNumbering\s*\(|addAbstractNum\s*\(|addNum\s*\(' srcRepository: Brints/unraveldocs-api
Length of output: 48
🏁 Script executed:
cat -n src/main/java/com/extractor/unraveldocs/wordexport/impl/DocxExportServiceImpl.java | sed -n '70,95p'Repository: Brints/unraveldocs-api
Length of output: 1430
🏁 Script executed:
cat -n src/main/java/com/extractor/unraveldocs/wordexport/impl/DocxExportServiceImpl.java | sed -n '150,175p'Repository: Brints/unraveldocs-api
Length of output: 971
🏁 Script executed:
rg -n 'getBulletNumId|getDecimalNumId' src/main/java/com/extractor/unraveldocs/wordexport/impl/DocxExportServiceImpl.java -A5Repository: Brints/unraveldocs-api
Length of output: 810
🏁 Script executed:
cat -n src/main/java/com/extractor/unraveldocs/wordexport/impl/DocxExportServiceImpl.java | head -60Repository: Brints/unraveldocs-api
Length of output: 2595
🏁 Script executed:
rg -n 'new XWPFDocument|createNumbering|Numbering' src/main/java/com/extractor/unraveldocs/wordexport/impl/DocxExportServiceImpl.javaRepository: Brints/unraveldocs-api
Length of output: 184
🏁 Script executed:
rg -n 'setNumID' src/main/java/com/extractor/unraveldocs/wordexport/impl/DocxExportServiceImpl.javaRepository: Brints/unraveldocs-api
Length of output: 186
Create numbering definitions before assigning list numIds.
These paragraphs are created on a fresh XWPFDocument, but getBulletNumId() and getDecimalNumId() (lines 163–169) only return hardcoded values 1 and 2 without ever creating numbering definitions. This causes list items to reference nonexistent numbering entries, preventing bullets and numbers from rendering. The code itself acknowledges this is incomplete with comments like "in a real app these would use a numbering.xml template" and "Ideally, you define numbering styles."
Also applies to: 161-169
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/extractor/unraveldocs/wordexport/impl/DocxExportServiceImpl.java`
around lines 79 - 85, The list items call getBulletNumId(document) and
getDecimalNumId(document) which return hardcoded IDs but never create numbering
definitions, so create proper numbering/abstract numbering before assigning
numIds: implement functions (or update getBulletNumId/getDecimalNumId) to add
AbstractNum definitions to the XWPFDocument numbering (using
XWPFDocument.createNumbering(), createAbstractNum/addAbstractNum/addNum or
similar) and return the resulting numId instead of hardcoded 1/2; update the
code paths that call getBulletNumId/getDecimalNumId in DocxExportServiceImpl
(the block handling "ul"/"ol" and the methods getBulletNumId/getDecimalNumId) to
ensure numbering entries exist on the document before calling
liPara.setNumID(...).
| String textToExport = ocrData.getExtractedText(); | ||
|
|
||
| if ("edited".equalsIgnoreCase(type)) { | ||
| if (ocrData.getEditedContent() == null || ocrData.getEditedContent().isBlank()) { | ||
| throw new BadRequestException("No edited text available for the document: " + documentId); | ||
| } | ||
| textToExport = ocrData.getEditedContent(); | ||
| } else { | ||
| if (ocrData.getExtractedText() == null || ocrData.getExtractedText().isBlank()) { | ||
| throw new BadRequestException("No text extracted from the document: " + documentId); | ||
| } | ||
| } |
There was a problem hiding this comment.
Reject unsupported type values instead of silently exporting the original text.
Any value other than "edited" currently drops into the else branch, so typos such as ?type=editted return the original OCR content with a 200 response. Since this API only documents original and edited, invalid values should fail with 400.
💡 Suggested fix
- String textToExport = ocrData.getExtractedText();
-
- if ("edited".equalsIgnoreCase(type)) {
+ String normalizedType = type == null ? "original" : type.trim().toLowerCase();
+ String textToExport;
+
+ if ("edited".equals(normalizedType)) {
if (ocrData.getEditedContent() == null || ocrData.getEditedContent().isBlank()) {
throw new BadRequestException("No edited text available for the document: " + documentId);
}
textToExport = ocrData.getEditedContent();
- } else {
+ } else if ("original".equals(normalizedType)) {
if (ocrData.getExtractedText() == null || ocrData.getExtractedText().isBlank()) {
throw new BadRequestException("No text extracted from the document: " + documentId);
}
+ textToExport = ocrData.getExtractedText();
+ } else {
+ throw new BadRequestException("Unsupported export type: " + type);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/extractor/unraveldocs/wordexport/impl/DownloadOcrServiceImpl.java`
around lines 56 - 67, The current handling in DownloadOcrServiceImpl uses an if
("edited".equalsIgnoreCase(type)) / else that treats any other value (including
typos) as "original"; change the logic to explicitly accept only "edited" or
"original" (case-insensitive) and throw a BadRequestException for any other
value; adjust the branches to validate editedContent when type equals "edited"
and validate extractedText when type equals "original", setting textToExport
accordingly (use the existing variables ocrData, textToExport, documentId in
your checks).
1 participant
Summary by CodeRabbit
New Features
Documentation