aggregation builder v1#230
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Adds an initial “Aggregation” UI and supporting API endpoint so users can build MongoDB aggregation pipelines in the Studio UI and preview outputs at each stage (Compass-inspired), addressing the core of issue #189.
Changes:
- Adds a new
aggregationBuilderroute and navbar entries for a new Aggregation tab. - Introduces a new frontend
aggregation-buildercomponent with stage editing, per-stage output previews, and auto-run results. - Adds a backend
Model.aggregateaction and frontend API bindings to execute aggregation pipelines with a server-enforced result limit.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/routes.js | Adds aggregationBuilder route and updates role/route allowlists. |
| frontend/src/navbar/navbar.js | Adds route-name detection for the Aggregation tab active state. |
| frontend/src/navbar/navbar.html | Adds Aggregation tab links (desktop + mobile) gated by hasAccess(). |
| frontend/src/api.js | Adds api.Model.aggregate() for both lambda and non-lambda modes. |
| frontend/src/aggregation-builder/aggregation-builder.js | New aggregation builder component logic (stages, auto-run, per-stage preview queries). |
| frontend/src/aggregation-builder/aggregation-builder.html | New UI layout for stage editor, pipeline JSON preview, and results list. |
| frontend/src/aggregation-builder/aggregation-builder.css | New styles for code/result panels and preview cards. |
| backend/authorize.js | Allows readonly (and other roles) to call Model.aggregate. |
| backend/actions/Model/index.js | Exports the new aggregate action. |
| backend/actions/Model/aggregate.js | Implements server-side aggregation execution with pipeline validation and limit clamping. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| name: 'aggregationBuilder', | ||
| component: 'aggregation-builder', | ||
| meta: { | ||
| authorized: true |
There was a problem hiding this comment.
meta.authorized is set to true for the new aggregationBuilder route, but the global navigation guard in frontend/src/index.js only enforces hasAccess() when to.meta.authorized is falsy. As-is, users can deep-link to #/aggregation-builder even when hasAccess(roles, 'aggregationBuilder') is false (navbar hides it, but the route guard won't redirect). Set meta.authorized: false for this route (or adjust the guard logic) so RBAC is enforced consistently.
| authorized: true | |
| authorized: false |
| module.exports = app => app.component('aggregation-builder', { | ||
| template: template, | ||
| props: ['roles'], | ||
| data: () => ({ | ||
| models: [], | ||
| selectedModel: null, | ||
| resultLimit: 20, | ||
| stages: [createDefaultStage()], | ||
| stageOperators: STAGE_OPERATORS, | ||
| isRunning: false, | ||
| errorMessage: '', | ||
| results: [], | ||
| visibleResultsCount: RESULT_PAGE_SIZE, | ||
| resultExpandedState: {}, | ||
| autoRunTimer: null, | ||
| previewRefreshTimer: null, | ||
| activeRunId: 0 | ||
| }), |
There was a problem hiding this comment.
PR description says it "closes #189", but the issue explicitly calls for an AI editor flow (prompt input + accept/reject) in addition to the aggregation builder tab. This PR adds the builder tab and previews, but does not add the AI editor pieces described in #189. Either adjust the PR description (don’t mark the issue closed yet) or add the missing AI editor functionality.
| class="w-full rounded-md border border-edge bg-page px-2 py-1.5 text-sm font-mono text-content focus:border-edge-strong focus:outline-none" | ||
| placeholder='{"status":"active"}' | ||
| ></textarea> | ||
| <p v-if="getStageError(stage)" class="text-xs text-red-600">{{getStageError(stage)}}</p> |
There was a problem hiding this comment.
It is a bit rough that the individual stages need to be full-on JSON rather than JavaScript objects
For one thing, that disallows dates and ObjectIds, which is problematic. Let's make it so the individual text areas support JavaScript, not just JSON
| v-for="(doc, index) in visibleResults" | ||
| :key="'result-' + index" | ||
| class="aggregation-builder-doc-card"> | ||
| <button |
There was a problem hiding this comment.
I don't like that you only see how many documents are in the result, but no details about the documents themselves by default. Let's try to reuse the list-json component for the results here for more consistent UX.
closes #189