11 - Code Review Automatico con Copilot

# Nella pagina Settings del repository:
# 1. Vai a Settings > Code review > Copilot
# 2. Seleziona "Automatic" sotto "Copilot code review"
# 3. Scegli il trigger:
#    - "All pull requests" - review automatica su ogni PR
#    - "When requested" - solo quando richiesto manualmente

# Per abilitare via API GitHub:
# PATCH /repos/{{ '{' }}owner}/{{ '{' }}repo}
# {{ '{' }}
#   "copilot_code_review": {{ '{' }}
#     "automatic": true,
#     "trigger": "all_pull_requests"
#   }
# }

# A livello di organizzazione (per tutti i repository):
# Organization Settings > Copilot > Policies > Code Review
# Enable: "Automatic code review for all repositories"

# .github/workflows/copilot-review.yml
name: Request Copilot Code Review

on:
  pull_request:
    types: [opened, synchronize]
    branches:
      - main
      - release/*
    paths:
      - 'src/**'
      - 'lib/**'
      - '!src/**/*.test.ts'
      - '!src/**/*.spec.ts'
      - '!docs/**'

jobs:
  request-review:
    runs-on: ubuntu-latest
    steps:
      - name: Request Copilot Review
        uses: actions/github-script@v7
        with:
          script: |
            // Richiedi review solo se la PR ha più di 10 righe modificate
            const {{ '{' }} data: files } = await github.rest.pulls.listFiles({{ '{' }}
              owner: context.repo.owner,
              repo: context.repo.repo,
              pull_number: context.payload.pull_request.number
            });

            const totalChanges = files.reduce((sum, f) =>
              sum + f.additions + f.deletions, 0);

            if (totalChanges > 10) {{ '{' }}
              await github.rest.pulls.requestReviewers({{ '{' }}
                owner: context.repo.owner,
                repo: context.repo.repo,
                pull_number: context.payload.pull_request.number,
                reviewers: ['copilot']  // Richiedi review a Copilot
              });
              console.log(`Requested Copilot review for PR #${{ '{' }}context.payload.pull_request.number}`);
            } else {{ '{' }}
              console.log(`Skipped: only ${{ '{' }}totalChanges} lines changed`);
            }

# Copilot Code Review Instructions

## Standard di Codice del Team

### TypeScript
- Usa `strict: true` in tsconfig.json
- Mai usare `any`: preferisci `unknown` con type guard
- Preferisci `interface` a `type` per oggetti
- Usa `readonly` per proprietà che non devono cambiare
- Enumera sempre i casi in switch su union type
- Usa optional chaining (`?.`) invece di check null manuali
- Preferisci `const` a `let`, mai `var`

### Naming Conventions
- Classi e interfacce: PascalCase (es. `UserService`, `IUserRepository`)
- Variabili e funzioni: camelCase (es. `getUserById`, `isActive`)
- Costanti: UPPER_SNAKE_CASE (es. `MAX_RETRY_COUNT`)
- File: kebab-case (es. `user-service.ts`)
- Test: [name].test.ts o [name].spec.ts
- Boolean: prefisso is/has/should (es. `isActive`, `hasPermission`)

### Error Handling
- Usa custom error classes (AppError, NotFoundError, etc.)
- Non catturare errori senza ri-lanciarli o loggarli
- Includi context nell'errore: `throw new NotFoundError('User', userId)`
- Non usare try/catch per flow control
- Tutti gli endpoint API devono avere error handler

### Sicurezza (PRIORITA ALTA)
- Mai concatenare input utente in query SQL
- Validare TUTTI gli input con class-validator
- Sanitizzare output HTML per prevenire XSS
- Verificare autorizzazione per ogni endpoint protetto
- Non esporre stack trace o dettagli interni in risposte API
- Rate limiting su endpoint pubblici
- Secrets solo via environment variables

### Performance
- Evitare query N+1: usa include/eager loading
- Paginazione obbligatoria per liste: max 100 elementi
- Caching per dati frequentemente accessi
- Async/await per I/O operations
- Evitare operazioni sincrone bloccanti

### Test
- Ogni nuova funzione pubblica deve avere almeno un test
- Test devono essere indipendenti (no ordine di esecuzione)
- Mock solo le dipendenze esterne (database, API, file system)
- Nomi descrittivi: "should [action] when [condition]"
- Almeno un test per il caso di errore

### Angular (Frontend)
- Usa standalone components
- Signals per lo state management
- OnPush change detection per tutti i componenti
- Lazy loading per route non critiche
- Nessun `subscribe()` nei template: usa async pipe o toSignal

# Regole specifiche per linguaggio

## Python (Backend ML)
- Type hints obbligatorie per tutti i parametri e return
- Docstring Google style per ogni funzione pubblica
- Usa `dataclasses` o `pydantic` per data objects
- Black formatting (line length 88)
- Import ordinati con isort

## SQL (Migrations)
- Mai DROP TABLE senza backup plan documentato
- ALTER TABLE deve essere reversibile
- Index per ogni foreign key
- Nomi colonne in snake_case
- Commento SQL per ogni migration spiegando il "perché"

## YAML/Config
- Commenti per ogni sezione non ovvia
- Secrets referenziati via variabili, mai valori hardcoded
- Validazione schema per file di configurazione

# Pattern Architetturali

## Layered Architecture
Il progetto segue un'architettura a layer:
- Controller → Service → Repository
- I controller NON devono accedere direttamente ai repository
- I repository NON devono contenere business logic
- Ogni layer comunica solo con il layer immediatamente inferiore

