android/nowinandroid

migrations = listOf(
    IntToStringIdsMigration,
),

ListToMapMigration exists and has unit tests, but nothing in the code registers it (searched the whole repository; it is used only in its own file and its tests). The app reads only the map fields. Users whose stored data still uses the older list fields keep their followed topics, authors and bookmarks there, and that data silently disappears from the app. This is loss of user data on upgrade.

Register it after the existing migration: listOf(IntToStringIdsMigration, ListToMapMigration). Add a test that runs the real DataStore factory with a legacy file.

newsResourceFtsDao.insertAll(
    newsResourceDao.getNewsResources(useFilterTopicIds = false, useFilterNewsIds = false)
        .first().map(PopulatedNewsResource::asFtsEntity)

The full-text entities have no stable row key, so "replace on conflict" never fires. Each sync appends a full copy of every article and topic, and nothing ever deletes the old rows. Storage grows without bound, and counts include the duplicates.

Give the full-text rows a stable rowid, or clear and rebuild both tables in one transaction. Skip the rebuild when nothing changed.

syncSubscriber.subscribe()

// First sync the repositories in parallel

The subscription runs before any data is fetched and is not guarded. On a device without Play Services, or after a transient messaging error, the worker fails and no content syncs.

Wrap the call in a try/catch that rethrows cancellation and logs everything else, or run it after the data sync.

customTabsIntent.launchUrl(context, uri)

The URI comes straight from server data (Uri.parse(userNewsResource.url) at NewsFeed.kt:74, NewsResourceCardList.kt:46 and ForYouScreen.kt:479). The ForYouScreen.kt call runs automatically from a deep link, with no tap. If the device or profile has no browser, ActivityNotFoundException is not caught, so the app crashes on the main reading path. A non-web scheme in the data is handed to whatever app claims it.

Before launching, accept only http and https (and ideally check the host). Resolve the intent or catch ActivityNotFoundException, and fall back to a snackbar.

@Query("SELECT topicId FROM topicsFts WHERE topicsFts MATCH :query")

The caller builds the query as "*$searchQuery*". Quotes or FTS operators typed by the user produce a malformed expression, which shows up only as a generic load-failed state. NewsResourceFtsDao.kt:34 has the same pattern.

Escape or quote user input in one helper before it reaches MATCH.

}.orElse("http://example.com")

A build without local.properties (for example on CI) compiles successfully but points the prod network layer at a placeholder http host. Cleartext traffic is blocked on modern Android, and sync errors are only logged (finding 11), so the misconfiguration is invisible.

Fail prod and release builds when BACKEND_URL is absent, require https://, and keep a placeholder only for explicitly local builds.

Log.i("suspendRunCatching", "Failed to evaluate a suspendRunCatchingBlock. Returning failure Result", exception)

A broken backend looks healthy. Users see stale content with no explanation, and permanent errors are retried just like transient ones.

Log at warning or error level and report the failure. Record the last successful sync time and show stale data in the UI. Tell permanent errors apart from transient ones.

val headerImageUrl: String,

The database entity, the domain model and the card all treat the header image as optional. A single item with no image causes a deserialization error, the news fetch fails, and the worker retries forever on the same payload.

Declare val headerImageUrl: String? = null. Give other non-critical fields defaults too, and consider decoding item by item so one bad record is skipped.

