màj wiki avec retour MES lot-5 AD
This commit is contained in:
Arthur Ria
@@ -0,0 +1,190 @@
|
||||
---
|
||||
title: "Code Review Process (Code, Functional, Documentation)"
|
||||
type: operation
|
||||
sources:
|
||||
- sources/archives/Revue_Code.md
|
||||
- sources/archives/Revue_Fonctionnel.md
|
||||
- sources/archives/Revue_Documentation.md
|
||||
related:
|
||||
- operations/git-workflow.md
|
||||
- operations/git-branch-lifecycle.md
|
||||
- operations/custom-application-management.md
|
||||
- operations/development-methodology.md
|
||||
last_compiled: "2026-04-17"
|
||||
---
|
||||
|
||||
# Code Review Process (Code, Functional, Documentation)
|
||||
|
||||
## Overview
|
||||
|
||||
Procédure interne Mecalux EasyWMS France pour la **revue d'une tâche de développement custom** avant intégration sur `develop`. La revue se fait sur **trois axes** complémentaires, généralement par des reviewers distincts (parfois cumulés) :
|
||||
|
||||
1. **Revue Code** — qualité technique, conventions de nommage, identification du custom (`CST_`), gestion des `null`, erreurs classiques sur le modèle Outbound/Kit
|
||||
2. **Revue Fonctionnel** — exécution des cas de test Jira, comportement des touches `Enter` / `Échap` sur les dialogues
|
||||
3. **Revue Documentation** — Jira commentée, présence des éléments dans la branche Git, mise à jour du **reten** (manuel de retention en Markdown — cf. [development-methodology](development-methodology.md))
|
||||
|
||||
Sources : Confluence EasyWMS France — *Revue - Code* (v10, 08/01/2026), *Revue - Fonctionnel* (v2, 27/02/2024), *Revue - Documentation* (v3, 04/03/2024).
|
||||
|
||||
> Référentiel global Mecalux Espagne : [code review checklist](https://msscc.mecalux.com/documentation/Development/master/ES/map_development_concepts/code_review/index.md), [nomenclature](https://msscc.mecalux.com/documentation/Development/master/ES/map_development_concepts/nomenclature/index.md), [null management best practices](https://msscc.mecalux.com/documentation/Development/master/ES/map_application_development/null_management_best_practices/index.md).
|
||||
|
||||
---
|
||||
|
||||
## 1. Revue Code
|
||||
|
||||
### 1.1 Pièges sur les requêtes Outbound
|
||||
|
||||
**`OutboundLines` vs `OutboundOrderLines`** :
|
||||
|
||||
> ⚠️ Sur les requêtes en **writing**, utiliser **`OutboundLines`** (pas `OutboundOrderLines`).
|
||||
|
||||
Avec `OutboundOrderLines`, dès qu'une ligne d'ordre de sortie est annulée, on obtient dans les logs :
|
||||
|
||||
```
|
||||
Le nombre de ligne d'ordre d'expédition ne peut pas être négatif
|
||||
```
|
||||
|
||||
### 1.2 Kits sans assemblage : `OutboundOrderOutboundOrderLineDetails`
|
||||
|
||||
Si le projet utilise les **kits sans assemblage** (cf. [kits](../concepts/kits.md)) :
|
||||
|
||||
| Propriété | Contenu |
|
||||
|---|---|
|
||||
| `OutboundOrderOutboundOrderLineDetails` | **Tous les composants** du kit |
|
||||
| `OutboundLines` | **Uniquement le kit lui-même** |
|
||||
|
||||
### 1.3 `OutboundLine.ProductConversion` peut être `null`
|
||||
|
||||
> ⚠️ Sur les requêtes en **writing**, l'attribut `ProductConversion` d'une ligne d'ordre de sortie peut être `null`.
|
||||
|
||||
Cas : la commande demande un **support spécifique** sans article — on a alors le **code support renseigné mais pas l'article**, donc pas de conversion produit.
|
||||
|
||||
### 1.4 Identification du custom dans le code
|
||||
|
||||
**Délimiter les blocs de code custom** ajoutés au milieu de code standard :
|
||||
|
||||
```csharp
|
||||
potentiel code standard
|
||||
|
||||
//StartCustom
|
||||
Code custom
|
||||
//EndCustom
|
||||
|
||||
potentiel code standard
|
||||
```
|
||||
|
||||
### 1.5 Préfixe `CST_` sur les éléments custom
|
||||
|
||||
Chaque nouvel élément ou élément modifié doit être **préfixé `CST_`**. Trois questions à se poser :
|
||||
|
||||
| Question | Règle |
|
||||
|---|---|
|
||||
| L'élément est-il **visible par le client** ? | **Pas de préfixe** (ex : les paramètres SmartUI ne prennent pas `CST_`) |
|
||||
| Peut-on **modifier le nom** de l'élément ? | Un workflow non en mode **"full edition"** ne permet pas de modifier le nom des éléments |
|
||||
| Est-ce l'élément de **plus haut niveau** ? | Dans un **workflow custom**, seul le **workflow** porte le préfixe ; ses sous-éléments ne le portent pas. Inversement dans un **workflow standard**, **tous** les éléments ajoutés/modifiés prennent le préfixe |
|
||||
|
||||
> ⚠️ **À ne pas oublier :**
|
||||
> - Un **dialogue est modifié** même si on ne change que l'**implémentation de l'activité**
|
||||
> - Les **transitions** doivent aussi être identifiées
|
||||
>
|
||||
> ⚠️ Lors du passage d'un workflow de **"partial overriden" → "overriden" (full edition)**, on **perd l'identification de couleur** des activités/transitions modifiées. Il faut alors préfixer **toutes** les activités modifiées par `CST_` (ouvrir le workflow dans sa version précédente en parallèle pour ne rien oublier).
|
||||
|
||||
### 1.6 Nommage des éléments
|
||||
|
||||
| Type d'élément | Règle de nommage |
|
||||
|---|---|
|
||||
| **ViewField** d'une vue | 2 préfixes + code du viewfield → `ViewField_<Vue>_<Champ>` (ex : `ViewField_OutboundOrderVList_OutboundOrderStatus`) |
|
||||
| **Paramètres SmartUI** | Préfixer du process impacté, en MAJUSCULES (ex : `EXPEDITION_ALLOWED_CONTAINER`) |
|
||||
| **Attributs d'un workflow** | Commencent par une **minuscule** (ne pas oublier `CST_` dans un workflow standard) |
|
||||
| **Paramètres formels d'un workflow** | Commencent par une **majuscule** |
|
||||
| **Paramètres d'un dialogue** | Commencent par une **majuscule** |
|
||||
| **Paramètres d'une requête** | Commencent par une **minuscule** |
|
||||
|
||||
### 1.7 Pas de "code en dur" dans les workflows
|
||||
|
||||
Éviter les constantes en dur. En particulier, les actions **"Enter"** et **"Escape"** après un dialogue doivent utiliser :
|
||||
|
||||
| Variable | Usage |
|
||||
|---|---|
|
||||
| `ProcessContext.EnterAction` | Tester l'appui sur `Entrée` |
|
||||
| `ProcessContext.EscapeAction` | Tester l'appui sur `Échap` |
|
||||
|
||||
### 1.8 Gestion des `NullReferenceException`
|
||||
|
||||
Particulièrement : **`FirstOrDefault()`** doit être sécurisé.
|
||||
|
||||
```csharp
|
||||
// ❌ Dangereux — exception si aucun ordre n'existe
|
||||
Context.OutboundOrders.FirstOrDefault(s => s.Code == outboundOrderCode).OutboundLines
|
||||
|
||||
// ✅ Sécurisé
|
||||
var order = Context.OutboundOrders.FirstOrDefault(s => s.Code == outboundOrderCode);
|
||||
if (order != null) { ... }
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 2. Revue Fonctionnelle
|
||||
|
||||
### 2.1 Exécution des cas de test
|
||||
|
||||
Exécuter **tous les cas de test fonctionnel** décrits dans la tâche Jira. À défaut, exécuter les **cas standard d'utilisation du custom**.
|
||||
|
||||
### 2.2 Tester `Échap` sur les dialogues
|
||||
|
||||
Pour chaque dialogue ajouté ou modifié, tester le comportement de la touche **`Échap`** en se posant la question :
|
||||
|
||||
> Que veut faire l'opérateur en pressant Échap ?
|
||||
> - **Revenir à l'écran précédent** ?
|
||||
> - **Passer à la suite du process** ?
|
||||
> - **Ne rien faire** ?
|
||||
|
||||
Le comportement par défaut peut ne pas être celui attendu fonctionnellement → bien tester explicitement.
|
||||
|
||||
> Lié à la règle **1.7** ci-dessus : `ProcessContext.EscapeAction` doit être utilisé pour intercepter `Échap` proprement.
|
||||
|
||||
---
|
||||
|
||||
## 3. Revue Documentation
|
||||
|
||||
### 3.1 Jira
|
||||
|
||||
Vérifier que **les éléments modifiés** sont décrits et que les modifications sont **explicites dans les commentaires** de la tâche.
|
||||
|
||||
### 3.2 Git
|
||||
|
||||
Vérifier la **présence des éléments modifiés** dans la branche Git concernée (cf. [git-branch-lifecycle](git-branch-lifecycle.md)).
|
||||
|
||||
> Vérification utile : l'**historique des commits** Git du développeur pour s'assurer qu'il n'a **pas inclus de modifications hors périmètre** de sa tâche. Ce cas survient quand le dev a été réalisé **avec une custom app qui n'était pas au même "niveau"** que la branche sur laquelle il l'exporte (typiquement : création de la branche **après** import de la custom app dans le Builder, alors que d'autres commits ont eu lieu entre temps).
|
||||
>
|
||||
> Cf. [custom-application-management](custom-application-management.md) — règles d'import/export pour éviter ce cas.
|
||||
|
||||
### 3.3 Reten
|
||||
|
||||
Vérifier que la description de la tâche est **présente dans le reten** avec **l'intégralité des éléments modifiés**.
|
||||
|
||||
| Cas | Localisation dans le reten |
|
||||
|---|---|
|
||||
| Le custom **modifie le comportement d'un process** | Décrit dans les **chapitres de process** (Entries, Exits, Picking, etc.) avec **description fonctionnelle, technique et éléments custom** |
|
||||
| Le custom **ne modifie pas le comportement d'un process** | Présent dans les **tableaux d'éléments custom en fin de reten**, avec description de la modification |
|
||||
| Des **custom attributes** ont été utilisés | Listés dans la partie **"1.2 General custom elements"** du reten — et **idéalement aussi dans la tâche Jira liée** au custom quand il s'agit d'un process custom |
|
||||
|
||||
> Le reten est un manuel de retention en **Markdown**, géré dans le repo Git du projet (cf. [development-methodology](development-methodology.md#manuel-de-reten)).
|
||||
|
||||
---
|
||||
|
||||
## Common errors (revue détecte)
|
||||
|
||||
- **Erreur "Le nombre de ligne d'ordre d'expédition ne peut pas être négatif"** dans les logs → utilisation de `OutboundOrderLines` au lieu de `OutboundLines` sur une requête writing
|
||||
- **NullReferenceException** sur des chaînages `.FirstOrDefault(...).Property` → assigner d'abord à une variable + tester `null`
|
||||
- **Workflow "overriden" sans préfixe `CST_`** sur les activités modifiées → relire le workflow précédent en parallèle pour identifier toutes les modifs
|
||||
- **Élément hors périmètre de la tâche dans le commit** → custom app importée à un niveau différent de la branche ; ré-importer après pull/rebase
|
||||
- **Reten non mis à jour** → bloquant pour la revue documentation (le projet ne pourra pas être maintenu post-MEP sans le reten)
|
||||
- **`Échap` sur dialogue produit un comportement involontaire** → manquait un test fonctionnel `ProcessContext.EscapeAction`
|
||||
|
||||
## Related
|
||||
|
||||
- [Git Workflow (Git Flow)](git-workflow.md) — workflow Git Flow où s'insèrent les revues (avant `Finish Feature`)
|
||||
- [Git Branch Lifecycle](git-branch-lifecycle.md) — la revue est faite avant l'intégration dans `develop` (phase 1)
|
||||
- [Custom Application Management](custom-application-management.md) — règles d'import/export pour éviter les modifs hors périmètre
|
||||
- [Development Methodology](development-methodology.md) — vue d'ensemble (reten, branches, custom apps)
|
||||
- [Kits](../concepts/kits.md) — utile pour comprendre les pièges sur `OutboundOrderOutboundOrderLineDetails` (kits sans assemblage)
|
||||
Reference in New Issue
Block a user