Skip to content

fix(api): return original email without OAuth suffix in bookings#99

Open
tomerqodo wants to merge 2 commits into
qodo_action_req_1_base_fixapi_return_original_email_without_oauth_suffix_in_bookings_pr4from
qodo_action_req_1_head_fixapi_return_original_email_without_oauth_suffix_in_bookings_pr4
Open

fix(api): return original email without OAuth suffix in bookings#99
tomerqodo wants to merge 2 commits into
qodo_action_req_1_base_fixapi_return_original_email_without_oauth_suffix_in_bookings_pr4from
qodo_action_req_1_head_fixapi_return_original_email_without_oauth_suffix_in_bookings_pr4

Conversation

@tomerqodo

Copy link
Copy Markdown

Benchmark PR from agentic-review-benchmarks#4

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (2) 📎 Requirement gaps (0)

Grey Divider


Action required

1. output.service.ts naming violation 📘 Rule violation ✓ Correctness
Description
• The modified service file uses a dot-suffix filename (output.service.ts) and does not match the
  required <Entity>Service.ts / PascalCase filename convention.
• This breaks the repository/service naming convention, reducing discoverability and making it
  harder to reliably locate or refactor service implementations.
Code

apps/api/v2/src/ee/bookings/2024-08-13/services/output.service.ts[R93-99]

export class OutputBookingsService_2024_08_13 {
  constructor(private readonly bookingsRepository: BookingsRepository_2024_08_13) {}

+  private getDisplayEmail(email: string): string {
+    return email.replace(/\+[a-zA-Z0-9]{25}/, "");
+  }
+
Evidence
PR Compliance ID 7 requires service files to be named in the <Entity>Service.ts PascalCase pattern
and match the exported class name. The modified file path ends with output.service.ts (dot-suffix,
not PascalCase), while exporting OutputBookingsService_2024_08_13.

AGENTS.md
apps/api/v2/src/ee/bookings/2024-08-13/services/output.service.ts[92-99]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The modified service file name `output.service.ts` violates the required service naming convention (PascalCase `&lt;Entity&gt;Service.ts` with filename/class name alignment).

## Issue Context
Compliance requires consistent naming for service classes/files to maintain discoverability and predictable architecture.

## Fix Focus Areas
- apps/api/v2/src/ee/bookings/2024-08-13/services/output.service.ts[92-99]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. booking => missing parentheses 📘 Rule violation ✓ Correctness
Description
• New arrow functions omit parentheses around a single parameter (e.g., booking =>), which
  violates the Biome formatting requirement to always include arrow function parentheses.
• This creates inconsistent formatting and may cause CI/format checks to fail if Biome is enforced.
Code

apps/api/v2/src/ee/bookings/2024-08-13/services/output.service.ts[R414-417]

