AMM-997 | Retrieval of multiple files from a subcategory #239
Conversation
WalkthroughThe changes update the persistence and retrieval of subcategory file data. The Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant CommonServiceImpl
participant KMFileManagerRepository
participant SubCategoryDetails
Client->>CommonServiceImpl: getSubCategories(request)
CommonServiceImpl->>KMFileManagerRepository: find subcategories by category ID
loop For each subcategory
CommonServiceImpl->>KMFileManagerRepository: getFilesBySubCategoryID(subCategoryID)
KMFileManagerRepository-->>CommonServiceImpl: List<KMFileManager>
CommonServiceImpl->>SubCategoryDetails: Attach file list and representative file info
end
CommonServiceImpl-->>Client: List<SubCategoryDetails> (each with file list)
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (4)
✨ Finishing Touches
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. 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)
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)
src/main/java/com/iemr/common/service/kmfilemanager/KMFileManagerServiceImpl.java (1)
179-181: Remove redundant assignment.The line
kmFileManager.setSubCategoryID(kmFileManager.getSubCategoryID());is redundant as it sets the field to its current value without any transformation or validation.- - kmFileManager.setSubCategoryID(kmFileManager.getSubCategoryID()); -src/main/java/com/iemr/common/service/services/CommonServiceImpl.java (1)
137-173: Excellent refactoring with minor suggestions for improvement.The refactored
getSubCategoriesmethod correctly implements the requirement to retrieve multiple files per subcategory. The logic is sound and maintains backward compatibility by setting representative file metadata.Consider these improvements for robustness:
// Fetch all files under this subcategory from KMFileManager List<KMFileManager> files = kmFileManagerRepository.getFilesBySubCategoryID(subCatId); ArrayList<KMFileManager> fileList = new ArrayList<>(files); String fileURL = null; String fileNameWithExtension = null; if (!fileList.isEmpty()) { KMFileManager firstFile = fileList.get(0); // Just for representative file URL and name - fileURL = getFilePath(firstFile.getFileUID()); - fileNameWithExtension = firstFile.getFileName() + firstFile.getFileExtension(); + if (firstFile.getFileUID() != null) { + fileURL = getFilePath(firstFile.getFileUID()); + } + if (firstFile.getFileName() != null && firstFile.getFileExtension() != null) { + fileNameWithExtension = firstFile.getFileName() + firstFile.getFileExtension(); + } }This adds null safety checks for file properties to prevent potential null pointer exceptions.
Additionally, consider the performance impact if subcategories have a large number of files, as this will now load all files eagerly rather than on-demand.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/com/iemr/common/data/kmfilemanager/KMFileManager.java(1 hunks)src/main/java/com/iemr/common/repository/kmfilemanager/KMFileManagerRepository.java(1 hunks)src/main/java/com/iemr/common/service/kmfilemanager/KMFileManagerServiceImpl.java(3 hunks)src/main/java/com/iemr/common/service/services/CommonServiceImpl.java(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/main/java/com/iemr/common/service/services/CommonServiceImpl.java (1)
Learnt from: helenKaryamsetty
PR: PSMRI/Common-API#145
File: src/main/java/com/iemr/common/service/abdmfacility/AbdmFacilityServiceImpl.java:16-20
Timestamp: 2024-12-18T08:53:22.725Z
Learning: In AbdmFacilityServiceImpl, no exceptions are thrown because the UI layer takes responsibility for handling all error scenarios and directly uses the raw response returned by the repository.
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: style-check / checkstyle
- GitHub Check: Package-test
- GitHub Check: Build
- GitHub Check: Analyze (java)
🔇 Additional comments (5)
src/main/java/com/iemr/common/service/kmfilemanager/KMFileManagerServiceImpl.java (2)
128-128: LGTM - Improved code readability.The blank line addition improves code readability by separating methods.
204-204: LGTM - Improved code readability.The blank line addition improves code readability by separating methods.
src/main/java/com/iemr/common/repository/kmfilemanager/KMFileManagerRepository.java (1)
77-80: LGTM - Well-implemented repository method.The new
getFilesBySubCategoryIDmethod correctly implements the requirement to retrieve multiple files per subcategory. The query properly filters by subcategory ID and excludes deleted records, which aligns with the PR objectives.src/main/java/com/iemr/common/data/kmfilemanager/KMFileManager.java (1)
113-119: ```shell
#!/bin/bashSearch for Liquibase changelog definitions
rg -Hni "changeSet" --glob '.xml' --glob '.yml' --glob '*.yaml'
Search for any migration scripts referencing SubCategoryID
rg -Hni "SubCategoryID" --glob '.sql' --glob '.xml' --glob '.yml' --glob '.yaml'
</details> <details> <summary>src/main/java/com/iemr/common/service/services/CommonServiceImpl.java (1)</summary> `60-60`: **LGTM - Necessary import added.** The import for `KMFileManager` is correctly added to support the new functionality. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| @Expose | ||
| private Integer categoryID; | ||
| @Transient | ||
| // @Transient |
| + "where kmFileManager.fileUID = :fileUID") | ||
| List<Object[]> getFileNameByUID(@Param("fileUID") String fileUID); | ||
|
|
||
| //newChange |
|
2 participants
📋 Description
JIRA ID: 997
Please provide a summary of the change and the motivation behind it. Include relevant context and details.
Only a single file was being fetched per subcategory , because the file UID was overwritten in subcategory table everytime the new file was added to the subcategory. So, a table change was required to add Subcategory id as a foreign key in km file manager table. And code changes were required to fetch all files linked to a particular subcategory.
✅ 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
New Features
Bug Fixes
Refactor