Skip to content

Add hasOverride API to toggles-flow and toggles-prefs#479

Open
erikeelde wants to merge 16 commits into
mainfrom
feature/has-override
Open

Add hasOverride API to toggles-flow and toggles-prefs#479
erikeelde wants to merge 16 commits into
mainfrom
feature/has-override

Conversation

@erikeelde

Copy link
Copy Markdown
Owner

Summary

  • Adds hasOverride(key, comparator) to the Toggles (Flow) and TogglesPreferences (Prefs) interfaces, letting consumers check whether a toggle is currently being served from a non-default scope
  • Introduces ScopeComparator (fun interface) and DefaultScopeComparator in each library — consumers can pass a lambda for custom override detection logic
  • Noop variants return false unconditionally; .api files updated manually (BCV unavailable under AGP 9)
  • toggles-core is unchanged — clarified its purpose in CLAUDE.md as the ContentProvider contract module only

Test Plan

  • ./gradlew :toggles-flow:test :toggles-prefs:test — all unit tests pass
  • ./gradlew :toggles-flow:check :toggles-prefs:check — detekt + lint clean
  • ./gradlew :toggles-flow-noop:assembleDebug :toggles-prefs-noop:assembleDebug — noop modules compile
  • Verify toggles-flow/api/toggles-flow.api and toggles-prefs/api/toggles-prefs.api include new entries for ScopeComparator, DefaultScopeComparator, hasOverride, and $DefaultImpls bridge

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings April 27, 2026 06:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new hasOverride(key, comparator) API to the toggles-flow and toggles-prefs client libraries so consumers can detect when a toggle is being served from a non-default scope, with a pluggable comparator (ScopeComparator) and a default implementation (DefaultScopeComparator). This also updates the noop variants and public API surface files accordingly.

Changes:

  • Add ScopeComparator + DefaultScopeComparator to toggles-flow and toggles-prefs, plus noop equivalents.
  • Extend Toggles (Flow) and TogglesPreferences (Prefs) with hasOverride(...) and implement it in *Impl.
  • Add/extend Robolectric-based tests and update .api baseline files; add design/plan docs and clarify toggles-core responsibility in CLAUDE.md.

Reviewed changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
toggles-prefs/src/test/java/se/eelde/toggles/prefs/DefaultScopeComparatorTest.kt Unit tests for prefs default override detection.
toggles-prefs/src/test/java/se/eelde/toggles/TogglesPreferencesReturnsProviderValues.kt Integration tests asserting hasOverride behavior + custom comparator invocation.
toggles-prefs/src/main/java/se/eelde/toggles/prefs/TogglesPreferencesImpl.kt Implements hasOverride by delegating to provider + comparator.
toggles-prefs/src/main/java/se/eelde/toggles/prefs/TogglesPreferences.kt Adds hasOverride to the prefs public interface with a default comparator.
toggles-prefs/src/main/java/se/eelde/toggles/prefs/ScopeComparator.kt Introduces ScopeComparator + DefaultScopeComparator for prefs.
toggles-prefs/api/toggles-prefs.api Updates public API baseline for prefs.
toggles-prefs-noop/src/main/java/se/eelde/toggles/prefs/TogglesPreferencesImpl.kt Noop hasOverride implementation returning false.
toggles-prefs-noop/src/main/java/se/eelde/toggles/prefs/TogglesPreferences.kt Adds hasOverride to noop prefs interface.
toggles-prefs-noop/src/main/java/se/eelde/toggles/prefs/ScopeComparator.kt Noop comparator types (always false).
toggles-prefs-noop/api/toggles-prefs-noop.api Updates noop prefs public API baseline.
toggles-flow/src/test/java/se/eelde/toggles/flow/FlowTest.kt Adds flow-side hasOverride integration tests.
toggles-flow/src/test/java/se/eelde/toggles/flow/FakeToggles.kt Updates test fake to implement hasOverride.
toggles-flow/src/test/java/se/eelde/toggles/flow/DefaultScopeComparatorTest.kt Unit tests for flow default override detection.
toggles-flow/src/main/java/se/eelde/toggles/flow/TogglesImpl.kt Implements flow hasOverride by mapping over observed ToggleState.
toggles-flow/src/main/java/se/eelde/toggles/flow/Toggles.kt Adds hasOverride to the flow public interface with a default comparator.
toggles-flow/src/main/java/se/eelde/toggles/flow/ScopeComparator.kt Introduces ScopeComparator + DefaultScopeComparator for flow.
toggles-flow/api/toggles-flow.api Updates flow public API baseline.
toggles-flow-noop/src/main/java/se/eelde/toggles/flow/TogglesImpl.kt Noop flow hasOverride implementation returning false.
toggles-flow-noop/src/main/java/se/eelde/toggles/flow/Toggles.kt Adds hasOverride to noop flow interface.
toggles-flow-noop/src/main/java/se/eelde/toggles/flow/ScopeComparator.kt Noop comparator types (always false).
toggles-flow-noop/api/toggles-flow-noop.api Updates noop flow public API baseline.
docs/superpowers/specs/2026-04-24-has-override-design.md Design spec for the new API.
docs/superpowers/plans/2026-04-25-has-override.md Implementation plan documenting intended steps.
CLAUDE.md Clarifies that toggles-core is contract-only and where shared logic should live.

