Skip to content

Review fixes, new settings and drawer wordmark [minor] - #26

Merged
bardesss merged 10 commits into
mainfrom
review-fixes
Nov 1, 2026
Merged

bardesss merged 10 commits into
mainfrom
review-fixes

Conversation

@bardesss

@bardesss bardesss commented Nov 1, 2026

Copy link
Copy Markdown
Owner

Fixes from a full review of the repo, plus a list bug and two UI changes from device testing.

Security and CI

  • ci.yml actions are pinned to commit SHAs, with the version in a comment on the same line.
  • release.yml no longer uses the Gradle cache (cache-disabled: true), so the signed build never consumes a cache another workflow wrote.
  • The release build runs testDebugUnitTest and lintDebug before signing, in a step without the signing secrets.
  • A dry run uploads its artifact as dry-run-wick-<tag>.
  • Dependabot's github-actions PRs get the [skip release] prefix.
  • Login rejects http:// addresses with its own message. A cleartext refusal (UnknownServiceException) is reported as such instead of "Is your VPN on?".
  • FaviconAuth also checks the scheme before adding the key.
  • Icon URLs are limited to http and https.
  • The API key field is no longer kept in saved instance state.
  • dataExtractionRules exclude the datastore and the database from cloud backup and device transfer.

State bugs

  • The edit token is cached together with the Auth it was fetched for, and is dropped when the account changes.
  • OkHttp calls are cancelled when their coroutine is cancelled, and the client has a 5 minute callTimeout. A sign out on a dead VPN no longer waits for the socket timeouts.
  • signingOut lives in AppContainer. It lasts through a rotation, but process death no longer restores a blank screen.
  • upsertKeepingLocal treats items covered by a pending mark-all like items with a pending mark, so a failed mark-all no longer brings rows back as unread under a 0 count.
  • Both DataStores have a ReplaceFileCorruptionHandler.
  • A new list after a refresh leaves out items that are read locally but not yet on the server. Prune and markMissingRead still use the full server id set. The refresh waits for applied marks first.
  • A 2xx login answer without Auth= counts as no Google Reader API, and 403 is mapped the same way everywhere.

Consistency and accessibility

  • User facing messages and ages come from string and plural resources. The views expose error types instead of strings.
  • Shared constants and known API paths are defined once, and the two v0.1.0 migrations share one format constant and helper.
  • AccountSwitch and wipeAccount move out of the settings UI package.
  • TalkBack:
    • rows announce read or unread
    • ages are read in long form
    • the hidden pill is out of the tree
    • login errors are a live region
    • headings are marked
    • the wordmark reads as "Wick"

Dead code

  • htmlUrl is removed from the entity, the DTOs and the model. Room goes to schema 2 with an AutoMigration and @DeleteColumn, so the pending marks queue survives. 2.json is exported and a migration test covers it.
  • Also removed: unused DAO queries, WickColors.Core, CategoryDto.id, OriginDto.htmlUrl, an unused import, and the duplicate fallback in openLink.
  • Test-only APIs are made private or internal.

UI

  • The drawer shows the wordmark above "All unread".
  • Settings are one row each:
    • switches for hiding empty feeds, summary images and haptics
    • two-segment controls for sort order and link target
  • The settings title sits where the list header title does, and every settings row is the same height.

README

  • The feature list is written as plain sentences.
  • Server examples are generic.
  • A note says the API key travels as a query string for favicons, so it appears in your own server's access logs.

Not in this PR

  • The baseline profile has to be generated on a device.
  • The settings and session storage split was skipped, to avoid a data migration.

Tests

235 unit tests, lint and assembleRelease pass. Only the drawer and settings snapshots changed.

Pin every action in ci.yml to a SHA and stop the release build from reading
the shared Gradle cache. Run the tests and lint before a release is built,
name dry run APKs apart and keep Dependabot action bumps from releasing.

Refuse plain http addresses at sign in with their own message and tell a
refused cleartext call apart from an unreachable server. Cancel HTTP calls
with their coroutine and cap each call, so a sign out over a dead VPN ends
on time. Keep the edit token with the sign in it belongs to, treat 403 as
bad credentials everywhere and a login answer without an Auth line as no
API. Favicons only get the key over the API's own scheme, icons must be
http or https, images go out with a plain User-Agent, backups and device
transfers leave the data behind, the API key is not kept in saved state
and the sign out guard no longer survives a process restart.
A refresh while a mark all is still queued wrote the covered items back
as unread, so the list showed rows under a header that said 0. Items a
pending mark all covers now stay read, by the same rule the queue used
when it marked them, unless a later item mark is pending.

A corrupt credentials or settings file is replaced with an empty one
instead of crashing every start, and the deliberate blocking read at
startup is explained where it happens.
Opening a feed could show rows that were already dimmed: the server still
listed them as unread because their read marks had not been sent yet, and
the new list took every id it fetched. A new list now leaves out the items
that are read here, while the fetched ids still decide what is kept and
what is marked read. A refresh also waits until the marks made just before
it are in the database.
Error messages, item ages and the new screen reader texts come from string
resources; the view models hand out a resource and the screens word it.
Ages are a value instead of an English token, and screen readers hear
them in full. Rows say whether they are read, the hidden new items pill
is gone from the accessibility tree, login errors are announced, headers
are headings and the wordmark reads as Wick. The repeated grey and scrim
colours live in WickColors.
Drop the feed site address, which nothing read, with a Room migration to
version 2 that keeps the queue of read marks, and test that migration from
the exported version 1 schema. Remove the item reader only tests used, the
unused queue delete, the unused colour and DTO fields, and an unused
import. Make the mark all arm's inner steps private and the work names
internal, and let a link fall back to a plain view only from a Custom Tab.

Share one list of known API paths, one store format and migrate helper,
one way to find a view's feeds, and one log tag, tap target, note and
button style, chunk size and HTML input cap. Move the account wipe and
switch out of the settings UI, rename the drawer filter, use the v2
Compose test rule, keep the bar colours to the versions that read them and
say why the test JVM flag is there.
The feature list reads as plain sentences, the requirements name https and
the known API paths without tying them to one server, and the privacy part
says that the API key travels in favicon addresses and so lands in the
server's access logs.
@bardesss
bardesss merged commit c9dfea1 into main Nov 1, 2026
1 check passed
@bardesss
bardesss deleted the review-fixes branch November 1, 2026 12:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant