Skip to content

Use rohlik-api 0.3.0 (shared shop list, minimum order value); 1.0.0-beta4 - #93

Merged
dvejsada merged 3 commits into
masterfrom
claude/ha-integration-open-issues-1hg1fx
Sep 27, 2026
Merged

dvejsada merged 3 commits into
masterfrom
claude/ha-integration-open-issues-1hg1fx

Conversation

@dvejsada

Copy link
Copy Markdown
Owner

Why

rohlik-api 0.3.0, now on PyPI, ships the Rohlík Group shop presets (SITES), a currency on cart models, and the shop's minimum order value (Cart.minimum_order_price). The integration kept its own copy of the shop table from #92 and had no way to show the minimum.

Changes

  • Dependency: rohlik-api bumped from 0.2.0 to 0.3.0 in manifest.json and requirements_test.txt.
  • Shop list: now comes from rohlik_api.SITES, and the local copy in const.py is removed. The codes stored on config entries (cz, de, at, hu, ro) are unchanged, so existing entries are unaffected.
  • Cart Total sensor: new Minimum Order Price attribute, present only when the shop reports a minimum.
    • Can Order is unchanged. It mirrors submitConditionPassed, which also needs checkout details such as a delivery slot, so it can't tell you whether the minimum is met.
    • The README explains the difference.
  • Cart to-do: each line shows its own currency, falling back to the shop's.
  • Version: bumped to 1.0.0-beta4 in the manifest and README, following the same pattern as the beta3 bump. This beta also ships the multi-shop support from Support Knuspr.de, Gurkerl.at, Kifli.hu and Sezamo.ro #92 (knuspr.de #91).

Code review

A code review ran on the branch, and its fixes are in the second commit:

  • hub.py imports Site directly from rohlik_api.
  • New test: the translated shop labels (en/cs) must match rohlik_api.SITES. If a future library release adds a shop, CI fails instead of the setup screen showing an option without a label.
  • New test: cart to-do lines show the item's own currency, falling back to the shop's.

Two findings were deliberately left as they are:

  • Unknown shop key: get_site() still falls back to Rohlík.cz. Changing that would change existing behavior; the new translation test partly guards against the shop list drifting.
  • Minimum of 0: Minimum Order Price is shown even when it is 0. Anonymous carts report 0, but nothing shows that a logged-in cart does, so filtering it out would be guesswork.

Follow-ups (not in this PR)

  • Dashboard cards: the cart card in rohlik-cz-HA-cards still uses its manual min_order option. It should prefer the new Minimum Order Price attribute when present.
  • Real minimum value: the value for a logged-in account hasn't been observed yet.

Testing

python -m pytest tests/: 79 passed, against rohlik-api 0.3.0 from PyPI. The new tests cover the attribute being present or absent, the to-do currency, and the translation/SITES consistency.

🤖 Generated with Claude Code

https://claude.ai/code/session_0135WLPbnfESWnAJmw4P8uRH


Generated by Claude Code

- Bump rohlik-api to 0.3.0 (manifest and test requirements).
- Take the shop presets (base URL, name, currency, timezone) from
  rohlik_api.SITES instead of a local copy in const.py. The codes stored
  on config entries (cz, de, at, hu, ro) are the same, so existing entries
  are unaffected.
- Cart Total sensor: new "Minimum Order Price" attribute from
  Cart.minimum_order_price, present only when the shop reports it.
  "Can Order" stays as is; it also needs checkout details, so it is not a
  minimum check.
- Cart to-do items show each item's own currency, falling back to the
  shop's.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0135WLPbnfESWnAJmw4P8uRH
…o currency

- hub.py imports Site directly from rohlik_api rather than through const.
- Test that the translated shop options match rohlik_api.SITES, so a shop
  added in a later rohlik-api release cannot appear without a label.
- Test the cart to-do item currency and its fallback to the shop's.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0135WLPbnfESWnAJmw4P8uRH
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0135WLPbnfESWnAJmw4P8uRH

Copy link
Copy Markdown
Owner Author

