Skip to content

feat(api-gateway): add support for onlyViews parameter in meta endpoint#10809

Merged
vasilev-alex merged 3 commits intomasterfrom
only-views-qp
May 4, 2026
Merged

feat(api-gateway): add support for onlyViews parameter in meta endpoint#10809
vasilev-alex merged 3 commits intomasterfrom
only-views-qp

Conversation

@vasilev-alex
Copy link
Copy Markdown
Member

Check List

  • Tests have been run in packages where changes have been made if available
  • Linter has been run for changed code
  • Tests for the changes have been added if not covered yet
  • Docs have been added / updated if required

@vasilev-alex vasilev-alex requested a review from a team as a code owner May 4, 2026 16:10
@github-actions github-actions Bot added the javascript Pull requests that update Javascript code label May 4, 2026
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 4, 2026

Claude finished @vasilev-alex's task in 2m 18s —— View job


Code Review: onlyViews parameter for meta endpoint

  • Gather context and read changed files
  • Review code quality and logic
  • Review security and input validation
  • Review tests
  • Review documentation
  • Post final review

Overall: This is a clean, well-scoped PR. The implementation is straightforward, follows existing patterns (extended parameter), and includes tests + docs. A few observations below.

Code Quality

The implementation is consistent with the existing codebase patterns. The filtering logic in both meta and metaExtended is simple and correct — filtering metaConfig.cubes by type === 'view' before passing to filterVisibleItemsInMeta. The viewGroups in the regular meta endpoint are naturally filtered via the visibleCubeNames set, which correctly narrows to only view-associated groups.

One minor type safety note: the (c: any) casts in the filter callbacks (see inline comment) bypass type checking, though this is consistent with existing patterns in the file.

Security

No concerns. The parameter is presence-based ('onlyViews' in req.query), matching the extended pattern. No user input is interpolated into queries or responses. The existing auth middleware and assertApiScope('meta', ...) checks remain in place.

One behavioral note: ?onlyViews=false will still activate filtering since it's presence-based (see inline comment). This matches extended but could be worth noting in docs.

Performance

The filtering happens client-side after fetching all metadata from the compiler API. Pushing the filter down to the compiler could avoid loading cube metadata entirely, but that's an optimization for later — the current approach is simple and correct.

Testing

Good coverage of both meta and metaExtended paths with onlyViews. The regular meta test also validates viewGroups filtering. I left a suggestion to add a negative assertion in the extended test to explicitly verify cubes are excluded.

Missing test scenario to consider: What happens when there are no views at all? The response would be { cubes: [] } with no viewGroups. This is probably fine behavior, but an explicit test would document that contract.

Documentation

The docs update is clear and well-structured. The parameter table documenting both onlyViews and extended is a nice addition — the extended parameter wasn't previously documented in this table format. The curl example is consistent with existing examples.

Summary

Solid PR — no blockers. The inline comments are suggestions for hardening, not required changes.

}
const cubesConfig = metaConfig.cubes;
const cubesConfig = onlyViews
? metaConfig.cubes.filter((c: any) => c.config?.type === 'view')
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.

Nit: The (c: any) cast bypasses type safety. If metaConfig.cubes has a typed element interface, consider using it here. Same applies to the identical filter in metaExtended below (line 712).

That said, this is consistent with other patterns in this file (e.g., the response: { cubes: any[], ... } declarations), so this is low priority.

await this.metaExtended({
context: req.context,
res: this.resToResultFn(res),
onlyViews: 'onlyViews' in req.query,
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.

Worth noting: since onlyViews is presence-based ('onlyViews' in req.query), a request like ?onlyViews=false will still activate filtering — which may confuse users. This is consistent with how extended works (line 438), so it's a preexisting pattern, but it might be worth documenting in the API reference that the parameter is a flag (presence-only, value is ignored).

Comment on lines +757 to +768
test('meta endpoint extended with onlyViews returns only views', async () => {
const { app } = await createApiGateway();

const res = await request(app)
.get('/cubejs-api/v1/meta?extended&onlyViews')
.set('Authorization', 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.e30.t-IDcSemACt8x4iTMCda8Yhe3iZaWbvV5XKSTbuAn0M')
.expect(200);

expect(res.body).toHaveProperty('cubes');
expect(res.body.cubes).toHaveLength(1);
expect(res.body.cubes[0].name).toBe('FooView');
});
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.

Consider adding an assertion that no cubes are present in the response:

Suggested change
test('meta endpoint extended with onlyViews returns only views', async () => {
const { app } = await createApiGateway();
const res = await request(app)
.get('/cubejs-api/v1/meta?extended&onlyViews')
.set('Authorization', 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.e30.t-IDcSemACt8x4iTMCda8Yhe3iZaWbvV5XKSTbuAn0M')
.expect(200);
expect(res.body).toHaveProperty('cubes');
expect(res.body.cubes).toHaveLength(1);
expect(res.body.cubes[0].name).toBe('FooView');
});
test('meta endpoint extended with onlyViews returns only views', async () => {
const { app } = await createApiGateway();
const res = await request(app)
.get('/cubejs-api/v1/meta?extended&onlyViews')
.set('Authorization', 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.e30.t-IDcSemACt8x4iTMCda8Yhe3iZaWbvV5XKSTbuAn0M')
.expect(200);
expect(res.body).toHaveProperty('cubes');
expect(res.body.cubes).toHaveLength(1);
expect(res.body.cubes[0].name).toBe('FooView');
expect(res.body.cubes.every(c => c.name !== 'Foo')).toBe(true);
});

This makes it explicit that cubes (non-views) are excluded, not just that views are present.

Comment on lines +353 to +357
curl \
-H "Authorization: TOKEN" \
-G \
http://localhost:4000/cubejs-api/v1/meta?onlyViews
```
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.

Minor: the -G flag tells curl to append --data-urlencode params as query string, but here the query param is already embedded in the URL directly. The -G flag is harmless but redundant in this case. This matches the existing example above though, so it's consistent.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 4, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.14%. Comparing base (2f5bb8c) to head (27cf77b).
⚠️ Report is 4 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (2f5bb8c) and HEAD (27cf77b). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (2f5bb8c) HEAD (27cf77b)
cubesql 1 0
Additional details and impacted files
@@             Coverage Diff             @@
##           master   #10809       +/-   ##
===========================================
- Coverage   83.36%   58.14%   -25.22%     
===========================================
  Files         254      216       -38     
  Lines       75354    16884    -58470     
  Branches        0     3424     +3424     
===========================================
- Hits        62818     9818    -53000     
+ Misses      12536     6563     -5973     
- Partials        0      503      +503     
Flag Coverage Δ
cube-backend 58.14% <100.00%> (?)
cubesql ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@vasilev-alex vasilev-alex merged commit e38fc38 into master May 4, 2026
38 checks passed
@vasilev-alex vasilev-alex deleted the only-views-qp branch May 4, 2026 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

javascript Pull requests that update Javascript code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants