Restructure project table - #406
Merged
Merged
Conversation
📝 WalkthroughWalkthroughThe projects page now renders project analysis summaries with derived statuses, filters, meters, data-store indicators, refresh handling, and truncation warnings. Shared data-store actions replace duplicated markup. Obsolete proposal and approval components and tests were removed. ChangesProject status table
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ProjectsTable
participant useProjectAnalysisSummary
participant HubAPI
participant KongRoutes
participant PodOrchestrator
participant summariseProjectAnalyses
ProjectsTable->>useProjectAnalysisSummary: refresh()
useProjectAnalysisSummary->>HubAPI: fetch analysis nodes
useProjectAnalysisSummary->>KongRoutes: fetch data-store routes
useProjectAnalysisSummary->>PodOrchestrator: fetch execution statuses
useProjectAnalysisSummary->>summariseProjectAnalyses: merge and summarize data
summariseProjectAnalyses-->>ProjectsTable: project summaries and statuses
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
app/components/projects/ProjectsTable.vue (3)
332-400: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueScope the style block.
The
<style>block is global, so.status-meter,.project-status, and.status-legend*leak into every page. All selectors target elements owned by this component, soscopedworks without change.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/components/projects/ProjectsTable.vue` around lines 332 - 400, Add the scoped attribute to the style block in ProjectsTable.vue so the component-specific selectors, including .status-meter, .project-status, and .status-legend*, apply only within ProjectsTable. Leave the existing CSS declarations unchanged.
18-22: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unused expander state and rename
proposals.
expandRowEntriesstays empty, so the expander column at line 185 never renders andexpandedRowsis never populated. The component renders projects, soproposalsis a stale name carried over fromProjectProposalTable. Drop the expander wiring or populate it, and rename the row state toprojects.Also applies to: 169-185
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/components/projects/ProjectsTable.vue` around lines 18 - 22, In ProjectsTable, rename the row data state `proposals` to `projects` and update all references to match the component’s project data. Remove the unused `expandedRows` state and `expandRowEntries`/expander-column wiring, or populate it consistently if expansion is required; keep the rendered table behavior aligned with the actual project rows.
85-93: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winHandle
refreshSummariesfailures separately.The analysis-summary fetches are already masked as
undefined, butrefreshSummariesstill throws ifsummariseProjectAnalysesorderiveProjectStatusruns on unexpected input. Thefinallymakesloadingfalse without an error message. CatchrefreshSummaries()at the top-level render and inonTableRefresh, keep the project rows rendered, and display the summary-load failure so stale summaries do not block refresh.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/components/projects/ProjectsTable.vue` around lines 85 - 93, Handle refreshSummaries failures separately in the top-level render flow and onTableRefresh: catch errors from summariseProjectAnalyses or deriveProjectStatus, display the summary-load failure, and continue rendering the existing project rows instead of propagating the exception. Preserve the loading cleanup and ensure table refresh still completes even when summary loading fails.test/components/projects/ProjectsTable.spec.ts (1)
79-85: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unused toast setup.
ProjectsTable.vuedoes not calluseToast, so themockToastdeclaration andvi.mocked(useToast).mockReturnValue(mockToast)line add only unused test setup.♻️ Proposed change
-import { useToast } from "primevue/usetoast"; import { computed, defineComponent } from "vue"; @@ describe("ProjectsTable.vue", () => { - let mockToast; let ProjectsTableTestComponent; beforeEach(() => { vi.restoreAllMocks(); // Reset mocks before each test - vi.mocked(useToast).mockReturnValue(mockToast);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/components/projects/ProjectsTable.spec.ts` around lines 79 - 85, Remove the unused mockToast declaration and the vi.mocked(useToast).mockReturnValue(mockToast) setup from the ProjectsTable.vue test’s beforeEach block, leaving the remaining mock-reset behavior unchanged.test/components/projects/constants.ts (1)
53-66: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueClone the shared proposal entry in
fakeTwoProposalsResp.
fakeTwoProposalsResp[0]reusesfakeProposalsResp[0], and the second entry copies itsnodeby reference.ProjectsTable.parseDataaddsproject_name,summary, andproject_statusto the formatted rows, so this fixture can carry test-local state into later expectations. Clone the source entry for both rows.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/components/projects/constants.ts` around lines 53 - 66, Update fakeTwoProposalsResp to clone fakeProposalsResp[0] for its first entry as well as the customized second entry, ensuring both rows have independent node objects. Preserve the second proposal’s existing project-specific fields while preventing ProjectsTable.parseData mutations from leaking into shared fixture state.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/components/projects/ProjectsTable.vue`:
- Around line 192-196: Update the tooltip binding in the ProjectsTable body
template to safely access the optional project relation, matching the
optional-chaining pattern already used for this field elsewhere in the
component. Preserve the existing project name rendering.
In `@app/components/shared/DataStoreBadge.vue`:
- Around line 32-41: Update the DataStoreBadge button rendering around the
createDataStore click handler so the optional state (required false) uses a
non-disabled wrapper or other non-interactive element that still exposes
missingTooltip to v-tooltip.top, while preserving the button and click behavior
for required stores. Ensure the “Data store missing, but not required” tooltip
can trigger without allowing creation from the optional state.
In `@app/composables/useProjectAnalysisSummary.ts`:
- Around line 42-55: Keep upstream failures distinct from valid empty responses
throughout useProjectAnalysisSummary and refreshSummaries: in
app/composables/useProjectAnalysisSummary.ts lines 42-55, mark failed
analysis-page requests as incomplete or unavailable; lines 64-73, preserve Kong
unavailability separately from an empty data-store Set; lines 76-103, preserve
unavailable pod-status data separately from missing analysis entries; and lines
111-150, expose these availability states so consumers do not publish normal
statuses from incomplete data. Update
test/composables/useProjectAnalysisSummary.test.ts lines 42-50, 90-100, and
124-132 to assert each unavailable state, with no direct change required beyond
validating the corresponding implementation behavior.
- Around line 25-28: Add PodStatus.Stopped to FINISHED_STATUSES so
mergeExecutionStatuses preserves a Hub stopped status when /po/status has no
matching analysis entry. Extend the no-entry coverage in the
useProjectAnalysisSummary test to verify stopped analyses remain counted and
summarized as Stopped.
---
Nitpick comments:
In `@app/components/projects/ProjectsTable.vue`:
- Around line 332-400: Add the scoped attribute to the style block in
ProjectsTable.vue so the component-specific selectors, including .status-meter,
.project-status, and .status-legend*, apply only within ProjectsTable. Leave the
existing CSS declarations unchanged.
- Around line 18-22: In ProjectsTable, rename the row data state `proposals` to
`projects` and update all references to match the component’s project data.
Remove the unused `expandedRows` state and `expandRowEntries`/expander-column
wiring, or populate it consistently if expansion is required; keep the rendered
table behavior aligned with the actual project rows.
- Around line 85-93: Handle refreshSummaries failures separately in the
top-level render flow and onTableRefresh: catch errors from
summariseProjectAnalyses or deriveProjectStatus, display the summary-load
failure, and continue rendering the existing project rows instead of propagating
the exception. Preserve the loading cleanup and ensure table refresh still
completes even when summary loading fails.
In `@test/components/projects/constants.ts`:
- Around line 53-66: Update fakeTwoProposalsResp to clone fakeProposalsResp[0]
for its first entry as well as the customized second entry, ensuring both rows
have independent node objects. Preserve the second proposal’s existing
project-specific fields while preventing ProjectsTable.parseData mutations from
leaking into shared fixture state.
In `@test/components/projects/ProjectsTable.spec.ts`:
- Around line 79-85: Remove the unused mockToast declaration and the
vi.mocked(useToast).mockReturnValue(mockToast) setup from the ProjectsTable.vue
test’s beforeEach block, leaving the remaining mock-reset behavior unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ec9bff73-fec1-4581-8cff-39b447fae19d
📒 Files selected for processing (15)
app/components/analysis/AnalysesTable.vueapp/components/projects/ProjectProposalTable.vueapp/components/projects/ProjectsTable.vueapp/components/shared/DataStoreBadge.vueapp/components/table/ApproveRejectToggle.vueapp/composables/useProjectAnalysisSummary.tsapp/pages/projects.vueapp/utils/summarise-project-analyses.tstest/components/projects/ProjectProposalTable.spec.tstest/components/projects/ProjectsTable.spec.tstest/components/projects/constants.tstest/components/shared/DataStoreBadge.spec.tstest/components/table/ApproveRejectToggle.spec.tstest/composables/useProjectAnalysisSummary.test.tstest/utils/summarise-project-analyses.test.ts
💤 Files with no reviewable changes (4)
- app/components/projects/ProjectProposalTable.vue
- app/components/table/ApproveRejectToggle.vue
- test/components/projects/ProjectProposalTable.spec.ts
- test/components/table/ApproveRejectToggle.spec.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/composables/useProjectAnalysisSummary.ts`:
- Around line 74-86: Update the route-fetching logic in the project analysis
summary to follow ListRoutes.offset until no next page remains, aggregating
project IDs from every response and preserving unavailable handling for request
failures. Ensure summaryFor receives the complete projectIds set, and add
coverage for a project route returned on a subsequent page.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f4974790-b458-422c-939a-0c13e380a065
📒 Files selected for processing (5)
app/components/projects/ProjectsTable.vueapp/components/shared/DataStoreBadge.vueapp/composables/useProjectAnalysisSummary.tstest/components/shared/DataStoreBadge.spec.tstest/composables/useProjectAnalysisSummary.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- test/components/shared/DataStoreBadge.spec.ts
- app/components/shared/DataStoreBadge.vue
- test/composables/useProjectAnalysisSummary.test.ts
- app/components/projects/ProjectsTable.vue
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Tests