## Dependency Injection
- Tutte le dipendenze iniettate via costruttore
- Interfacce per le dipendenze (IUserService, IUserRepository)
- No singleton pattern manuale: usare il DI container

## Event-Driven Communication
- Comunicazione cross-service via eventi
- Eventi devono essere immutabili
- Ogni evento ha un tipo unico e un payload tipizzato
- Handler degli eventi devono essere idempotenti

## VERIFICA:
- Se un controller chiama direttamente un repository: ERRORE
- Se un service istanzia direttamente una dipendenza: WARNING
- Se una query SQL appare fuori da un repository: ERRORE
- Se business logic appare in un controller: WARNING

// File: src/repositories/user.repository.ts
// Riga aggiunta nella PR:

async findByName(name: string): Promise<User[]> {{ '{' }}
  const query = `SELECT * FROM users WHERE name LIKE '%${{ '{' }}name}%'`;
  return this.db.raw(query);
}

async findByName(name: string): Promise<User[]> {{ '{' }}
  return this.db.query(
    'SELECT * FROM users WHERE name LIKE $1',
    [`%${{ '{' }}name}%`]
  );
}

// File: src/services/order.service.ts

async getOrdersWithDetails(userId: string): Promise<OrderDetail[]> {{ '{' }}
  const orders = await this.orderRepo.findByUserId(userId);

  const result = [];
  for (const order of orders) {{ '{' }}
    const items = await this.itemRepo.findByOrderId(order.id);
    const customer = await this.customerRepo.findById(order.customerId);
    result.push({{ '{' }} ...order, items, customer });
  }

  return result;
}

async getOrdersWithDetails(userId: string): Promise<OrderDetail[]> {{ '{' }}
  const orders = await this.orderRepo.findByUserId(userId);

  if (orders.length === 0) return [];

  const orderIds = orders.map(o => o.id);
  const customerIds = [...new Set(orders.map(o => o.customerId))];

  // Batch query: 3 query totali invece di 2N+1
  const [allItems, allCustomers] = await Promise.all([
    this.itemRepo.findByOrderIds(orderIds),
    this.customerRepo.findByIds(customerIds)
  ]);

  const itemsByOrder = new Map();
  for (const item of allItems) {{ '{' }}
    if (!itemsByOrder.has(item.orderId)) {{ '{' }}
      itemsByOrder.set(item.orderId, []);
    }
    itemsByOrder.get(item.orderId).push(item);
  }

  const customersById = new Map(
    allCustomers.map(c => [c.id, c])
  );

  return orders.map(order => ({{ '{' }}
    ...order,
    items: itemsByOrder.get(order.id) || [],
    customer: customersById.get(order.customerId)
  }));
}

// File: src/controllers/payment.controller.ts

async processPayment(req: Request, res: Response) {{ '{' }}
  try {{ '{' }}
    const result = await this.paymentService.charge(
      req.body.amount,
      req.body.cardToken
    );
    res.json(result);
  } catch (e) {{ '{' }}
    console.log(e);
    res.status(500).json({{ '{' }} error: 'Something went wrong' });
  }
}

async processPayment(req: Request, res: Response) {{ '{' }}
  // Validazione input
  const {{ '{' }} amount, cardToken } = req.body;

  if (!amount || typeof amount !== 'number' || amount <= 0) {{ '{' }}
    return res.status(400).json({{ '{' }}
      error: 'Invalid amount',
      detail: 'Amount must be a positive number'
    });
  }

  if (!cardToken || typeof cardToken !== 'string') {{ '{' }}
    return res.status(400).json({{ '{' }}
      error: 'Invalid card token',
      detail: 'Card token is required'
    });
  }

  try {{ '{' }}
    const result = await this.paymentService.charge(amount, cardToken);
    res.json(result);
  } catch (error) {{ '{' }}
    logger.error('Payment processing failed', {{ '{' }}
      amount,
      error: error.message,
      stack: error.stack,
      requestId: req.id
    });

    if (error instanceof PaymentDeclinedError) {{ '{' }}
      return res.status(402).json({{ '{' }}
        error: 'Payment declined',
        detail: error.declineReason
      });
    }

    if (error instanceof ValidationError) {{ '{' }}
      return res.status(400).json({{ '{' }}
        error: 'Validation error',
        detail: error.message
      });
    }

    res.status(500).json({{ '{' }}
      error: 'Internal server error',
      requestId: req.id  // Per debug senza esporre dettagli
    });
  }
}

# Branch Protection Rules consigliate per integrare Copilot

# Settings > Branches > Branch protection rules > main

# Regole obbligatorie:
# 1. Require pull request reviews: 1 reviewer minimo
# 2. Require review from Code Owners: abilitato
# 3. Dismiss stale reviews: abilitato (quando nuovi push invalidano review)
# 4. Require status checks: abilitato
#    - Checks richiesti:
#      - ci/build (CI pipeline)
#      - ci/test (test suite)
#      - security/codeql (analisi statica)
# 5. Require Copilot code review: abilitato (opzionale)
#
# Nota: Copilot review può essere richiesta come check
# obbligatorio, ma si consiglia di NON renderla bloccante
# per evitare falsi positivi che rallentano il merge.
#
# Strategia consigliata:
# - Copilot review: obbligatoria ma non bloccante
# - Reviewer umano: obbligatorio e bloccante
# - CI/CD checks: obbligatori e bloccanti