suspend fun setThemeBrand(themeBrand: ThemeBrand) {
    userPreferences.updateData {

Some write methods catch IOException (lines 62–74 and 123–137). setThemeBrand, setDynamicColorPreference, setDarkThemeConfig, setNewsResourcesViewed and setShouldHideOnboarding do not. SettingsViewModel.kt:57–71 calls them in viewModelScope.launch with no exception handler, so a disk-write failure while changing a setting crashes the app.

Wrap every updateData call in one shared helper that handles IOException the same way.

} catch (ioException: IOException) {
    Log.e("NiaPreferences", "Failed to update user preferences", ioException)
}

The sync helper treats the version update as done and returns success. On the next run everything is fetched again, and the first-sync path marks every article as viewed again.

Let the exception propagate so the sync reports failure and is retried, or have the update return a success flag that the caller checks.

shouldDisplayUndoBookmark = true
lastRemovedBookmarkId = newsResourceId
userDataRepository.setNewsResourceBookmarked(newsResourceId, false)

If the user removes A and then B while the snackbar for A is still showing, the flag is already true, so no new snackbar appears. Undo then restores only B, and A's removal can no longer be undone.

Keep the removed ids in a list (or restart the snackbar for each removal), and make undo restore the item the snackbar refers to.

// TODO: Abstract the signing configuration to a separate file to avoid hardcoding this.
signingConfig = signingConfigs.named("debug").get()

The release workflow publishes an APK built this way. Anyone with the widely known debug key can produce an "update" that installs over it. The comment says this is a deliberate convenience, but it should never apply to published artifacts.

Sign releases with a dedicated keystore read from CI secrets. Allow the debug-key fallback only behind an explicit local-only flag, and make CI fail when a release is debug-signed.

android.useAndroidX=true

No android.enableJetifier entry exists in any properties, Gradle or TOML file. The intended "off" state relies on the build tool's default, which can change or be toggled by accident and slow builds.

Add android.enableJetifier=false next to android.useAndroidX=true.

<activity android:name=".MainActivity" android:configChanges="uiMode" android:exported="true">

The activity declares a browsable https filter but sets no launch mode and does not handle repeated intents idempotently. A notification tap and a link tap can stack instances or process the same link twice.

Use singleTop (or singleTask) and handle onNewIntent idempotently by remembering the last handled link. Add android:autoVerify="true" for App Links.

"current_key": "APla…"

This is a placeholder API key (the project id is a dummy). No real credentials or private keys were found anywhere in the code.

No action needed. For production builds, inject the real file from CI secrets and keep it out of the repository.

LaunchedEffect(Unit) { snapshotFlow { draggedOffset }.collect { ... onThumbMoved(currentTravel) ...

The press-to-scroll loop (around line 371) and the drag handler both call onThumbMoved. If the user presses the track and then drags during the press-scroll window, both write the position and the thumb can jitter.

Keep one gesture state (idle, pressing or dragging) and cancel the press loop when a drag starts. Use collectLatest for the press loop.

androidxComposeFoundation = "1.8.0-alpha07"

The BOM resolves a newer version anyway, so the pin misstates which API surface the code builds against. Both come from the alpha channel.

Let the BOM manage Foundation (drop the explicit version). Use the stable BOM for release builds, or document why alpha is needed.

suspend fun setShouldHideOnboarding(shouldHideOnboarding: Boolean) {
    userPreferences.updateData {
        it.copy { this.shouldHideOnboarding = shouldHideOnboarding }

Two other writers re-check this rule (lines 68 and 85), but this one does not. The stored pair can disagree with what the For You screen and the news sync expect.

Send every writer of these two fields through one function that enforces the rule, and document the rule next to the stored fields.

TopicEntity(
    id = topicId,
    name = "",

News sync inserts shell topic rows to satisfy the foreign key. No reader filters them, so if the topic sync never fills a row in, a blank topic that can be followed appears in the topics list.

Add an explicit placeholder flag (or filter blank names in readers), and check after a full sync that no placeholder rows remain.

val networkTopics = network.getTopics(ids = changedIds)

News sync fetches in chunks, but topic sync sends every changed id in one request. The HTTP client also sets no explicit timeouts and no response-size limit.

Fetch topics in chunks the same way news is fetched, cap the size of accepted change lists and responses, and set OkHttp timeouts.

private const val MAX_NUM_NOTIFICATIONS = 5

The notification cap, the sync batch size and the minimum search length can only change with an app release.

If these should be tunable, serve them from versioned remote config. Otherwise, document them as deliberate client constants.

.setContentTitle(getString(R.string.sync_work_notification_title))

A fix to the wording of a notification or other copy requires an app update.

Only if you need it: resolve strings through a helper that checks remote overrides first and falls back to the bundled resources.

started = SharingStarted.WhileSubscribed(5_000),

The same policy is written as 5_000 in about 14 places across 8 files, and as 5.seconds.inWholeMilliseconds in SettingsViewModel. Changing it means editing every copy, and one is easy to miss.

Define one shared constant or stateIn helper in a core module and use it everywhere.

sealed interface TopicUiState { data class Success(...); data object Error; data object Loading }

About eight screens repeat this skeleton. It is likely intentional, but nothing records that decision.

Document the per-screen choice, or introduce a shared generic type for the simple cases.

- name: Upload app
  uses: actions/upload-release-asset@v1

People who download the APK cannot check that it is intact.

Publish a SHA-256 hash and the file size next to the APK. The same applies to the copies made by build_android_release.sh.

- name: Build release variant including baseline profile generation
  run: ./gradlew :app:assembleDemoRelease ...

A tag on a commit that CI never validated still produces a public release.

Run the tests and a smoke check before the release is created, or make the release job depend on a green build of the same commit.

cp $APP_OUT/apk/prod/release/app-prod-release.apk $DIST_DIR/app-prod-release.apk

After a failed build, the script still copies the files, and it silently overwrites earlier artifacts. The variables are unquoted.

Check that DIST_DIR is set, and copy only when the build succeeded. Refuse to overwrite existing files, or ask first. Quote the variables.