set up consumer group related models#3263
Conversation
d1a73d7 to
40bde89
Compare
5980862 to
f53190d
Compare
| } | ||
|
|
||
| /** Error icon to use for Flink Database resource containers items if fetching resources fails. */ | ||
| export const ERROR_ICON = new ThemeIcon("warning", new ThemeColor("problemsErrorIcon.foreground")); |
There was a problem hiding this comment.
Moved to src/icons.ts
e88952f to
d5c9efc
Compare
d5c9efc to
9b4450d
Compare
| get canResetOffsets(): boolean { | ||
| const resettableStates = [ConsumerGroupState.Empty, ConsumerGroupState.Dead]; | ||
| return resettableStates.includes(this.state); | ||
| } |
There was a problem hiding this comment.
Not used by anything yet, just prep for upcoming commands
…sistency and update tests; add CCloud link section to tooltips
There was a problem hiding this comment.
Pull request overview
This PR sets up the foundational model classes for Consumer Groups and Consumer members to support the upcoming Consumer Groups view. It introduces ConsumerGroup and Consumer model classes along with their corresponding *TreeItem representations, following the established patterns in the codebase. The PR also refactors the ERROR_ICON constant from flinkDatabaseResourceContainer.ts to icons.ts for better reusability across the codebase.
Changes:
- Added
ConsumerGroupandConsumermodels with comprehensive properties including state management, membership tracking, and CCloud URL generation - Added
ConsumerGroupTreeItemandConsumerTreeItemclasses with state-based icon coloring and detailed tooltips - Refactored
ERROR_ICONto centralizedicons.tslocation - Added test fixtures and comprehensive unit tests covering all model behaviors
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/models/consumerGroup.ts |
New model classes for ConsumerGroup and Consumer with TreeItem representations, state management, and URL generation |
src/models/consumerGroup.test.ts |
Comprehensive unit tests for all model behaviors including state transitions, URL generation, and tree item rendering |
tests/unit/testResources/consumerGroup.ts |
Test fixtures providing factory functions and pre-configured test instances for all connection types |
src/icons.ts |
Added ERROR_ICON constant for reuse across models |
src/models/flinkDatabaseResourceContainer.ts |
Updated to import ERROR_ICON from centralized location |
src/models/flinkDatabaseResourceContainer.test.ts |
Updated imports to reference ERROR_ICON from centralized location |
src/extension.ts |
Added comments explaining why consumer groups can't be included in static context value arrays |
|
Lucia Cerchie (Cerchie)
left a comment
There was a problem hiding this comment.
This LGTM, I did run Claude against it and it had one nit, that these are missing JSDocs:
`ConsumerGroup.id` getter
`ConsumerGroup.hasMembers` getter (has inline comment but no JSDoc)
`ConsumerGroup.searchableText()`
`ConsumerGroup.ccloudUrl` getter
`Consumer.id` getter
`Consumer.searchableText()`
`Consumer.ccloudUrl` getter
but given how clear the names of these are I'm not going to block on that




Summary of Changes
No functional changes here, just adding the basic
ConsumerGroup,Consumermodels with*TreeItemsiblings for each before the view provider starts using them with the previously-merged data-fetching operations.assignmentsproperty and integrate that part of the fetching logic.Closes #3299
Associated PRs
ResourceContainerfor Flink database and Kafka cluster child resources #3292Pull request checklist
Please check if your PR fulfills the following (if applicable):
Tests
Release notes