Skip to content

Conversation

@michalskrivanek
Copy link
Contributor

@michalskrivanek michalskrivanek commented Oct 15, 2025

defaults to true, pass "false" to get the ended leases as well

Summary by CodeRabbit

  • New Features
    • Added an optional filter to lease listings to show only active leases. This enables quicker discovery and cleaner results when browsing leases.
    • The feature is fully backward-compatible: existing requests behave the same unless the new filter is used.
    • API consumers can opt in by specifying the new parameter; no changes are required for current integrations.

defaults to true, pass "false" to get the ended leases as well
@coderabbitai
Copy link

coderabbitai bot commented Oct 15, 2025

Walkthrough

Added an optional only_active boolean (field 5) to ListLeasesRequest in proto/jumpstarter/client/v1/client.proto to enable filtering leases by active state; no other changes.

Changes

Cohort / File(s) Summary
Proto: ListLeasesRequest enhancement
proto/jumpstarter/client/v1/client.proto
Added field optional bool only_active = 5 to ListLeasesRequest for active-lease filtering. No other schema changes.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Client
  participant API as LeaseService API
  participant Store as Lease Store

  rect rgba(230,245,255,0.5)
  note over Client,API: List leases with optional active filter (only_active)
  Client->>API: ListLeasesRequest { only_active: true|false|null }
  API->>Store: Query leases (apply active filter if only_active==true)
  Store-->>API: Leases []
  API-->>Client: ListLeasesResponse { leases: [...] }
  end

  opt only_active omitted or false
    note over API: Returns all leases (no active-only filter)
  end

  opt only_active true
    note over API: Returns only active leases
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • mangelajo

Poem

Thump-thump goes my fearless paw,
A toggle appears—what I saw!
only_active, crisp and bright,
Filters leases, day or night.
I nibble specs, approve with cheer,
Hop! The proto’s crystal clear. 🥕✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title accurately summarizes the key change by specifying the addition of the only_active field to the ListLeasesRequest message and is both concise and clear. There's no unnecessary detail or ambiguity, and it aligns with the PR objectives.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f67bf42 and 46db003.

📒 Files selected for processing (1)
  • proto/jumpstarter/client/v1/client.proto (1 hunks)
🔇 Additional comments (1)
proto/jumpstarter/client/v1/client.proto (1)

143-143: Default semantics need verification.

Proto3 optional bool surfaces as false to generated clients unless they explicitly set the field or check presence. That contradicts the stated “defaults to true” behavior unless server-side logic treats “not set” as true. Please confirm the implementation actually enforces that default so existing clients aren’t surprised. If the intent is “default true,” consider inverting the flag (e.g., include_ended) or documenting the presence-based contract.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@mangelajo mangelajo merged commit 1a9a9e4 into jumpstarter-dev:main Oct 15, 2025
2 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