    const bookingsMap = new Map(databaseBookings.map(booking => [booking.id, booking]));
-    
+
    const transformed = bookingsIds.map(bookingId => {
      const databaseBooking = bookingsMap.get(bookingId);
Evidence
PR Compliance ID 9 mandates always using arrow function parentheses. The added code uses `booking
=> and bookingId =>` without parentheses.

AGENTS.md
apps/api/v2/src/ee/bookings/2024-08-13/services/output.service.ts[414-417]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Arrow functions were added without parentheses around parameters, violating the project&#x27;s Biome formatting standard.

## Issue Context
Compliance requires &quot;always use arrow function parentheses&quot; for consistent code formatting.

## Fix Focus Areas
- apps/api/v2/src/ee/bookings/2024-08-13/services/output.service.ts[414-417]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Recurring seated order nondeterministic 🐞 Bug ✓ Correctness
Description
getOutputRecurringSeatedBookings no longer sorts by start time, so the returned array order is
  whatever order bookingsIds happens to be in.
• The recurring booking IDs come from a Prisma findMany without orderBy, so the ID order is not
  guaranteed and can vary between runs/DBs.
• This is inconsistent with getOutputRecurringBookings (which sorts) and can break clients that
  expect recurring bookings to be chronological.
Code

apps/api/v2/src/ee/bookings/2024-08-13/services/output.service.ts[R411-418]

  async getOutputRecurringSeatedBookings(bookingsIds: number[], showAttendees: boolean) {
    const databaseBookings = await this.bookingsRepository.getByIdsWithAttendeesWithBookingSeatAndUserAndEvent(bookingsIds);
-    
+
    const bookingsMap = new Map(databaseBookings.map(booking => [booking.id, booking]));
-    
+
    const transformed = bookingsIds.map(bookingId => {
      const databaseBooking = bookingsMap.get(bookingId);
      if (!databaseBooking) {
Evidence
The seated recurring output now returns the transformed list without sorting, while the underlying
fetch for recurring bookings has no ordering guarantees. In contrast, the non-seated recurring path
explicitly sorts by start time, so seated and non-seated recurring responses can diverge in
ordering.

apps/api/v2/src/ee/bookings/2024-08-13/services/output.service.ts[411-425]
apps/api/v2/src/ee/bookings/2024-08-13/repositories/bookings.repository.ts[174-185]
apps/api/v2/src/ee/bookings/2024-08-13/services/bookings.service.ts[602-612]
apps/api/v2/src/ee/bookings/2024-08-13/services/output.service.ts[231-244]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`getOutputRecurringSeatedBookings` returns results without sorting, but its inputs originate from Prisma `findMany` calls without `orderBy`, making response order non-deterministic and inconsistent with the non-seated recurring booking response ordering.

### Issue Context
This can break clients that assume recurring bookings are returned in chronological order.

### Fix Focus Areas
- apps/api/v2/src/ee/bookings/2024-08-13/services/output.service.ts[411-425]
- apps/api/v2/src/ee/bookings/2024-08-13/repositories/bookings.repository.ts[174-185]
- apps/api/v2/src/ee/bookings/2024-08-13/services/output.service.ts[231-244]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



ⓘ The new review experience is currently in Beta. Learn more

Qodo Logo

Comment on lines 93 to +99
export class OutputBookingsService_2024_08_13 {
constructor(private readonly bookingsRepository: BookingsRepository_2024_08_13) {}

private getDisplayEmail(email: string): string {
return email.replace(/\+[a-zA-Z0-9]{25}/, "");
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. output.service.ts naming violation 📘 Rule violation ✓ Correctness

• The modified service file uses a dot-suffix filename (output.service.ts) and does not match the
  required <Entity>Service.ts / PascalCase filename convention.
• This breaks the repository/service naming convention, reducing discoverability and making it
  harder to reliably locate or refactor service implementations.
Agent prompt
## Issue description
The modified service file name `output.service.ts` violates the required service naming convention (PascalCase `<Entity>Service.ts` with filename/class name alignment).

## Issue Context
Compliance requires consistent naming for service classes/files to maintain discoverability and predictable architecture.

## Fix Focus Areas
- apps/api/v2/src/ee/bookings/2024-08-13/services/output.service.ts[92-99]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines 414 to 417
const bookingsMap = new Map(databaseBookings.map(booking => [booking.id, booking]));

const transformed = bookingsIds.map(bookingId => {
const databaseBooking = bookingsMap.get(bookingId);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

2. booking => missing parentheses 📘 Rule violation ✓ Correctness

• New arrow functions omit parentheses around a single parameter (e.g., booking =>), which
  violates the Biome formatting requirement to always include arrow function parentheses.
• This creates inconsistent formatting and may cause CI/format checks to fail if Biome is enforced.
Agent prompt
## Issue description
Arrow functions were added without parentheses around parameters, violating the project's Biome formatting standard.

## Issue Context
Compliance requires "always use arrow function parentheses" for consistent code formatting.

## Fix Focus Areas
- apps/api/v2/src/ee/bookings/2024-08-13/services/output.service.ts[414-417]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines 411 to 418
async getOutputRecurringSeatedBookings(bookingsIds: number[], showAttendees: boolean) {
const databaseBookings = await this.bookingsRepository.getByIdsWithAttendeesWithBookingSeatAndUserAndEvent(bookingsIds);

const bookingsMap = new Map(databaseBookings.map(booking => [booking.id, booking]));

const transformed = bookingsIds.map(bookingId => {
const databaseBooking = bookingsMap.get(bookingId);
if (!databaseBooking) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

3. Recurring seated order nondeterministic 🐞 Bug ✓ Correctness

getOutputRecurringSeatedBookings no longer sorts by start time, so the returned array order is
  whatever order bookingsIds happens to be in.
• The recurring booking IDs come from a Prisma findMany without orderBy, so the ID order is not
  guaranteed and can vary between runs/DBs.
• This is inconsistent with getOutputRecurringBookings (which sorts) and can break clients that
  expect recurring bookings to be chronological.
Agent prompt
### Issue description
`getOutputRecurringSeatedBookings` returns results without sorting, but its inputs originate from Prisma `findMany` calls without `orderBy`, making response order non-deterministic and inconsistent with the non-seated recurring booking response ordering.

### Issue Context
This can break clients that assume recurring bookings are returned in chronological order.

### Fix Focus Areas
- apps/api/v2/src/ee/bookings/2024-08-13/services/output.service.ts[411-425]
- apps/api/v2/src/ee/bookings/2024-08-13/repositories/bookings.repository.ts[174-185]
- apps/api/v2/src/ee/bookings/2024-08-13/services/output.service.ts[231-244]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

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.

1 participant