feat: migrate event coordinator to shared KMP - #40
Merged
Conversation
|
Warning Review limit reached
More reviews will be available in 42 minutes and 21 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (23)
📝 WalkthroughWalkthroughThis PR refactors event management from a centralized mediator pattern to a ViewModel-based architecture. Legacy mediator classes are removed (~1,140 lines). New shared models, validators, and two ViewModels are introduced: ChangesEvent Management Architecture Refactoring
Sequence DiagramsequenceDiagram
participant CreateEventScreen
participant EventCreationViewModel
participant EventCreationValidator
participant Repository
CreateEventScreen->>EventCreationViewModel: updateOrganizerInfo(draft)
activate EventCreationViewModel
EventCreationViewModel->>EventCreationViewModel: clearValidationErrors
deactivate EventCreationViewModel
CreateEventScreen->>EventCreationViewModel: goNext()
activate EventCreationViewModel
EventCreationViewModel->>EventCreationValidator: validateCurrentStep()
activate EventCreationValidator
EventCreationValidator-->>EventCreationViewModel: ValidationResult
deactivate EventCreationValidator
alt Valid
EventCreationViewModel->>EventCreationViewModel: advance to next step
EventCreationViewModel-->>CreateEventScreen: true
else Invalid
EventCreationViewModel->>EventCreationViewModel: setValidationErrors
EventCreationViewModel-->>CreateEventScreen: false
end
deactivate EventCreationViewModel
CreateEventScreen->>CreateEventScreen: populate field errors from uiState
rect rgba(200, 150, 100, 0.5)
Note over CreateEventScreen,EventCreationViewModel: On final submit
CreateEventScreen->>EventCreationViewModel: requestSubmit()
activate EventCreationViewModel
EventCreationViewModel->>EventCreationValidator: validateAll(draft)
activate EventCreationValidator
EventCreationValidator-->>EventCreationViewModel: ValidationResult
deactivate EventCreationValidator
alt Valid
EventCreationViewModel->>Repository: createEvent(draft)
activate Repository
Repository-->>EventCreationViewModel: result
deactivate Repository
EventCreationViewModel-->>CreateEventScreen: true
else Invalid
EventCreationViewModel-->>CreateEventScreen: false
end
deactivate EventCreationViewModel
end
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/src/main/java/com/example/talkeys_new/screens/events/createEvent/CreateEvent1Screen.kt (1)
43-52:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInitialize Step 1 from the shared draft instead of resetting to blanks.
These local
remember { mutableStateOf("") }defaults wipe the visible form whenever this destination is recreated. Since the ViewModel is only updated on Next, users who return to Step 1 see empty fields even when the shared draft already contains their previous organizer info. Seed the local state fromeventCreationViewModel.uiState(or bind directly to it) so navigation and recreation preserve progress.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/com/example/talkeys_new/screens/events/createEvent/CreateEvent1Screen.kt` around lines 43 - 52, CreateEvent1Screen currently initializes local form state (organizerName, emailAddress, contactNumber, organizationName, cityState, socialMediaLinks, selectedFileUri, fileName) with hardcoded empty defaults which clears the UI when the Composable is recreated; instead seed these remember { mutableStateOf(...) } values from the shared draft in eventCreationViewModel.uiState (or bind the fields directly to the ViewModel state) so the Composable reads initial values from eventCreationViewModel.uiState.draft (or similar properties) and updates the ViewModel on Next; update the initialization for organizerName, emailAddress, contactNumber, organizationName, cityState, socialMediaLinks, selectedFileUri and fileName to use the corresponding values from eventCreationViewModel.uiState (falling back to the current defaults only if null) and ensure any file URI type is converted appropriately.app/src/main/java/com/example/talkeys_new/screens/events/createEvent/CreateEvent6Screen.kt (1)
252-268:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftSubmission navigates to success without persisting the event.
On
requestSubmit()success the flow immediatelymarkSubmissionHandled()+resetDraft()and routes toregistration_success, but the actual create-event API call is still a TODO (Line 260). Currently the user sees a success screen even though nothing is persisted. Also note that once the real submission is added, callingresetDraft()here (before the network call returns) would discard the draft prematurely on failure — perform the submit first and only reset/navigate after a confirmed success.Want me to open an issue to track the missing submission API call, or sketch a coroutine-based submit flow that resets/navigates only on confirmed success?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/com/example/talkeys_new/screens/events/createEvent/CreateEvent6Screen.kt` around lines 252 - 268, The current flow calls eventCreationViewModel.requestSubmit(), then immediately markSubmissionHandled(), resetDraft(), and navController.navigate("registration_success") without performing the create-event API call; change this to perform the network submission first and only on confirmed success call markSubmissionHandled(), resetDraft(), and navigate. Concretely: after updateReview(...) start an asynchronous submit (e.g., dispatch to ViewModel via a suspend function or viewModelScope) that invokes the real create-event repository API, set a loading state while awaiting the response, handle errors (do not resetDraft or navigate on failure), and only when the API returns success call eventCreationViewModel.markSubmissionHandled(), eventCreationViewModel.resetDraft(), and then navController.navigate("registration_success"); keep requestSubmit()/updateReview() semantics but move reset/navigate into the success callback and surface errors to the UI.
🧹 Nitpick comments (4)
shared/src/commonTest/kotlin/com/talkeys/shared/presentation/events/EventCreationViewModelTest.kt (2)
10-19: 💤 Low valueConsider moving validator tests to a separate test class.
This test directly invokes
EventCreationValidator.validateOrganizerInfo, but the class is namedEventCreationViewModelTest. Direct validator tests would be better organized in a dedicatedEventCreationValidatorTestclass, keeping this file focused on ViewModel behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@shared/src/commonTest/kotlin/com/talkeys/shared/presentation/events/EventCreationViewModelTest.kt` around lines 10 - 19, The test in EventCreationViewModelTest directly exercises EventCreationValidator.validateOrganizerInfo with OrganizerInfoDraft and should be moved into its own validator-focused test class; create a new EventCreationValidatorTest, relocate this test (and any other direct validator assertions) there, update the test class name and imports accordingly, and keep EventCreationViewModelTest only for ViewModel behavior tests that interact with EventCreationValidator indirectly.
78-121: ⚡ Quick winConsider extracting a complete draft helper and adding negative test case.
This test manually populates all six draft sections (44 lines), making it long and harder to maintain. A helper method like
validCompleteDraft()(similar tovalidOrganizerInfo()) would improve readability and enable reuse.Additionally, consider adding a test that verifies
requestSubmit()returnsfalseand does not setsubmissionRequestedwhen the draft is incomplete.Suggested helper extraction
private fun validCompleteDraft() = CompleteEventDraft( organizer = validOrganizerInfo(), details = EventDetailsDraft( eventName = "Kotlin Meetup", eventType = "Workshop", eventCategory = "Tech", eventDescription = "A long enough event description" ), schedule = EventScheduleDraft( eventDates = "2026-01-01", startTime = "10:00", endTime = "12:00", registrationDeadline = "2025-12-25", maxAttendees = "100", platformUsed = "Google Meet", willBeRecorded = "No" ), // ... rest of sections )Then the test becomes:
`@Test` fun requestSubmit_setsSubmissionRequestedOnlyForCompleteDraft() { val viewModel = EventCreationViewModel() val draft = validCompleteDraft() viewModel.updateOrganizerInfo(draft.organizer) viewModel.updateDetails(draft.details) // ... etc assertTrue(viewModel.requestSubmit()) assertTrue(viewModel.uiState.value.submissionRequested) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@shared/src/commonTest/kotlin/com/talkeys/shared/presentation/events/EventCreationViewModelTest.kt` around lines 78 - 121, Extract a reusable helper that returns a fully populated draft (e.g., validCompleteDraft() returning a CompleteEventDraft composed from validOrganizerInfo() and fully filled EventDetailsDraft, EventScheduleDraft, EventPricingDraft, EventAudienceDraft, EventReviewDraft) and use it in the requestSubmit_setsSubmissionRequestedOnlyForCompleteDraft test to replace the 44-line manual setup when calling EventCreationViewModel.updateOrganizerInfo/updateDetails/updateSchedule/updatePricing/updateAudience/updateReview; also add a negative test that leaves at least one draft section out, asserts requestSubmit() returns false, and verifies uiState.value.submissionRequested remains false and validationErrors is not empty to ensure incomplete drafts don’t trigger submissionRequested.shared/src/commonTest/kotlin/com/talkeys/shared/presentation/events/EventCoordinatorTest.kt (1)
86-89: ⚡ Quick winConsider using an interface for easier testing.
The fake repository extends the concrete
EventsRepositoryclass and passes real dependencies to the constructor. While this works, it couples the test to the concrete implementation.If
EventsRepositoryhad an interface, the test could use a pure fake implementation without depending on realEventsApiandApiClientinstances. This would make tests more isolated and easier to maintain.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@shared/src/commonTest/kotlin/com/talkeys/shared/presentation/events/EventCoordinatorTest.kt` around lines 86 - 89, The test helper repositoryReturning currently constructs an anonymous class extending the concrete EventsRepository and instantiates real dependencies (EventsApi, ApiClient), coupling tests to implementation; change to depend on an interface (e.g., create an EventsRepositoryInterface or IEventsRepository with suspend fun getAllEvents(forceRefresh: Boolean): ApiResult<List<EventSummary>>) and update repositoryReturning to return a simple fake implementation of that interface that returns the provided result, then update EventCoordinatorTest usage to depend on the interface instead of EventsRepository (adjust constructor or DI points that accept EventsRepository to accept the interface).app/src/main/java/com/example/talkeys_new/screens/events/createEvent/CreateEvent4Screen.kt (1)
629-633: ⚡ Quick winPrefer
popBackStack()for backward navigation.
navController.navigate("create_event_3")pushes a new instance of step 3 onto the back stack rather than returning to the existing one, so repeatedly moving back/forward grows the stack and makes the system back button behave unexpectedly. The same pattern is used in the other create-event screens.♻️ Proposed change
onClick = { eventCreationViewModel.goPrevious() - navController.navigate("create_event_3") + navController.popBackStack() },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/com/example/talkeys_new/screens/events/createEvent/CreateEvent4Screen.kt` around lines 629 - 633, The OutlinedButton onClick currently calls navController.navigate("create_event_3") which pushes a new instance instead of returning; change it to use navController.popBackStack() (or popBackStack("create_event_3", false) if you need to ensure target) after calling eventCreationViewModel.goPrevious() so the existing CreateEvent3 screen is returned to and the back stack does not grow; update the OutlinedButton onClick handler in CreateEvent4Screen (the block containing eventCreationViewModel.goPrevious()) accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@app/src/main/java/com/example/talkeys_new/screens/events/createEvent/CreateEvent3Screen.kt`:
- Around line 691-695: The "Previous" button handler currently calls
navController.navigate("create_event_2") which can push a duplicate destination;
replace that navigation call with a back-stack pop so it returns to the existing
step. In the OutlinedButton onClick (where eventCreationViewModel.goPrevious()
is invoked), remove navController.navigate("create_event_2") and call
navController.popBackStack() (or navController.popBackStack("create_event_2",
false) if you need to ensure we pop to that specific route) so the existing
screen is returned to instead of creating a new instance.
In
`@app/src/main/java/com/example/talkeys_new/screens/events/createEvent/CreateEvent5Screen.kt`:
- Around line 55-65: The current URI filename extraction in the
rememberLauncherForActivityResult callback (setting selectedFileName from
uri?.lastPathSegment) is unreliable for content:// URIs; replace that logic by
resolving the display name via ContentResolver querying
OpenableColumns.DISPLAY_NAME (e.g., add a helper like
resolveDisplayName(context, uri) that queries contentResolver.query(...
OpenableColumns.DISPLAY_NAME ...) and returns the column value, falling back to
uri.lastPathSegment.orEmpty()), then set selectedFileName =
resolveDisplayName(context, uri) while keeping selectedFileUri = uri.
- Around line 437-447: The call to selectedFileUri.toSharedImage(...) may
perform blocking I/O on the main thread inside CreateEvent5Screen when building
the EventAudienceDraft and calling eventCreationViewModel.updateAudience;
refactor so URI decoding/copying runs off the UI thread (e.g., make
toSharedImage a suspend function or wrap its work in
withContext(Dispatchers.IO)), await its result, then construct
EventAudienceDraft and call eventCreationViewModel.updateAudience(...) and
navController.navigate("create_event_6") only after the IO completes; ensure
toSharedImage either dispatches to Dispatchers.IO internally or document it as
suspend so callers move work off main.
In
`@shared/src/commonMain/kotlin/com/talkeys/shared/presentation/events/EventCreationValidator.kt`:
- Around line 56-60: In EventCreationValidator, change the Paid-event ticket
price check (where draft.eventType.equals("Paid", ignoreCase = true) and
requireField("ticketPrice", draft.ticketPrice, ...)) so that ticketPrice must
parse to a strictly positive number (> 0.0) rather than >= 0.0; update the
validation condition that currently uses draft.ticketPrice.toDoubleOrNull()?.let
{ it >= 0.0 } to require > 0.0 and adjust the error message returned by
put("ticketPrice", ...) to state that the price must be a positive amount for
Paid events.
In
`@shared/src/commonTest/kotlin/com/talkeys/shared/presentation/events/EventCoordinatorTest.kt`:
- Line 36: The test in EventCoordinatorTest currently asserts an ordered list of
IDs from state.filteredEvents, which can break if ordering changes; update the
assertion to compare the IDs as a collection that ignores order (e.g., convert
state.filteredEvents.map { it.id } to a set and assert equality with the
expected IDs as a set) so the test only verifies filtering, not ordering.
---
Outside diff comments:
In
`@app/src/main/java/com/example/talkeys_new/screens/events/createEvent/CreateEvent1Screen.kt`:
- Around line 43-52: CreateEvent1Screen currently initializes local form state
(organizerName, emailAddress, contactNumber, organizationName, cityState,
socialMediaLinks, selectedFileUri, fileName) with hardcoded empty defaults which
clears the UI when the Composable is recreated; instead seed these remember {
mutableStateOf(...) } values from the shared draft in
eventCreationViewModel.uiState (or bind the fields directly to the ViewModel
state) so the Composable reads initial values from
eventCreationViewModel.uiState.draft (or similar properties) and updates the
ViewModel on Next; update the initialization for organizerName, emailAddress,
contactNumber, organizationName, cityState, socialMediaLinks, selectedFileUri
and fileName to use the corresponding values from eventCreationViewModel.uiState
(falling back to the current defaults only if null) and ensure any file URI type
is converted appropriately.
In
`@app/src/main/java/com/example/talkeys_new/screens/events/createEvent/CreateEvent6Screen.kt`:
- Around line 252-268: The current flow calls
eventCreationViewModel.requestSubmit(), then immediately
markSubmissionHandled(), resetDraft(), and
navController.navigate("registration_success") without performing the
create-event API call; change this to perform the network submission first and
only on confirmed success call markSubmissionHandled(), resetDraft(), and
navigate. Concretely: after updateReview(...) start an asynchronous submit
(e.g., dispatch to ViewModel via a suspend function or viewModelScope) that
invokes the real create-event repository API, set a loading state while awaiting
the response, handle errors (do not resetDraft or navigate on failure), and only
when the API returns success call
eventCreationViewModel.markSubmissionHandled(),
eventCreationViewModel.resetDraft(), and then
navController.navigate("registration_success"); keep
requestSubmit()/updateReview() semantics but move reset/navigate into the
success callback and surface errors to the UI.
---
Nitpick comments:
In
`@app/src/main/java/com/example/talkeys_new/screens/events/createEvent/CreateEvent4Screen.kt`:
- Around line 629-633: The OutlinedButton onClick currently calls
navController.navigate("create_event_3") which pushes a new instance instead of
returning; change it to use navController.popBackStack() (or
popBackStack("create_event_3", false) if you need to ensure target) after
calling eventCreationViewModel.goPrevious() so the existing CreateEvent3 screen
is returned to and the back stack does not grow; update the OutlinedButton
onClick handler in CreateEvent4Screen (the block containing
eventCreationViewModel.goPrevious()) accordingly.
In
`@shared/src/commonTest/kotlin/com/talkeys/shared/presentation/events/EventCoordinatorTest.kt`:
- Around line 86-89: The test helper repositoryReturning currently constructs an
anonymous class extending the concrete EventsRepository and instantiates real
dependencies (EventsApi, ApiClient), coupling tests to implementation; change to
depend on an interface (e.g., create an EventsRepositoryInterface or
IEventsRepository with suspend fun getAllEvents(forceRefresh: Boolean):
ApiResult<List<EventSummary>>) and update repositoryReturning to return a simple
fake implementation of that interface that returns the provided result, then
update EventCoordinatorTest usage to depend on the interface instead of
EventsRepository (adjust constructor or DI points that accept EventsRepository
to accept the interface).
In
`@shared/src/commonTest/kotlin/com/talkeys/shared/presentation/events/EventCreationViewModelTest.kt`:
- Around line 10-19: The test in EventCreationViewModelTest directly exercises
EventCreationValidator.validateOrganizerInfo with OrganizerInfoDraft and should
be moved into its own validator-focused test class; create a new
EventCreationValidatorTest, relocate this test (and any other direct validator
assertions) there, update the test class name and imports accordingly, and keep
EventCreationViewModelTest only for ViewModel behavior tests that interact with
EventCreationValidator indirectly.
- Around line 78-121: Extract a reusable helper that returns a fully populated
draft (e.g., validCompleteDraft() returning a CompleteEventDraft composed from
validOrganizerInfo() and fully filled EventDetailsDraft, EventScheduleDraft,
EventPricingDraft, EventAudienceDraft, EventReviewDraft) and use it in the
requestSubmit_setsSubmissionRequestedOnlyForCompleteDraft test to replace the
44-line manual setup when calling
EventCreationViewModel.updateOrganizerInfo/updateDetails/updateSchedule/updatePricing/updateAudience/updateReview;
also add a negative test that leaves at least one draft section out, asserts
requestSubmit() returns false, and verifies uiState.value.submissionRequested
remains false and validationErrors is not empty to ensure incomplete drafts
don’t trigger submissionRequested.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 753c149e-ff3f-424c-afe0-63f6079e8c9a
📒 Files selected for processing (21)
app/src/main/java/com/example/talkeys_new/navigation/AppNavigation.ktapp/src/main/java/com/example/talkeys_new/screens/events/AndroidEventPlatformRequestHandler.ktapp/src/main/java/com/example/talkeys_new/screens/events/EventMediatedViewModel.ktapp/src/main/java/com/example/talkeys_new/screens/events/SharedEventsViewModelProvider.ktapp/src/main/java/com/example/talkeys_new/screens/events/createEvent/AndroidSharedImageReader.ktapp/src/main/java/com/example/talkeys_new/screens/events/createEvent/CreateEvent1Screen.ktapp/src/main/java/com/example/talkeys_new/screens/events/createEvent/CreateEvent2Screen.ktapp/src/main/java/com/example/talkeys_new/screens/events/createEvent/CreateEvent3Screen.ktapp/src/main/java/com/example/talkeys_new/screens/events/createEvent/CreateEvent4Screen.ktapp/src/main/java/com/example/talkeys_new/screens/events/createEvent/CreateEvent5Screen.ktapp/src/main/java/com/example/talkeys_new/screens/events/createEvent/CreateEvent6Screen.ktapp/src/main/java/com/example/talkeys_new/screens/events/mediator/EventMediator.ktapp/src/main/java/com/example/talkeys_new/screens/events/mediator/EventMediatorImpl.ktapp/src/main/java/com/example/talkeys_new/screens/events/mediator/EventMediatorProvider.ktshared/src/commonMain/kotlin/com/talkeys/shared/presentation/events/EventCoordinator.ktshared/src/commonMain/kotlin/com/talkeys/shared/presentation/events/EventCreationModels.ktshared/src/commonMain/kotlin/com/talkeys/shared/presentation/events/EventCreationValidator.ktshared/src/commonMain/kotlin/com/talkeys/shared/presentation/events/EventCreationViewModel.ktshared/src/commonMain/kotlin/com/talkeys/shared/presentation/events/EventsViewModelFactories.ktshared/src/commonTest/kotlin/com/talkeys/shared/presentation/events/EventCoordinatorTest.ktshared/src/commonTest/kotlin/com/talkeys/shared/presentation/events/EventCreationViewModelTest.kt
💤 Files with no reviewable changes (4)
- app/src/main/java/com/example/talkeys_new/screens/events/mediator/EventMediatorProvider.kt
- app/src/main/java/com/example/talkeys_new/screens/events/mediator/EventMediator.kt
- app/src/main/java/com/example/talkeys_new/screens/events/mediator/EventMediatorImpl.kt
- app/src/main/java/com/example/talkeys_new/screens/events/EventMediatedViewModel.kt
Comment on lines
691
to
695
| OutlinedButton( | ||
| onClick = { | ||
| eventCreationViewModel.goPrevious() | ||
| navController.navigate("create_event_2") | ||
| }, |
There was a problem hiding this comment.
❓ Verification inconclusive
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and print the relevant section around the referenced lines
FILE="app/src/main/java/com/example/talkeys_new/screens/events/createEvent/CreateEvent3Screen.kt"
if [ ! -f "$FILE" ]; then
echo "Missing file: $FILE"
exit 1
fi
echo "=== file line count ==="
wc -l "$FILE"
echo "=== snippet around lines 660-720 ==="
sed -n '650,720p' "$FILE" | nl -ba | sed -n '1,120p'
echo "=== find usages of create_event_2, goPrevious, navigate, popBackStack in CreateEvent3Screen.kt ==="
rg -n "create_event_2|goPrevious\(|popBackStack\(|navController\.navigate\(" "$FILE" || true
echo "=== find navigation controller setup / routes in this file ==="
rg -n "NavController|composable|route\s*=|navigate\(|popBackStack\(" "$FILE" || true
# Also search for the destination route string and how it’s defined elsewhere
echo "=== search for route 'create_event_2' across repo ==="
rg -n "\"create_event_2\"|create_event_2" app/src/main/java || true
echo "=== search for screen CreateEvent2 or composable for route create_event_2 ==="
rg -n "CreateEvent2|createEvent_2|create_event_2" app/src/main/java || trueRepository: gurnoorpannu/Talkeys_Official
Length of output: 1980
Use back-stack pop semantics for the “Previous” button here.
Previous currently calls navController.navigate("create_event_2"), which can push another copy of the wizard step onto the back stack instead of returning to the existing one, breaking expected Back behavior.
Minimal fix
onClick = {
eventCreationViewModel.goPrevious()
- navController.navigate("create_event_2")
+ navController.popBackStack()
},🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@app/src/main/java/com/example/talkeys_new/screens/events/createEvent/CreateEvent3Screen.kt`
around lines 691 - 695, The "Previous" button handler currently calls
navController.navigate("create_event_2") which can push a duplicate destination;
replace that navigation call with a back-stack pop so it returns to the existing
step. In the OutlinedButton onClick (where eventCreationViewModel.goPrevious()
is invoked), remove navController.navigate("create_event_2") and call
navController.popBackStack() (or navController.popBackStack("create_event_2",
false) if you need to ensure we pop to that specific route) so the existing
screen is returned to instead of creating a new instance.
Comment on lines
+55
to
+65
| val filePickerLauncher = rememberLauncherForActivityResult( | ||
| contract = ActivityResultContracts.GetContent() | ||
| ) { uri -> | ||
| selectedFileUri = uri | ||
| selectedFileName = uri?.lastPathSegment | ||
| ?.split("/") | ||
| ?.lastOrNull() | ||
| ?.split("?") | ||
| ?.firstOrNull() | ||
| ?: "" | ||
| } |
There was a problem hiding this comment.
Unreliable display name from lastPathSegment.
For content:// URIs returned by GetContent, lastPathSegment is typically an opaque document/row id rather than a real filename, so the shown name is often meaningless. Query OpenableColumns.DISPLAY_NAME via ContentResolver to get the actual file name.
🛠️ Suggested approach
fun resolveDisplayName(context: Context, uri: Uri): String {
context.contentResolver.query(uri, arrayOf(OpenableColumns.DISPLAY_NAME), null, null, null)?.use { c ->
if (c.moveToFirst()) return c.getString(0)
}
return uri.lastPathSegment.orEmpty()
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@app/src/main/java/com/example/talkeys_new/screens/events/createEvent/CreateEvent5Screen.kt`
around lines 55 - 65, The current URI filename extraction in the
rememberLauncherForActivityResult callback (setting selectedFileName from
uri?.lastPathSegment) is unreliable for content:// URIs; replace that logic by
resolving the display name via ContentResolver querying
OpenableColumns.DISPLAY_NAME (e.g., add a helper like
resolveDisplayName(context, uri) that queries contentResolver.query(...
OpenableColumns.DISPLAY_NAME ...) and returns the column value, falling back to
uri.lastPathSegment.orEmpty()), then set selectedFileName =
resolveDisplayName(context, uri) while keeping selectedFileUri = uri.
Comment on lines
+437
to
447
| eventCreationViewModel.updateAudience( | ||
| EventAudienceDraft( | ||
| communityChat = communityChat, | ||
| sponsors = sponsors, | ||
| audienceType = audienceType, | ||
| sponsorDeck = selectedFileUri.toSharedImage(context, selectedFileName) | ||
| ) | ||
| ) | ||
| if (eventCreationViewModel.goNext()) { | ||
| navController.navigate("create_event_6") | ||
| } |
There was a problem hiding this comment.
❓ Verification inconclusive
🏁 Script executed:
#!/bin/bash
fd -i 'AndroidSharedImageReader.kt' --exec cat {}Repository: gurnoorpannu/Talkeys_Official
Length of output: 2009
🏁 Script executed:
rg -n "toSharedImage\\b" app/src/main/java -SRepository: gurnoorpannu/Talkeys_Official
Length of output: 1980
Ensure toSharedImage isn’t doing blocking I/O on the UI thread (CreateEvent5Screen.kt, 437-447)
selectedFileUri.toSharedImage(...) is executed in the click handler. If it reads from ContentResolver/streams synchronously on the calling thread, it can block UI and trigger jank/ANR. Move the URI-to-image decoding/copying off the main thread (e.g., withContext(Dispatchers.IO) or a suspend function), then update EventAudienceDraft and navigate after completion.
Confirm toSharedImage’s implementation doesn’t perform blocking file/ContentResolver reads on the caller thread (or explicitly dispatches internally to Dispatchers.IO).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@app/src/main/java/com/example/talkeys_new/screens/events/createEvent/CreateEvent5Screen.kt`
around lines 437 - 447, The call to selectedFileUri.toSharedImage(...) may
perform blocking I/O on the main thread inside CreateEvent5Screen when building
the EventAudienceDraft and calling eventCreationViewModel.updateAudience;
refactor so URI decoding/copying runs off the UI thread (e.g., make
toSharedImage a suspend function or wrap its work in
withContext(Dispatchers.IO)), await its result, then construct
EventAudienceDraft and call eventCreationViewModel.updateAudience(...) and
navController.navigate("create_event_6") only after the IO completes; ensure
toSharedImage either dispatches to Dispatchers.IO internally or document it as
suspend so callers move work off main.
Comment on lines
+56
to
+60
| if (draft.eventType.equals("Paid", ignoreCase = true)) { | ||
| requireField("ticketPrice", draft.ticketPrice, "Ticket price is required") | ||
| if (draft.ticketPrice.isNotBlank() && draft.ticketPrice.toDoubleOrNull()?.let { it >= 0.0 } != true) { | ||
| put("ticketPrice", "Ticket price must be a valid amount") | ||
| } |
There was a problem hiding this comment.
ticketPrice >= 0.0 permits a price of 0 for a Paid event.
For eventType == "Paid", a ticket price of 0 passes validation, which contradicts the paid selection. Consider requiring a strictly positive amount.
🛠️ Proposed fix
- if (draft.ticketPrice.isNotBlank() && draft.ticketPrice.toDoubleOrNull()?.let { it >= 0.0 } != true) {
+ if (draft.ticketPrice.isNotBlank() && draft.ticketPrice.toDoubleOrNull()?.let { it > 0.0 } != true) {
put("ticketPrice", "Ticket price must be a valid amount")
}📝 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
| if (draft.eventType.equals("Paid", ignoreCase = true)) { | |
| requireField("ticketPrice", draft.ticketPrice, "Ticket price is required") | |
| if (draft.ticketPrice.isNotBlank() && draft.ticketPrice.toDoubleOrNull()?.let { it >= 0.0 } != true) { | |
| put("ticketPrice", "Ticket price must be a valid amount") | |
| } | |
| if (draft.eventType.equals("Paid", ignoreCase = true)) { | |
| requireField("ticketPrice", draft.ticketPrice, "Ticket price is required") | |
| if (draft.ticketPrice.isNotBlank() && draft.ticketPrice.toDoubleOrNull()?.let { it > 0.0 } != true) { | |
| put("ticketPrice", "Ticket price must be a valid amount") | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@shared/src/commonMain/kotlin/com/talkeys/shared/presentation/events/EventCreationValidator.kt`
around lines 56 - 60, In EventCreationValidator, change the Paid-event ticket
price check (where draft.eventType.equals("Paid", ignoreCase = true) and
requireField("ticketPrice", draft.ticketPrice, ...)) so that ticketPrice must
parse to a strictly positive number (> 0.0) rather than >= 0.0; update the
validation condition that currently uses draft.ticketPrice.toDoubleOrNull()?.let
{ it >= 0.0 } to require > 0.0 and adjust the error message returned by
put("ticketPrice", ...) to state that the price must be a positive amount for
Paid events.
|
|
||
| val state = coordinator.uiState.value | ||
| assertEquals(3, state.allEvents.size) | ||
| assertEquals(listOf("1", "2"), state.filteredEvents.map { it.id }) |
There was a problem hiding this comment.
Avoid assuming event ordering in assertions.
The assertion compares filteredEvents as an ordered list. If the implementation changes event ordering (e.g., sorts by name instead of ID), this test will fail even when the filtering logic is correct.
Consider comparing as a set to ignore ordering:
assertEquals(setOf("1", "2"), state.filteredEvents.map { it.id }.toSet())🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@shared/src/commonTest/kotlin/com/talkeys/shared/presentation/events/EventCoordinatorTest.kt`
at line 36, The test in EventCoordinatorTest currently asserts an ordered list
of IDs from state.filteredEvents, which can break if ordering changes; update
the assertion to compare the IDs as a collection that ignores order (e.g.,
convert state.filteredEvents.map { it.id } to a set and assert equality with the
expected IDs as a set) so the test only verifies filtering, not ordering.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary by CodeRabbit
New Features
Refactor
Tests