Comment thread toggles-prefs/api/toggles-prefs.api Outdated
Comment on lines +28 to +35
object DefaultScopeComparator : ScopeComparator {
override fun hasOverride(state: ToggleState): Boolean {
val defaultScope = state.scopes.firstOrNull { it.name == ToggleScope.DEFAULT_SCOPE }
val selectedScope = state.scopes.maxByOrNull { it.timeStamp }
if (defaultScope == null || selectedScope == null) return false
if (defaultScope.id == selectedScope.id) return false
return state.configurationValues.any { it.scope == selectedScope.id }
}

Copilot AI Apr 27, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The example uses ToggleScope.DEFAULT_SCOPE, but the default-scope constant lives in ColumnNames.ToggleScope.DEFAULT_SCOPE (there is no ToggleScope.DEFAULT_SCOPE). Update the snippet so it matches the actual API/implementation.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a66012f — corrected to ColumnNames.ToggleScope.DEFAULT_SCOPE.

Comment thread docs/superpowers/specs/2026-04-24-has-override-design.md
Comment on lines +214 to +242
- [ ] **Step 1: Write the failing integration test**

Add these two test methods to the existing `FlowTest` class in `toggles-flow/src/test/java/se/eelde/toggles/flow/FlowTest.kt`. Add the needed imports too: `import kotlinx.coroutines.flow.first`, `import org.junit.Assert.assertFalse`, `import org.junit.Assert.assertNotNull`, `import se.eelde.toggles.core.ToggleState`.

```kotlin
@Test
fun `hasOverride returns false for key with no overriding scope value`() = runTest {
val toggles = TogglesImpl(context)
val result = toggles.hasOverride("no-override-key").first()
@OptIn(ExperimentalCoroutinesApi::class)
advanceUntilIdle()
assertFalse(result)
}

@Test
fun `hasOverride passes ToggleState to custom comparator`() = runTest {
var capturedState: ToggleState? = null
val capturingComparator = ScopeComparator { state ->
capturedState = state
false
}
val toggles = TogglesImpl(context)
toggles.hasOverride("some-key", capturingComparator).first()
@OptIn(ExperimentalCoroutinesApi::class)
advanceUntilIdle()
assertNotNull(capturedState)
}
```

Copilot AI Apr 27, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This plan still instructs adding advanceUntilIdle() / ExperimentalCoroutinesApi calls in FlowTest, but the final test code no longer uses them. Consider updating/removing these steps so the plan reflects the current implementation and doesn’t reintroduce unnecessary imports in future edits.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a66012f — removed the stale advanceUntilIdle() and @OptIn(ExperimentalCoroutinesApi::class) calls from the plan; the final tests don't use them.

erikeelde and others added 14 commits April 27, 2026 10:32
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add hasOverride method to the Toggles interface with a default ScopeComparator
parameter, and implement it in TogglesImpl by observing toggle state and passing
it to the comparator. Also update FakeToggles test helper for compatibility.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Remove dead advanceUntilIdle() calls and @OptIn annotations from test
methods where first() has already returned. Add assertNull check for
capturedState.configuration. Update toggles-flow.api with new public
types: ScopeComparator (fun interface), DefaultScopeComparator (object),
Toggles$DefaultImpls, and hasOverride methods in Toggles and TogglesImpl.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…sImpl

Implements hasOverride method with optional ScopeComparator parameter (defaults to DefaultScopeComparator).
Also updates the API file to include ScopeComparator and DefaultScopeComparator that were added in Task 4.
Both integration tests pass: one verifies false return for unknown keys, another captures ToggleState.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ScopeComparator takes ToggleState as a parameter to maintain source
compatibility with the real library. The noop modules need toggles-core
on their classpath to compile this interface.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@erikeelde erikeelde force-pushed the feature/has-override branch from 8776d4a to 2711f4a Compare April 27, 2026 08:38
erikeelde and others added 2 commits April 27, 2026 10:39
- Reorder toggles-prefs.api so DefaultScopeComparator precedes ScopeComparator (alphabetical BCV order)
- Fix spec: ToggleScope.DEFAULT_SCOPE -> ColumnNames.ToggleScope.DEFAULT_SCOPE
- Fix spec: getToggleState -> observeToggleState (actual function name in TogglesImpl)
- Remove stale advanceUntilIdle/ExperimentalCoroutinesApi from plan (not used in final tests)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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