🕵️‍♂️ Guide pratique sur la revue de code

La revue de code est une pratique courante en développement logiciel. Elle consiste à examiner le code d'un collègue (ou d'une autre équipe) afin d'identifier des problèmes avant qu'ils n'atteignent la production (application live). Une bonne revue de code permet de détecter des bugs, des failles de sécurité, des problèmes de performance et des violations des bonnes pratiques d'architecture.

Ce guide vous accompagne dans la rédaction d'issues de qualité suite à une revue de code.

Activité en classe (bug fonctionnel)

Identifiez le bug et la source du bug dans l'application suivante:

🔗 Accédez à l'application d'exemple pour l'activité en classe

Activité en classe (Problème de code)

Voici le code pour un contrôleur d'API permettant de faire la gestion de clients et afficher le profil d'un client dans une application. Le code présente certaines lacunes au niveau de l'architecture et des bonnes pratiques, identifiez-les!

using Microsoft.AspNetCore.Mvc;
using System.Threading.Tasks;
using MonApplication.Services;
using MonApplication.Repositories;
using MonApplication.Models;

namespace MonApplication.Controllers
{
    [ApiController]
    [Route("api/[controller]")]
    public class ClientController : ControllerBase
    {
        private readonly IClientService _clientService;
        private readonly IClientRepository _clientRepository;
        private readonly ICommandeRepository _commandeRepository;

        public ClientController(
            IClientService clientService,
            IClientRepository clientRepository,
            ICommandeRepository commandeRepository)
        {
            _clientService = clientService;
            _clientRepository = clientRepository;
            _commandeRepository = commandeRepository;
        }

        [HttpGet("{id}")]
        public async Task<IActionResult> ObtenirClient(int id)
        {
            var client = await _clientService.ObtenirClientAsync(id);

            if (client == null)
            {
                return NotFound();
            }

            return Ok(client);
        }

        [HttpGet("{id}/profil-complet")]
        public async Task<IActionResult> ObtenirProfilComplet(int id)
        {
            // On a besoin de plus d'infos que ce que le service retourne
            var client = await _clientRepository.ObtenirParIdAsync(id);

            if (client == null)
            {
                return NotFound();
            }

            var commandes = await _commandeRepository.ObtenirParClientIdAsync(id);

            return Ok(new
            {
                client.Id,
                client.Prenom,
                client.Nom,
                client.Email,
                client.NumeroTelephone,
                client.DateNaissance,
                client.NumeroAssuranceSociale,
                client.NumeroCarte,
                client.DateExpirationCarte,
                client.AdresseFacturation,
                client.MotDePasseHash,
                NombreCommandes = commandes.Count,
                DerniereCommande = commandes.OrderByDescending(c => c.DateCreation).FirstOrDefault()
            });
        }

        [HttpPut("{id}")]
        public async Task<IActionResult> ModifierClient(int id, [FromBody] ClientUpdateRequest request)
        {
            var client = await _clientRepository.ObtenirParIdAsync(id);

            if (client == null)
            {
                return NotFound();
            }

            client.Prenom = request.Prenom;
            client.Nom = request.Nom;
            client.Email = request.Email;
            client.NumeroTelephone = request.NumeroTelephone;
            client.AdresseFacturation = request.AdresseFacturation;

            await _clientRepository.MettreAJourAsync(client);

            return Ok(client);
        }

        [HttpDelete("{id}")]
        public async Task<IActionResult> SupprimerClient(int id)
        {
            var client = await _clientRepository.ObtenirParIdAsync(id);

            if (client == null)
            {
                return NotFound();
            }

            await _clientRepository.SupprimerAsync(client);

            return Ok("Client supprimé");
        }

        [HttpGet("recherche")]
        public async Task<IActionResult> RechercherClients([FromQuery] string terme)
        {
            var clients = await _clientRepository.ObtenirTousAsync();

            var resultats = clients
                .Where(c => c.Nom.Contains(terme) || 
                            c.Email.Contains(terme) || 
                            c.NumeroTelephone.Contains(terme))
                .ToList();

            return Ok(resultats);
        }
    }

    public class ClientUpdateRequest
    {
        public string Prenom { get; set; }
        public string Nom { get; set; }
        public string Email { get; set; }
        public string NumeroTelephone { get; set; }
        public string AdresseFacturation { get; set; }
    }
}

Les types de problèmes à identifier

Lors d'une revue de code, vous pouvez identifier deux grandes catégories de problèmes:

1. Bug fonctionnel

Un bug fonctionnel est un problème où le code ne fait pas ce qu'il devrait faire. Le comportement observé diffère du comportement attendu.

Exemples:

Un calcul mathématique incorrect
Une condition logique inversée
Une variable non mise à jour
Un cas limite non géré
Une erreur qui ne devrait pas apparaitre