@claude review this PR. Focus on correctness with rohlik-api 0.3.0 (the SITES / Site fields and the Cart / CartItem fields used), regressions for existing config entries, Home Assistant conventions for the new sensor attribute, and whether the version bump is consistent.


Generated by Claude Code

@claude

claude Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Claude finished @dvejsada's task in 3m 2s —— View job


Review of PR #93

  • Gather context (diff, CLAUDE.md, changed files)
  • Check rohlik-api 0.3.0 SITES/Site usage correctness
  • Check Cart/CartItem field usage (minimum order price, currency)
  • Check for regressions with existing config entries
  • Check HA conventions for new sensor attribute
  • Check version bump consistency
  • Post final review

SITES / Site (rohlik-api 0.3.0)

  • const.py now imports SITES, Site from rohlik_api and drops the local @dataclass Site — get_site()'s behavior is unchanged (still falls back to cz for None/unknown keys). Nothing else in the codebase imported Site from .const (checked with a repo-wide grep), so removing it there is safe.
  • hub.py now imports Site directly from rohlik_api for its type hint (custom_components/rohlikcz/hub.py:19,397) — matches the PR description's stated review fix.
  • config_flow.py still does from .const import ... SITES, get_site and options=list(SITES) (custom_components/rohlikcz/config_flow.py:22,50) — since const.py re-exports the name via its from rohlik_api import SITES, Site import, this keeps working unchanged.
  • Existing config entries store site codes cz/de/at/hu/ro; the new tests/test_config_flow.py::test_every_site_has_a_translated_option confirms both en.json and cs.json selector options exactly match rohlik_api.SITES' keys. Good regression guard for a library-side shop list.

Cart / CartItem fields

  • sensor.py:1016 guards cart.minimum_order_price is not None before adding the attribute — consistent with tests/fixtures_data.py:56's Cart(...) construction, which omits minimum_order_price entirely, implying it defaults to None on the model. The "present only when the shop reports it" and "0 is shown as-is" behaviors described in the PR body are both exercised by test_cart_minimum_order_price_attribute / test_cart_without_minimum_order_price in tests/test_init.py.
  • todo.py:66 — product.currency or self._rohlik_hub.currency correctly falls back to the shop's currency when a line item doesn't carry its own, and is covered by test_cart_todo_uses_item_currency (mixed line-item currencies + a cart-level fallback).
  • I could not reach PyPI/GitHub from this sandbox to independently verify rohlik_api 0.3.0's actual field names, but everything here is internally consistent with the (passing, per the PR description) test suite and with the fixtures using the real installed package, so I'm not flagging this as a risk.

Regressions for existing entries

None spotted. The site codes are unchanged, get_site()'s fallback logic is unchanged, and Can Order (cart.can_make_order) is untouched — only a new, independently-gated attribute was added alongside it.

HA conventions for the new attribute

Minimum Order Price is added as a raw numeric value with a Title Case, space-containing key in extra_state_attributes. That diverges from HA's usual snake_case attribute-key guidance, but it matches every other attribute already in this sensor (Total items, Can Order) and elsewhere in sensor.py (e.g. Deposit Currency at sensor.py:956), so it's consistent with this integration's existing (pre-existing, not introduced here) convention rather than a new deviation. No unit is attached to the attribute, but that also matches the existing local pattern for numeric attributes on this sensor, and it shares context with the main state's currency unit.

Version bump

Consistent: manifest.json (requirements → rohlik-api==0.3.0, version → 1.0.0-beta4), requirements_test.txt → 0.3.0, and both readme.md mentions (banner + HACS install step) updated to beta4. No other file pins the version or the dependency.

Note

I wasn't able to run pytest or reach the network in this sandboxed environment (Bash/WebFetch calls required approval I couldn't grant), so I relied on static review of the diff plus the existing test fixtures/tests rather than re-executing the suite myself.

Nothing here blocks merging — the two deliberately-left findings (unknown-shop fallback, zero minimum) are reasonable calls as explained in the PR description.

@dvejsada
dvejsada merged commit 952f359 into master Sep 27, 2026
4 checks passed
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.

2 participants