Refactor UploadManager.uploadNews Realm queries into VoicesRepository#11623
Refactor UploadManager.uploadNews Realm queries into VoicesRepository#11623
Conversation
…sitory - Created `NewsUploadData` DTO to hold unmanaged news properties - Added `getPendingNews(chatRepository)` and `markNewsUploaded(...)` to `VoicesRepository` and `VoicesRepositoryImpl` - Updated `UploadManager.uploadNews` to use `VoicesRepository` instead of direct Realm operations - Injected `VoicesRepository` into `UploadManager` via Dagger/Hilt (`ServiceModule`) Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
❌ 1 blocking issue (1 total)
|
Okuro3499
left a comment
There was a problem hiding this comment.
1. Repository-to-repository coupling via parameter (VoicesRepository:40)
suspend fun getPendingNews(chatRepository: ChatRepository): List<NewsUploadData>
VoicesRepository is now accepting ChatRepository as a parameter of a repository interface method. This creates an explicit horizontal dependency between two domain repositories — a direct violation of layered architecture. Repositories should not depend on each other; they share the same layer and should both depend on lower-level services only.
Fix: inject ChatRepository into VoicesRepositoryImpl's constructor (it already has Hilt @Inject) and remove the parameter from the interface.
2. news.id!! force-unwrap crash (UploadManager.kt:674)
voicesRepository.markNewsUploaded(
news.id!!, // ← throws NPE if id is null
...
)
The old code used equalTo("id", news.id) which would simply find nothing if null. The new call crashes the process. NewsUploadData.id is typed String?, so this is a genuine regression.
Fix: skip the item (continue) or guard with ?: continue before calling markNewsUploaded.
This refactor extracts the raw Realm database queries previously located in
UploadManager.uploadNewsand moves them into theVoicesRepositorylayer. A new DTO,NewsUploadData, has been created to safely pass the necessary fields and the serialized JSON (viachatRepository) back to theUploadManager. Additionally, the database modifications required after a successful upload have been abstracted intoVoicesRepository.markNewsUploaded(...). This aligns with the repository pattern and keeps database coupling out of the service layer.PR created automatically by Jules for task 15545347176171038591 started by @dogi