Caractéristiques:

Reproductible avec des étapes précises
Mesurable (on peut comparer le résultat obtenu vs attendu)
Souvent découvert en testant l'application

2. Problème de code (architecture, sécurité, performance, maintenabilité)

Un problème de code est un problème où le code fonctionne, mais est mal conçu. L'application semble fonctionner correctement, mais le code présente des risques ou des défauts de conception.

Exemples:

Exposition de données sensibles via une API
Contournement de l'architecture en couches
Code dupliqué
Absence de validation des entrées
Requêtes inefficaces (performance)

Caractéristiques:

Pas toujours visible en testant l'application
Nécessite une lecture attentive du code
Impact potentiel sur la sécurité, la maintenabilité ou la performance

Une issue = Un problème

Ne jamais mélanger plusieurs problèmes dans une même issue.

Même si vous identifiez plusieurs problèmes dans le même fichier ou la même méthode, créez une issue distincte pour chacun. Cela permet:

Un suivi clair de chaque correctif
Une priorisation indépendante
Une revue de code plus simple lors de la correction
Un historique propre dans le projet

Comment nommer une issue

Le titre de l'issue doit être court, précis et descriptif. Il doit permettre de comprendre le problème sans ouvrir l'issue.

Structure recommandée

Résumé du problème - Localisation

Exemples de bons titres

Exemple / Cas	Titre
Bug	`TPS non recalculée après application d'un escompte - FacturationService`
Sécurité	`Exposition du NAS et des données de carte de crédit - ClientController`
Architecture	`Contournement du service dans ObtenirProfilComplet - ClientController`
Performance	`Filtrage des clients en mémoire au lieu de la BD - ClientController`

Ces titres communiquent clairement le problème et l'endroit où ce problème est présent

Exemples de mauvais titres

Titre	Problème
`Bug dans les factures`	Trop vague, quel bug?
`Le code ne marche pas`	Non descriptif
`Problème`	Inutile
`ClientController.cs`	Décrit un fichier, pas un problème
`Plusieurs problèmes de sécurité et d'architecture`	Mélange plusieurs issues

Ces titres sont peu descriptifs et trop généraux, ils ne décrivent pas clairement le problème.

Gabarit Gitlab pour les issues

## Description

(Décrire le problème de façon concise.)

## Étapes pour reproduire

