Ticket: Implementar V2 findAllTransferencia#389
Open
mateusmrosa wants to merge 1 commit intohomolfrom
Open
Conversation
Contributor
WalkthroughA new DTO for advanced transfer filtering and pagination was introduced. The controller now exposes a Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant Service
participant Prisma
Client->>Controller: GET /transferencia/v2?filters
Controller->>Service: findAllTransferenciaV2(filters, user)
Service->>Service: _getWhereClauseForFindAll(filters, user)
Service->>Prisma: transferencia.findMany(where, select, skip, take)
Prisma-->>Service: Transfer records
Service->>Service: _mapTransferenciaToDto(records)
Service->>Service: encodePageToken(filters, total_rows)
Service-->>Controller: PaginatedWithPagesDto<TransferenciaDto>
Controller-->>Client: PaginatedWithPagesDto<TransferenciaDto>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/src/casa-civil/transferencia/dto/filter-transferencia-v2.dto.ts(1 hunks)backend/src/casa-civil/transferencia/transferencia.controller.ts(2 hunks)backend/src/casa-civil/transferencia/transferencia.service.ts(5 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: `filtroparapagina` atualiza `route.query` somente no `submit` do formulário; consequentemente, watch...
Learnt from: robsonsobral
PR: AppCivico/smae#348
File: frontend/src/views/parlamentares/ParlamentaresLista.vue:43-55
Timestamp: 2025-06-11T00:41:40.158Z
Learning: `FiltroParaPagina` atualiza `route.query` somente no `submit` do formulário; consequentemente, watchers sobre esses parâmetros disparam apenas por submissão, não por digitação.
Applied to files:
backend/src/casa-civil/transferencia/transferencia.service.ts
🧬 Code Graph Analysis (1)
backend/src/casa-civil/transferencia/transferencia.controller.ts (3)
backend/src/casa-civil/transferencia/entities/transferencia.dto.ts (1)
TransferenciaDto(18-44)backend/src/casa-civil/transferencia/dto/filter-transferencia-v2.dto.ts (1)
FilterTransferenciaV2Dto(6-50)backend/src/auth/models/PessoaFromJwt.ts (1)
PessoaFromJwt(16-249)
🔇 Additional comments (3)
backend/src/casa-civil/transferencia/transferencia.controller.ts (1)
38-47: LGTM! The new V2 endpoint follows established patterns.The implementation correctly reuses the same authentication and authorization decorators as the existing endpoint, maintaining consistency in security controls.
backend/src/casa-civil/transferencia/dto/filter-transferencia-v2.dto.ts (1)
6-50: Well-structured DTO with proper validation constraints.The DTO correctly implements validation decorators and transformations. The 500-item limit for
ippis a good safeguard against potential performance issues.backend/src/casa-civil/transferencia/transferencia.service.ts (1)
1120-1160: Good refactoring to extract common logic.The extraction of WHERE clause building, select clause, and mapping logic into helper methods improves code maintainability and ensures consistency between the V1 and V2 endpoints.
Comment on lines
+1917
to
+1959
| async findAllTransferenciaV2( | ||
| filters: FilterTransferenciaV2Dto, | ||
| user: PessoaFromJwt | ||
| ): Promise<PaginatedWithPagesDto<TransferenciaDto>> { | ||
| const ipp = filters.ipp ?? 25; | ||
| const page = filters.pagina ?? 1; | ||
| const skip = (page - 1) * ipp; | ||
| let total_registros = 0; | ||
| let token_paginacao = filters.token_paginacao; | ||
| const filtersForHash = { ...filters }; | ||
| delete filtersForHash.pagina; | ||
| delete filtersForHash.token_paginacao; | ||
| if (token_paginacao) { | ||
| const decoded = this.decodePageToken(token_paginacao, filtersForHash); | ||
| total_registros = decoded.total_rows; | ||
| } else { | ||
| const where = await this._getWhereClauseForFindAll(filters, user); | ||
| total_registros = await this.prisma.transferencia.count({ where }); | ||
| token_paginacao = this.encodePageToken(filtersForHash, total_registros); | ||
| } | ||
| const where = await this._getWhereClauseForFindAll(filters, user); | ||
| const rows = await this.prisma.transferencia.findMany({ | ||
| where, | ||
| orderBy: [{ ano: 'desc' }, { identificador_nro: 'desc' }], | ||
| skip: skip, | ||
| take: ipp, | ||
| select: this._getTransferenciaSelectClause(), | ||
| }); | ||
|
|
||
| const total_paginas = Math.ceil(total_registros / ipp); | ||
|
|
||
| const linhas = rows.map((r) => this._mapTransferenciaToDto(r)); | ||
|
|
||
| return { | ||
| total_registros: total_registros, | ||
| paginas: total_paginas, | ||
| pagina_corrente: page, | ||
| token_paginacao: token_paginacao, | ||
| tem_mais: page < total_paginas, | ||
| token_ttl: PAGINATION_TOKEN_TTL, | ||
| linhas: linhas, | ||
| }; | ||
| } |
Contributor
There was a problem hiding this comment.
Optimize to avoid redundant database queries.
The method executes redundant queries that impact performance:
- The WHERE clause is built twice (lines 1933 and 1937)
- The count query runs even when
total_registrosis retrieved from the token (line 1934)
Apply this optimization:
async findAllTransferenciaV2(
filters: FilterTransferenciaV2Dto,
user: PessoaFromJwt
): Promise<PaginatedWithPagesDto<TransferenciaDto>> {
const ipp = filters.ipp ?? 25;
const page = filters.pagina ?? 1;
const skip = (page - 1) * ipp;
let total_registros = 0;
let token_paginacao = filters.token_paginacao;
const filtersForHash = { ...filters };
delete filtersForHash.pagina;
delete filtersForHash.token_paginacao;
+
+ const where = await this._getWhereClauseForFindAll(filters, user);
+
if (token_paginacao) {
const decoded = this.decodePageToken(token_paginacao, filtersForHash);
total_registros = decoded.total_rows;
} else {
- const where = await this._getWhereClauseForFindAll(filters, user);
total_registros = await this.prisma.transferencia.count({ where });
token_paginacao = this.encodePageToken(filtersForHash, total_registros);
}
- const where = await this._getWhereClauseForFindAll(filters, user);
const rows = await this.prisma.transferencia.findMany({
where,
orderBy: [{ ano: 'desc' }, { identificador_nro: 'desc' }],
skip: skip,
take: ipp,
select: this._getTransferenciaSelectClause(),
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async findAllTransferenciaV2( | |
| filters: FilterTransferenciaV2Dto, | |
| user: PessoaFromJwt | |
| ): Promise<PaginatedWithPagesDto<TransferenciaDto>> { | |
| const ipp = filters.ipp ?? 25; | |
| const page = filters.pagina ?? 1; | |
| const skip = (page - 1) * ipp; | |
| let total_registros = 0; | |
| let token_paginacao = filters.token_paginacao; | |
| const filtersForHash = { ...filters }; | |
| delete filtersForHash.pagina; | |
| delete filtersForHash.token_paginacao; | |
| if (token_paginacao) { | |
| const decoded = this.decodePageToken(token_paginacao, filtersForHash); | |
| total_registros = decoded.total_rows; | |
| } else { | |
| const where = await this._getWhereClauseForFindAll(filters, user); | |
| total_registros = await this.prisma.transferencia.count({ where }); | |
| token_paginacao = this.encodePageToken(filtersForHash, total_registros); | |
| } | |
| const where = await this._getWhereClauseForFindAll(filters, user); | |
| const rows = await this.prisma.transferencia.findMany({ | |
| where, | |
| orderBy: [{ ano: 'desc' }, { identificador_nro: 'desc' }], | |
| skip: skip, | |
| take: ipp, | |
| select: this._getTransferenciaSelectClause(), | |
| }); | |
| const total_paginas = Math.ceil(total_registros / ipp); | |
| const linhas = rows.map((r) => this._mapTransferenciaToDto(r)); | |
| return { | |
| total_registros: total_registros, | |
| paginas: total_paginas, | |
| pagina_corrente: page, | |
| token_paginacao: token_paginacao, | |
| tem_mais: page < total_paginas, | |
| token_ttl: PAGINATION_TOKEN_TTL, | |
| linhas: linhas, | |
| }; | |
| } | |
| async findAllTransferenciaV2( | |
| filters: FilterTransferenciaV2Dto, | |
| user: PessoaFromJwt | |
| ): Promise<PaginatedWithPagesDto<TransferenciaDto>> { | |
| const ipp = filters.ipp ?? 25; | |
| const page = filters.pagina ?? 1; | |
| const skip = (page - 1) * ipp; | |
| let total_registros = 0; | |
| let token_paginacao = filters.token_paginacao; | |
| const filtersForHash = { ...filters }; | |
| delete filtersForHash.pagina; | |
| delete filtersForHash.token_paginacao; | |
| const where = await this._getWhereClauseForFindAll(filters, user); | |
| if (token_paginacao) { | |
| const decoded = this.decodePageToken(token_paginacao, filtersForHash); | |
| total_registros = decoded.total_rows; | |
| } else { | |
| total_registros = await this.prisma.transferencia.count({ where }); | |
| token_paginacao = this.encodePageToken(filtersForHash, total_registros); | |
| } | |
| const rows = await this.prisma.transferencia.findMany({ | |
| where, | |
| orderBy: [{ ano: 'desc' }, { identificador_nro: 'desc' }], | |
| skip: skip, | |
| take: ipp, | |
| select: this._getTransferenciaSelectClause(), | |
| }); | |
| const total_paginas = Math.ceil(total_registros / ipp); | |
| const linhas = rows.map((r) => this._mapTransferenciaToDto(r)); | |
| return { | |
| total_registros: total_registros, | |
| paginas: total_paginas, | |
| pagina_corrente: page, | |
| token_paginacao: token_paginacao, | |
| tem_mais: page < total_paginas, | |
| token_ttl: PAGINATION_TOKEN_TTL, | |
| linhas: linhas, | |
| }; | |
| } |
🤖 Prompt for AI Agents
In backend/src/casa-civil/transferencia/transferencia.service.ts between lines
1917 and 1959, the method redundantly builds the WHERE clause twice and performs
a count query even when total_registros is already obtained from the decoded
token. To fix this, build the WHERE clause once before the conditional check,
then use it for both the count query and the findMany query. Also, skip the
count query if total_registros is retrieved from the token to avoid unnecessary
database calls.
Comment on lines
+2070
to
+2080
| private decodePageToken(jwt: string, filters: object): TransferenciaPageTokenJwtBody { | ||
| try { | ||
| const decoded = this.jwtService.verify(jwt) as TransferenciaPageTokenJwtBody; | ||
| if (decoded.search_hash !== Object2Hash(filters)) { | ||
| throw new Error('Filter criteria changed during pagination.'); | ||
| } | ||
| return decoded; | ||
| } catch (error) { | ||
| throw new HttpException(`Token de paginação inválido ou expirado. ${error.message}`, 400); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Avoid exposing internal error details in user-facing messages.
Line 2078 includes the raw error message in the HttpException, which could expose internal implementation details to the client.
Apply this fix to sanitize the error message:
private decodePageToken(jwt: string, filters: object): TransferenciaPageTokenJwtBody {
try {
const decoded = this.jwtService.verify(jwt) as TransferenciaPageTokenJwtBody;
if (decoded.search_hash !== Object2Hash(filters)) {
throw new Error('Filter criteria changed during pagination.');
}
return decoded;
} catch (error) {
- throw new HttpException(`Token de paginação inválido ou expirado. ${error.message}`, 400);
+ throw new HttpException('Token de paginação inválido ou expirado.', 400);
}
}🤖 Prompt for AI Agents
In backend/src/casa-civil/transferencia/transferencia.service.ts around lines
2070 to 2080, the catch block includes the raw error message in the
HttpException, potentially exposing internal details. Modify the catch block to
throw a generic error message without appending error.message, ensuring the
client receives a sanitized, user-friendly message instead of internal error
details.
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
Improvements