Skip to content

feat(mill): add has_adjustable_speed and has_timer capability flags#1109

Open
silasg wants to merge 1 commit intographefruit:developfrom
silasg:feature/mill-capability-flags
Open

feat(mill): add has_adjustable_speed and has_timer capability flags#1109
silasg wants to merge 1 commit intographefruit:developfrom
silasg:feature/mill-capability-flags

Conversation

@silasg
Copy link
Copy Markdown
Contributor

@silasg silasg commented Apr 3, 2026

Add two boolean properties to the Mill model that control whether mill_speed and mill_timer fields are visible in brew forms and detail views. When a selected grinder lacks a capability, the corresponding brew field is hidden regardless of global/preparation settings.

  • Extend Mill interface and class with has_adjustable_speed, has_timer (default true for backward compatibility, no migration needed)
  • Add HAS_ADJUSTABLE_SPEED, HAS_TIMER to millFunctionPipe
  • Wire choosenMill into brew-brewing and brew-detail components
  • Add checkboxes to mill add/edit/detail views
  • Add i18n keys for all supported languages
  • Add unit tests for Mill model and brew-brewing visibility
  • Fix pre-existing bug: millFunctionPipe used PREPARATION enum for HAS_PHOTOS
  • Increase mill-add modal breakpoint from 0.35 to 0.5

Notes:

  • Non-English translations (ar, el, es, fr, id, it, nl, no, pl, pt, tr, zh) are automated and need review by native speakers.
  • As discussed I tried to gradually introduce tests for altered pre-existing files. For the Brew page template that required lots of setup, which will need to be extended when tests of other functionality are added. I tried to stick to a 'baby steps' approach as good as I could.

Add two boolean properties to the Mill model that control whether
mill_speed and mill_timer fields are visible in brew forms and detail
views. When a selected grinder lacks a capability, the corresponding
brew field is hidden regardless of global/preparation settings.

- Extend Mill interface and class with has_adjustable_speed, has_timer
  (default true for backward compatibility, no migration needed)
- Add HAS_ADJUSTABLE_SPEED, HAS_TIMER to millFunctionPipe
- Wire choosenMill into brew-brewing and brew-detail components
- Add checkboxes to mill add/edit/detail views
- Add i18n keys for all supported languages
- Add unit tests for Mill model and brew-brewing visibility
- Fix pre-existing bug: millFunctionPipe used PREPARATION enum for HAS_PHOTOS
- Increase mill-add modal breakpoint from 0.35 to 0.5

Note: Non-English translations (ar, el, es, fr, id, it, nl, no, pl, pt,
tr, zh) are automated and need review by native speakers.
@graphefruit
Copy link
Copy Markdown
Owner

Hey @silasg,
basically what I miss in this PR:
What happens to the old userbase?
Basically has_adjustable_speed is initialized with false so persons updating the app, had shown RPM before, would be curious WHY the value is not shown anymore (because now the mill is the gate-keeper).
This should be handled in the uiUpdate, to check fi the RPM e.g. is activated globally or in a customized preparation method, and then update all the grinders to true, so the old userbase will still see their values and can change it afterwards.

@silasg
Copy link
Copy Markdown
Contributor Author

silasg commented Apr 4, 2026

Hi @graphefruit, for that reason I set both fields to true in the constructor: 3fb8060#diff-55c24f958be9f96d1f2dfa384de3fecd8c0f0ba502304e8a024248299e63a2c4R19

I also added a test case for that (see 3fb8060#diff-73160c80b39c50e950f649207d028e17fab44f691e9d0e823476000594e7f2eaR34 ) and tested it manually on my phone -- both seemed to work. I only uncheck the boxes in the form if new grinders are added after the update was rolled out.

I thought this is the simplest and most backwards compatible way of introducing the feature. But of course I am happy to change it, in case I missed something.

@graphefruit
Copy link
Copy Markdown
Owner

Hi @graphefruit, for that reason I set both fields to true in the constructor

How could I miss this, sorry!

I thought this is the simplest and most backwards compatible way of introducing the feature. But of course I am happy to change it, in case I missed something.

No thats fine for me.

I'll test it tomorrow and give it a merge afterwards

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