(Comment reproduire le problème (étapes, configurations, séquence, etc.)

## Quel est le comportement actuel?

(Ce qui se produit lorsque vous effectuez l'action)

## Quel est le comportement attendu?

(Ce qui devrait se produire ou ce que vous devriez voir)

## Captures d'écran et logs pertinents

(Ajoutez le ou les logs pertinents - utiliser les blocs de code markdown (```) pour formater le log ou le code.
Vous pouvez aussi inclure toute capture d'écran pertinente)

## Solutions possibles

(Si vous avez de l'information sur la cause du problème, n'hésitez pas à détailler.)

/label ~revue

Issue Templates

Vous pouvez créer des gabarits de issues (issue template) en suivant la procédure suivante: https://docs.gitlab.com/user/project/description_templates/#create-a-description-template.

Comment bien remplir une issue

Section: Description

Objectif: Expliquer le problème de façon concise mais complète.

Doit contenir:

Ce qui ne fonctionne pas ou ce qui est mal conçu
L'impact du problème (si pertinent)
Le contexte nécessaire à la compréhension

Exemple (bug):

## Description

La TPS n'est pas recalculée lorsqu'un escompte est appliqué à une facture. 
La TVQ est correctement recalculée sur le nouveau sous-total, mais la TPS 
reste calculée sur le montant original, ce qui génère un total erroné.

Exemple (sécurité):

## Description

L'endpoint `GET /api/client/{id}/profil-complet` expose des données hautement 
sensibles dans sa réponse JSON: numéro d'assurance sociale (NAS), numéro de 
carte de crédit, date d'expiration de la carte, et le hash du mot de passe. 
Ces données ne devraient pas être retournées par l'API.

À éviter:

"Le code ne marche pas"
"Il y a un bug"
"C'est mal codé"

Section : Étapes pour reproduire

Objectif: Permettre à quelqu'un d'autre de reproduire le problème sans poser de questions.

Doit contenir:

Des étapes numérotées et précises
Les données de test utilisées
Les configurations nécessaires (si applicable)

Exemple (bug):

## Étapes pour reproduire

Accéder au module Facturation
Ouvrir une facture existante (ex: FACT-2024-00847, sous-total de 5 200,00 $)
Dans le panneau "Appliquer un escompte", entrer 10%
Cliquer sur "Appliquer l'escompte"
Observer les montants de TPS et TVQ dans le récapitulatif

Exemple (code):

## Étapes pour reproduire

Démarrer l'application
Effectuer une requête GET sur `/api/client/1/profil-complet`
Observer la réponse JSON retournée

À éviter:

"Regarder le code"
"Tester l'application"
"Appliquer un escompte" (sans préciser où, comment, avec quelles valeurs)

Section: Comportement actuel

Objectif: Décrire factuellement ce qui se passe actuellement.

Doit contenir:

Des données concrètes (chiffres, messages, JSON)
Des captures d'écran si utile
Une description factuelle, sans jugement

Exemple (bug) :

## Quel est le comportement actuel?

Après application d'un escompte de 10% sur une facture de 5 200,00 $ :

| Ligne                     | Montant affiché |
| ------------------------- | --------------- |
| Sous-total                | 5 200,00 $      |
| Escompte (10%)            | - 520,00 $      |
| Sous-total après escompte | 4 680,00 $      |
| TPS (5%)                  | **260,00 $**    |
| TVQ (9,975%)              | 466,83 $        |
| **Total**                 | **5 406,83 $**  |

La TPS affichée (260,00 $) est calculée sur le sous-total original 
(5 200 $ × 5% = 260 $) au lieu du nouveau sous-total après escompte.

Exemple (code):

## Quel est le comportement actuel?

La réponse JSON contient des données sensibles qui ne devraient pas être exposées:

    ```json
    {
    "id": 1,
    "prenom": "Jean",
    "nom": "Tremblay",
    "numeroAssuranceSociale": "123 456 789",
    "numeroCarte": "4532XXXXXXXX1234",
    "dateExpirationCarte": "2026-08",
    "motDePasseHash": "a8f5f167f44f4964e6c998dee827110c..."
    }

**À éviter:**
- "Le montant est pas correct"
- "Le code expose des données sensibles" (sans préciser lesquelles)

### Section: Comportement attendu

**Objectif:** Décrire ce qui devrait se passer.

**Doit contenir:**
- Le résultat correct attendu
- Les calculs ou données de référence (si applicable)
- Un contraste clair avec le comportement actuel

**Exemple (bug):**
```markdown
## Quel est le comportement attendu?

La TPS devrait être recalculée sur le sous-total après escompte :

| Ligne                     | Montant attendu             |
| ------------------------- | --------------------------- |
| Sous-total                | 5 200,00 $                  |
| Escompte (10%)            | - 520,00 $                  |
| Sous-total après escompte | 4 680,00 $                  |
| TPS (5%)                  | **234,00 $** (4 680 $ × 5%) |
| TVQ (9,975%)              | 466,83 $                    |
| **Total**                 | **5 380,83 $**              |

L'écart est de 26,00 $ par facture.

À éviter:

"Le montant devrait être correct"
Répéter la description sans apporter de valeur

Section: Captures d'écran et logs pertinents

Objectif: Fournir des preuves et faciliter le diagnostic.

Peut contenir:

Extraits de code problématique (avec numéros de lignes)
Calculs de vérification
Messages d'erreur
Captures d'écran

Exemple (bug):

## Captures d'écran et logs pertinents

[img](./img/screenshot-bug.png)

Exemple (code):

## Captures d'écran et logs pertinents

Extrait du code problématique (`ClientController.cs`, lignes 52-59):

```csharp
return Ok(new
{
    // ...
    client.NumeroAssuranceSociale,  // À retirer
    client.NumeroCarte,              // À retirer
    client.DateExpirationCarte,      // À retirer
    client.MotDePasseHash,           // À retirer
    // ...
});
```

À éviter:

"(aucun)"
Oublier cette section alors qu'elle apporterait de la clarté

Section: Solutions possibles

Objectif: Proposer une piste de correction si vous en avez une.

Cette section est optionnelle mais très appréciée. Elle montre que vous avez analysé le problème en profondeur.

Exemple (bug):

## Solutions possibles

Dans `FacturationService.cs`, méthode `AppliquerEscompteAsync`, ajouter 
le recalcul de la TPS avant le calcul du total :

```csharp
// Ligne 85-86 actuelles:
facture.TVQ = nouveauSousTotal * TAUX_TVQ;
facture.Total = nouveauSousTotal + facture.TPS + facture.TVQ;

// Correction suggérée:
facture.TPS = nouveauSousTotal * TAUX_TPS;  // Ajouter cette ligne
facture.TVQ = nouveauSousTotal * TAUX_TVQ;
facture.Total = nouveauSousTotal + facture.TPS + facture.TVQ;
```

Exemple (code) :

## Solutions possibles

Créer un DTO qui définit explicitement les propriétés 
à exposer.

```csharp
public class ClientProfilDto
{
    public int Id { get; set; }
    public string Prenom { get; set; }
    public string Nom { get; set; }
    public string Email { get; set; }
    public string NumeroTelephone { get; set; }
}
```

Mapper l'entité vers le DTO.

```csharp
return Ok(new ClientProfilDto
{
    Id = client.Id,
    Prenom = client.Prenom,
    Nom = client.Nom,
    Email = client.Email,
    NumeroTelephone = client.NumeroTelephone
});
```

À éviter:

"Corriger le code"
"Je sais pas"

Checklist avant de soumettre une issue

Avant de soumettre votre issue, vérifiez :

Le titre est clair et descriptif
L'issue ne traite que d'UN SEUL problème
Les étapes de reproduction sont précises et numérotées
Le comportement actuel contient des données concrètes
Le comportement attendu est clairement différencié
Le code problématique est identifié (fichier, méthode, lignes)
Le ton est professionnel et factuel (pas accusateur)

Exemple complet: Issue bien rédigée (bug)

## Description

La TPS n'est pas recalculée lorsqu'un escompte est appliqué à une facture. 
La TVQ est correctement recalculée sur le nouveau sous-total, mais la TPS 
reste calculée sur le montant original, ce qui génère un total erroné.

## Étapes pour reproduire

1. Accéder au module Facturation
2. Ouvrir une facture existante
3. Dans le panneau "Appliquer un escompte", entrer 10%
4. Cliquer sur "Appliquer l'escompte"
5. Observer les montants de TPS et TVQ dans le récapitulatif

## Quel est le comportement actuel?

Après application d'un escompte de 10% sur une facture de 5 200,00 $ :

| Ligne                     | Montant affiché |
| ------------------------- | --------------- |
| Sous-total après escompte | 4 680,00 $      |
| TPS (5%)                  | **260,00 $**    |
| TVQ (9,975%)              | 466,83 $        |
| **Total**                 | **5 406,83 $**  |

La TPS (260,00 $) est calculée sur 5 200 $ au lieu de 4 680 $.

## Quel est le comportement attendu?

| Ligne                     | Montant attendu |
| ------------------------- | --------------- |
| Sous-total après escompte | 4 680,00 $      |
| TPS (5%)                  | **234,00 $**    |
| TVQ (9,975%)              | 466,83 $        |
| **Total**                 | **5 380,83 $**  |

Écart : 26,00 $ par facture.

## Captures d'écran et logs pertinents

Code problématique dans `FacturationService.cs`, lignes 85-86 :

```csharp
facture.TVQ = nouveauSousTotal * TAUX_TVQ;
facture.Total = nouveauSousTotal + facture.TPS + facture.TVQ;
// facture.TPS n'est jamais recalculée!
```

## Solutions possibles

Ajouter le recalcul de la TPS :

```csharp
facture.TPS = nouveauSousTotal * TAUX_TPS;
facture.TVQ = nouveauSousTotal * TAUX_TVQ;
facture.Total = nouveauSousTotal + facture.TPS + facture.TVQ;
```

/label ~revue

Conclusion

Une bonne issue de revue de code doit permettre à quelqu'un qui ne connaît pas le projet de :

Comprendre le problème en lisant le titre et la description
Reproduire le problème en suivant les étapes
Vérifier le problème en comparant le comportement actuel et attendu
Localiser le problème dans le code
Corriger le problème grâce aux pistes de solution

Prenez le temps de bien rédiger vos issues. Une issue bien rédigée fait gagner du temps à toute l'équipe!

Activité en classe (bug fonctionnel)​

🔗 Accédez à l'application d'exemple pour l'activité en classe

Activité en classe (Problème de code)​

Les types de problèmes à identifier​

1. Bug fonctionnel​

2. Problème de code (architecture, sécurité, performance, maintenabilité)​

Une issue = Un problème​

Comment nommer une issue​

Structure recommandée​

Exemples de bons titres​

Exemples de mauvais titres​

Gabarit Gitlab pour les issues​

Comment bien remplir une issue​

Section: Description​

Section : Étapes pour reproduire​

Section: Comportement actuel​

Section: Captures d'écran et logs pertinents​

Section: Solutions possibles​

Checklist avant de soumettre une issue​

Exemple complet: Issue bien rédigée (bug)​

Conclusion​

Activité en classe (bug fonctionnel)

Activité en classe (Problème de code)

Les types de problèmes à identifier

1. Bug fonctionnel

2. Problème de code (architecture, sécurité, performance, maintenabilité)

Une issue = Un problème

Comment nommer une issue

Structure recommandée

Exemples de bons titres

Exemples de mauvais titres

Gabarit Gitlab pour les issues

Comment bien remplir une issue

Section: Description

Section : Étapes pour reproduire

Section: Comportement actuel

Section: Captures d'écran et logs pertinents

Section: Solutions possibles

Checklist avant de soumettre une issue

Exemple complet: Issue bien rédigée (bug)

Conclusion