Skip to content

fix: missing permissions viewId args when ignoreViewQuery is true#1490

Merged
boris-w merged 5 commits into
developfrom
fix/ignore-view-query-permissions-viewid
Apr 29, 2025
Merged

fix: missing permissions viewId args when ignoreViewQuery is true#1490
boris-w merged 5 commits into
developfrom
fix/ignore-view-query-permissions-viewid

Conversation

@boris-w
Copy link
Copy Markdown
Contributor

@boris-w boris-w commented Apr 29, 2025

No description provided.

@boris-w boris-w requested review from Copilot and tea-artist April 29, 2025 03:36
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes the issue where the permissions viewId argument is not properly set when ignoreViewQuery is true.

  • Introduces the ignoreViewQuery property into the query type.
  • Computes a conditional viewId based on the ignoreViewQuery flag.
  • Updates calls to permissions and prepareQuery to reflect the computed viewId.

this.knex.queryBuilder(),
{
viewId,
viewId: query.viewId,
Copy link

Copilot AI Apr 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you've computed a conditional viewId on line 450 based on ignoreViewQuery, update the permissions configuration to use the computed viewId instead of query.viewId. Consider replacing this line with 'viewId,' to maintain consistent behavior.

Suggested change
viewId: query.viewId,
viewId: viewId,

Copilot uses AI. Check for mistakes.
Comment on lines 539 to +540
const { dbTableName, queryBuilder, viewCte, filter, search, orderBy, groupBy, fieldMap } =
await this.prepareQuery(tableId, {
...query,
viewId: query.ignoreViewQuery ? undefined : query.viewId,
});
await this.prepareQuery(tableId, query);
Copy link

Copilot AI Apr 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The prepareQuery call now passes the original query object, so the computed viewId is not being used. Instead, pass the computed viewId (i.e. {...query, viewId}) so that ignoreViewQuery is correctly applied during query preparation.

Copilot uses AI. Check for mistakes.
@coveralls
Copy link
Copy Markdown
Collaborator

coveralls commented Apr 29, 2025

Pull Request Test Coverage Report for Build 14723345725

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 11 of 11 (100.0%) changed or added relevant lines in 1 file are covered.
  • 5 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.01%) to 80.376%

Files with Coverage Reduction New Missed Lines %
apps/nestjs-backend/src/features/base/BatchProcessor.class.ts 2 68.29%
apps/nestjs-backend/src/features/base/base-import-processor/base-import-csv.processor.ts 3 65.35%
Totals Coverage Status
Change from base Build 14703616199: -0.01%
Covered Lines: 36711
Relevant Lines: 45674

💛 - Coveralls

@boris-w boris-w merged commit c28703b into develop Apr 29, 2025
11 checks passed
@boris-w boris-w deleted the fix/ignore-view-query-permissions-viewid branch April 29, 2025 04:28
@github-actions
Copy link
Copy Markdown
Contributor

🧹 Preview Environment Cleanup

tea-artist added a commit that referenced this pull request Mar 23, 2026
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.

3 participants