Add the Ingredients and Areas tabs - #16
Merged
Merged
Conversation
* **Domain**:
* Created `IngredientsProducer` to wrap `IngredientRepository` and emit `StoreReadResponse` state via `produceRetainedState`.
* Added `IngredientsPresenter` to map store responses into `IngredientsState` (Loading, Success, Error) and manage retry triggers.
* Handled `IngredientClicked` events to navigate to `RecipesScreen.ByIngredient`.
* **UI**:
* Added `IngredientsScreen` to render the state, displaying ingredients in an adaptive `LazyVerticalGrid`.
* Implemented loading spinners, an error/retry fallback, and empty state handling.
* Created `IngredientItem` using `AppCard` and Coil's `AsyncImage` to display scaled category thumbnails.
Signed-off-by: Scott Olcott <scottolcott@gmail.com>
* **Domain**:
* Added `AreasPresenter` and `AreasProducer` to manage UI state and Store5 responses for the new screen.
* **Network & Model**:
* Added `Area` models, DTOs, and `AreaApi` to fetch supported regions from the network.
* **Repository**:
* Added `AreaRepositoryImpl` backed by Store5 to orchestrate network fetching, local caching, and filtering.
* **Storage**:
* Added `AreaEntity` and `AreaDao` to the Room database, updating the schema.
* Added `AreasFetchHistoryDataStore` to track fetch times and manage cache invalidation, wired up across all four platform `StorageFactory` implementations.
* **UI**:
* Added `AreasScreen` displaying a responsive grid of supported areas.
Signed-off-by: Scott Olcott <scottolcott@gmail.com>
Replaced the `no_categories_found` string resource with `no_ingredients_found`, updated the LazyVerticalGrid `contentType` from `"category_item"` to `"ingredient_item"`, and renamed the `category` and `onCategoryClick` parameters in `IngredientItem` to match the ingredient type. Signed-off-by: Scott Olcott <scottolcott@gmail.com>
Replaced the `no_categories_found` string resource with `no_ingredients_found`, updated the LazyVerticalGrid `contentType` from `"category_item"` to `"ingredient_item"`, and renamed the `category` and `onCategoryClick` parameters in `IngredientItem` to match the ingredient type. Signed-off-by: Scott Olcott <scottolcott@gmail.com>
* **Network**:
* Updated `RecipeApi.getByArea` to accept an optional `country` parameter. If provided, it fetches both `a=area` and `a=country` endpoints and returns the distinct merged recipes.
* **Repository**:
* Updated `RecipeRepository.recipesByArea` to first query `AreaRepository.getArea()`. If the area has a mapped country, both strings are passed to the updated `RecipesKey.ByArea` to fetch the combined results.
* Added `GetArea` to `AreasKey`, `AreaRepository`, and `AreaDao` to support single-area lookups.
* **Storage**:
* Simplified `AreasFetchHistory` and `AreasFetchHistoryDataStore` to track a single global `lastFetchTime` instead of a map keyed by `AreasKey`. Every area fetch calls the same `api.getAreas()` endpoint and writes the full list to the database, making per-key expiration redundant.
* **UI**:
* Renamed the implicit `it` parameter to `area` in the `AreasScreen` lazy list for clarity.
Signed-off-by: Scott Olcott <scottolcott@gmail.com>
`IngredientsFetchHistory` previously tracked fetch times in a `Map<IngredientsKey, Instant>`, requiring garbage collection logic to prune expired entries. This has been simplified to store only a single global `lastFetchTime`.
* **Storage**:
* Replaced the `lastFetchTimes` map in `IngredientsFetchHistory` with a single nullable `lastFetchTime: Instant?`.
* Removed the `IngredientsKey` and `expirationThreshold` arguments from `updateLastFetchTime`, `getLastFetchTime`, and `refreshNeeded`.
* Updated KDocs in `IngredientsFetchHistory` and `AreasFetchHistory` to remove references to `[key]`.
* **Repository**:
* Updated `IngredientRepository` to drop `IngredientsKey` and the cache expiration threshold when writing to or reading from the fetch history.
Signed-off-by: Scott Olcott <scottolcott@gmail.com>
* Bump `composeAdaptive` from 1.3.0-beta02 to 1.3.0-rc01. * Bump `kermit` from 2.1.0 to 2.2.0. * Bump `store` from 5.1.0-alpha11 to 5.1.0-beta01. * Remove `kermit-android-debug` and `kermit-core-android-debug` dependencies from the version catalog and `app/build.gradle.kts` debug configurations. Signed-off-by: Scott Olcott <scottolcott@gmail.com>
* **Network**:
* Moved the `libs.kermit.ktor` dependency from `nonJvmMain` to the common source set.
* Removed the `expect`/`actual` declarations for `provideKtorLogger` and consolidated the `KermitKtorLogger` instantiation into `commonMain`.
* Deleted `NetworkProviders.nonJvm.kt` and removed the corresponding `jvmMain` actual implementation.
* Removed the unused `runtimeConfig` parameter from `provideCoilKtorClient`.
Signed-off-by: Scott Olcott <scottolcott@gmail.com>
ktfmt-gradle 0.27.0 registers no task for any `androidMain` source set in
these modules, so `src/androidMain` was checked by nothing at all and drifted
out of Google style unnoticed.
Two of its behaviours combine to produce that. Its KMP handler skips every
source set named `android*` unless `com.android.kotlin.multiplatform.library`
is applied -- but it runs from `plugins.withId("org.jetbrains.kotlin.multiplatform")`,
which fires the moment the Kotlin plugin is applied, one line before
`kmp.library` applies the AGP plugin it looks for. The Android handler it
defers to then returns early, because the KMP library plugin's extension is
not the `CommonExtension` that handler requires.
Register the missing format and check tasks in the `formatting` convention
plugin, filtered by ktfmt's own source-set exclusion pattern so KSP output
under `build/` is left alone, and hang them off `ktfmtFormat`/`ktfmtCheck` so
`/verify` covers every source set again.
The three reformatted files are the backlog that gap accumulated.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018NME4RvM9ESMNfA3HReTKG
`list.php?a=list` declares `strCountry` nullable in the v2 schema, and `NetworkJson` sets `ignoreUnknownKeys` but not `coerceInputValues` -- which tolerates unknown keys, not a null in a non-null slot. A single row without a country would therefore fail the decode of all 195 and empty the Areas tab. Every row carries one today, so this is hardening rather than a fix for an observed failure. `AreaItem` keeps the country's box laid out when there is nothing to put in it: the grid sizes a row to its tallest card, so a card that shed a line would leave a gap under every one of its neighbours. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018NME4RvM9ESMNfA3HReTKG
The two `filter.php?a=` calls ran one after the other, and the shared client sets `expectSuccess`, so a 404 on the country request threw away an area request that had already succeeded -- the more likely of the two to have results, since `filter.php` matches the `strArea` string meals actually carry. Run them together, wrap the country call so its failure is a missing contribution rather than a lost screen, and skip it entirely when the country is the same word as the area, which it is for a good share of the list. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018NME4RvM9ESMNfA3HReTKG
`recipesByArea` drove the recipe Store's key off `areaRepository.getArea()`. On a cold areas cache that flow emits `Data(null)` first, producing `ByArea(area, null)` and starting a recipe fetch, then `Data(Area)`, producing `ByArea(area, "United Kingdom")` -- which `flatMapLatest` uses to cancel the first stream and fetch again. Two round trips, a visible loading flicker, and two entries in the recipe fetch-history for one screen. Take `country` back out of `RecipesKey.ByArea` and look it up once, suspending, inside the fetcher where the request is actually built. `recipesByArea` returns to the single-key shape every other reader in this class uses. `countryFor` waits on the first *settled* response rather than the first non-null area: a Store stream never completes, so waiting for the area itself would hang forever on one the list does not contain. That leaves `AreasKey`, `getArea`, `filterAreasByName` and half of `AreaDao` with no callers, so they go too. With `GetAll` the only key left, `Unit` says what a sealed type with one member was saying less directly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018NME4RvM9ESMNfA3HReTKG
`AreaDao.insert` is `OnConflictStrategy.REPLACE`, which updates rows the list still contains but never removes one it has dropped, so an area withdrawn upstream would stay in the grid forever. `CategoryRepositoryImpl` clears its table before inserting for the same reason. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018NME4RvM9ESMNfA3HReTKG
Categories, Ingredients and Areas each carried the same fifty lines: the retained last-known list, the `StoreReadResponse` `when`, and a third copy of the `Error.Custom` TODO nobody has answered yet. `rememberListUi` holds that mapping once, over a `ListUi` that is deliberately not a `CircuitUiState` -- Circuit pairs a state with exactly one UI, so each tab keeps its own state type and only the translation is shared. `Loading`, `Content(items, isRefreshing)` and `Failure(message)` are the three cases they all had. `produceStoreState` does the same for the producers, which differed only in which repository call they collected. The producer classes stay: they are the `@Inject` seam between `:domain` and `:repository` and the presenters take them as constructor parameters. `CategoriesProducer` becomes internal to match its two siblings; `commonTest` is in the same module and still constructs it directly. `rememberUpdatedState` in `produceStoreState` is what the compose-rules `LambdaParameterInRestartableEffect` check asks for: the effect restarts on `retryTrigger` alone, so it must not close over the lambda it was first composed with. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018NME4RvM9ESMNfA3HReTKG
Both tabs shipped without tests while `CategoriesPresenterTest` covered the identical presenter next to them, so these mirror it: loading, success, error, retry, and the navigation each click produces. `refreshKeepsPreviousAreas` is the one case Categories does not cover, and it is the behaviour `rememberListUi` exists for -- a return to `Loading` renders as `isRefreshing` over the list already on screen rather than dropping back to a spinner. It drives the responses one at a time through a `MutableSharedFlow` because a flow that emits both up front collapses into a single recomposition, which hides the transition entirely. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018NME4RvM9ESMNfA3HReTKG
`IngredientScreen` was the odd one out beside `CategoriesScreen` and `AreasScreen`. Sharing a name with the imported `IngredientsScreen` data object is fine -- the other two already do, and `@CircuitInject` resolves the class. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018NME4RvM9ESMNfA3HReTKG
`list.php?a=list` is no longer "not wired" -- it backs `AreasResource` and returns records with both `strArea` and `strCountry`, not the bare names the table claimed. Also note the trap that shaped `getByArea`: the list returns ~195 rows with a real key, while `filter.php?a=` only matches the exact `strArea` string meals carry, 14 distinct values on v1. Most rows in the Areas tab therefore have no recipes behind them, and asking for both spellings recovers some but not all of that gap. Worth knowing before treating an empty area as a bug in the screen. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018NME4RvM9ESMNfA3HReTKG
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
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.
Adds the Ingredients and Areas Home tabs end to end, then a follow-up pass that
fixes what a review against
mainturned up.The feature
AreaEntity/AreaDao, anAreasFetchHistoryDataStore,AreaApionlist.php?a=list,AreaRepository, presenters and producers for both tabs, andtwo grid screens. Ingredient fetch history collapses to a single timestamp,
matching the fact that its fetcher ignores the key and always pulls the whole
list. The Ktor logger moves to
commonMainnow that Kermit 2.2.0 no longerneeds the per-platform split.
Fixes on top
Browse-by-area fetched twice.
recipesByAreadrove the recipe Store's keyoff
areaRepository.getArea(). On a cold areas cache that flow emitsData(null)first, producingByArea(area, null)and starting a fetch, thenData(Area), producingByArea(area, "United Kingdom")— whichflatMapLatestuses to cancel the first stream and fetch again. Two round trips, a loading
flicker, and two entries in the fetch-history for one screen.
countrycomesback out of the key and is looked up once, suspending, inside the fetcher.
Verified against the running app: one
filter.php?a=Britishand onefilter.php?a=United+Kingdom, issued together, no second round.The two
filter.phpcalls ran sequentially, and since the client setsexpectSuccess, a 404 on the country request discarded an area request that hadalready succeeded. Now concurrent, with the country call best-effort and skipped
when it is the same word as the area.
strCountryis nullable in the v2 schema andNetworkJsonhas nocoerceInputValues, so one null row would have failed the decode of all 195.Every row carries one today — this is hardening, not a fix for an observed
failure.
The areas writer upserted without evicting, so an area dropped upstream would
have stayed in the grid forever.
ktfmt never covered
androidMainWorth a look on its own. ktfmt-gradle 0.27.0 registers no task for any
androidMainsource set in these modules: its KMP handler skipsandroid*source sets unless
com.android.kotlin.multiplatform.libraryis applied, but itruns from
plugins.withId("org.jetbrains.kotlin.multiplatform"), which fires oneline before
kmp.libraryapplies that plugin; the Android handler it defers tothen returns early on an extension type it does not recognise. Two of the three
files it had let drift predate this branch. The
formattingconvention pluginnow registers the missing tasks, filtered by ktfmt's own exclusion pattern so KSP
output under
build/is left alone.Cleanup
Categories, Ingredients and Areas had become three copies of one design.
rememberListUiholds theStoreReadResponse→ Loading/Content/Failure mappingonce — deliberately not a
CircuitUiState, since Circuit pairs a state withexactly one UI, so each tab keeps its own state type and only the translation is
shared.
produceStoreStatedoes the same for the producers. Net −320/+128, andthe
Error.CustomTODO exists in one place instead of three.Known limitation
list.php?a=listreturns ~195 areas, butfilter.php?a=only matches the exactstrAreastring meals actually carry — 14 distinct values on v1. Most rows inthe Areas tab therefore have no recipes behind them, and asking for both
spellings recovers some but not all of that gap. Recorded in the
themealdb-apiskill so an empty area is not mistaken for a bug in the screen. Deciding what the
tab should show is left for a follow-up.
Verification
ktfmtCheck+detektAllclean,allTestsgreen (85 tests, 11 new coveringboth new presenters), all six targets for every library module, plus
:app:assembleDebug,:desktopApp:buildand:webApp:buildso every Metro graphis exercised. Each of the nine commits was compile-checked individually. Manually
walked the desktop app from a wiped database: all three tabs populate, British
returns 15 recipes, back restores from cache, no UI errors.
🤖 Generated with Claude Code
https://claude.ai/code/session_018NME4RvM9ESMNfA3HReTKG