Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(API): Imports: Achats: ajout d'appels à Validata #4949

Open
wants to merge 6 commits into
base: staging
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
siret;description;fournisseur;date;prix_ht;famille_produits;caracteristiques;definition_local
82399356058716;"Pommes vertes"; Le bon traiteur;2022-05-02; 90.11; PRODUITS_LAITIERS;"BIO, LOCAL"; DEPARTMENT
82399356058716;"Pommes vertes"; Le bon traiteur;2022-05-02;90.11; PRODUITS_LAITIERS;"BIO, LOCAL"; DEPARTMENT
2 changes: 1 addition & 1 deletion api/tests/test_import_failures.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ def test_purchase_import_file_too_big(self):
self._assertImportFailureCreated(authenticate.user, ImportType.PURCHASE, file_path)

@authenticate
def test_purchase_import_erros(self):
def test_purchase_import_errors(self):
CanteenFactory.create(siret="82399356058716", managers=[authenticate.user])
CanteenFactory.create(siret="36462492895701")
file_path = "./api/tests/files/achats/purchases_bad.csv"
Expand Down
46 changes: 21 additions & 25 deletions api/tests/test_import_purchases.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ def test_import_purchases_different_separators(self):
with open("./api/tests/files/achats/purchases_good_separator_semicolon.csv") as purchase_file:
response = self.client.post(reverse("import_purchases"), {"file": purchase_file})
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(Purchase.objects.count(), 2)
self.assertEqual(Purchase.objects.count(), 1 + 1)

@authenticate
@override_settings(CSV_PURCHASE_CHUNK_LINES=1)
Expand Down Expand Up @@ -232,43 +232,39 @@ def test_import_bad_purchases(self):
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(Purchase.objects.count(), 0)
errors = response.json()["errors"]
self.assertEqual(errors.pop(0)["message"], "Champ 'siret' : Le siret de la cantine ne peut pas être vide")
self.assertEqual(
errors.pop(0)["message"], "Une cantine avec le siret « 86180597100897 » n'existe pas sur la plateforme."
)
self.assertEqual(errors.pop(0)["message"], "Vous n'êtes pas un gestionnaire de cette cantine.")
self.assertEqual(
errors.pop(0)["message"], "Champ 'description du produit' : La description ne peut pas être vide"
)
self.assertEqual(errors.pop(0)["message"], "Champ 'fournisseur' : Le fournisseur ne peut pas être vide")
self.assertEqual(errors.pop(0)["message"], "Champ 'date' : La date ne peut pas être vide")
self.assertEqual(errors.pop(0)["message"], "Une valeur doit être renseignée.")
# self.assertEqual(
# errors.pop(0)["message"], "Une cantine avec le siret « 86180597100897 » n'existe pas sur la plateforme."
# )
# self.assertEqual(errors.pop(0)["message"], "Vous n'êtes pas un gestionnaire de cette cantine.")
self.assertEqual(errors.pop(0)["message"], "Une valeur doit être renseignée.")
self.assertEqual(errors.pop(0)["message"], "Une valeur doit être renseignée.")
self.assertEqual(errors.pop(0)["message"], "Une valeur doit être renseignée.")
self.assertEqual(
errors.pop(0)["message"],
"Champ 'date' : Le format de date de la valeur «\xa02022-02-31\xa0» est correct (AAAA-MM-JJ), mais la date n’est pas valide.",
"La date doit être écrite sous la forme `aaaa-mm-jj`.",
)
self.assertEqual(
errors.pop(0)["message"],
"Champ 'date' : Le format de date de la valeur «\xa02022/03/01\xa0» n’est pas valide. Le format correct est AAAA-MM-JJ.",
"La date doit être écrite sous la forme `aaaa-mm-jj`.",
)
self.assertEqual(errors.pop(0)["message"], "Champ 'prix HT' : Le prix ne peut pas être vide")
self.assertEqual(
errors.pop(0)["message"], "Champ 'prix HT' : La valeur «\xa0A price\xa0» doit être un nombre décimal."
self.assertEqual(errors.pop(0)["message"], "Une valeur doit être renseignée.")
self.assertTrue(
errors.pop(0)["message"].startswith("A price ne respecte pas le motif imposé (expression régulière")
)
self.assertEqual(
errors.pop(0)["message"],
"Champ 'famille de produits' : La valeur «\xa0'NOPE'\xa0» n’est pas un choix valide.",
self.assertTrue(
errors.pop(0)["message"].startswith("NOPE ne respecte pas le motif imposé (expression régulière"),
)
self.assertEqual(
errors.pop(0)["message"],
"Champ 'caractéristiques' : L'élément n°2 du tableau n’est pas valide\xa0: La valeur «\xa0'NOPE'\xa0» n’est pas un choix valide.",
self.assertTrue(
errors.pop(0)["message"].startswith("BIO,NOPE ne respecte pas le motif imposé (expression régulière"),
)
self.assertEqual(
errors.pop(0)["message"],
"Champ 'définition de local' : La définition de local est obligatoire pour les produits locaux",
'Row at position "15" has a missing cell in field "caracteristiques" at position "7"',
)
self.assertEqual(
errors.pop(0)["message"],
"Format fichier : 8 colonnes attendues, 6 trouvées.",
'Row at position "15" has a missing cell in field "definition_local" at position "8"',
)

@authenticate
Expand Down Expand Up @@ -352,7 +348,7 @@ def test_import_file_many_errors(self):
self.assertEqual(Purchase.objects.count(), 0)
body = response.json()
self.assertEqual(len(body["errors"]), 30)
self.assertEqual(body["errorCount"], 56)
self.assertEqual(body["errorCount"], 48)

@authenticate
def test_encoding_autodetect_windows1252(self):
Expand Down
27 changes: 16 additions & 11 deletions api/views/purchaseimport.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

from api.permissions import IsAuthenticated
from api.serializers import PurchaseSerializer
from common.api.validata import process_errors, validate_file_against_schema
from common.utils import file_import
from common.utils.siret import normalise_siret
from data.models import Canteen, ImportFailure, ImportType, Purchase
Expand All @@ -41,15 +42,20 @@ def __init__(self, **kwargs):
self.is_duplicate_file = False
self.duplicate_purchases = []
self.duplicate_purchase_count = 0
self.data_schema = json.load(open("data/schemas/imports/achats.json"))
self.expected_header = [field["name"] for field in self.data_schema["fields"]]
self.schema_url = (
"https://raw.githubusercontent.com/betagouv/ma-cantine/refs/heads/staging/data/schemas/imports/achats.json"
Copy link
Collaborator

Choose a reason for hiding this comment

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

On peut aussi en profiter pour refactor la fonction validate en passant le fichier directement
voir code compl'alim

Copy link
Member Author

Choose a reason for hiding this comment

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

dans ton exemple le schema est aussi une url il me semble ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oui pardon, j'ai confondu avec le fichier

)
self.schema_json = json.load(open("data/schemas/imports/achats.json"))
self.expected_header = [field["name"] for field in self.schema_json["fields"]]
super().__init__(**kwargs)

def post(self, request):
self.start = time.time()
logger.info("Purchase bulk import started")
try:
self.file = request.data["file"]

# Step 1: Format validation
file_import.validate_file_size(self.file)
file_import.validate_file_format(self.file)

Expand All @@ -59,6 +65,14 @@ def post(self, request):
self.dialect = file_import.get_csv_file_dialect(self.file)
file_import.verify_first_line_is_header(self.file, self.dialect, self.expected_header)

# Step 2: Schema validation (Validata)
report = validate_file_against_schema(self.file, self.schema_url)
self.errors = process_errors(report)
if len(self.errors):
self._log_error("Echec lors de la validation du fichier (schema achats.json - Validata)")
return self._get_success_response()

# Step 3: ma-cantine validation (permissions, last checks...) + import
with transaction.atomic():
self._process_file()

Expand Down Expand Up @@ -175,18 +189,9 @@ def _create_purchase_for_canteen(self, siret, row):
raise PermissionDenied(detail="Vous n'êtes pas un gestionnaire de cette cantine.")

description = row.pop(0)
if description == "":
raise ValidationError({"description": "La description ne peut pas être vide"})
provider = row.pop(0)
if provider == "":
raise ValidationError({"provider": "Le fournisseur ne peut pas être vide"})
date = row.pop(0)
if date == "":
raise ValidationError({"date": "La date ne peut pas être vide"})

price = row.pop(0).strip().replace(",", ".")
if price == "":
raise ValidationError({"price_ht": "Le prix ne peut pas être vide"})

# We try to round the price. If we can't, we will let Django's field validation
# manage the error - hence the `pass` in the exception handler
Expand Down
29 changes: 29 additions & 0 deletions common/api/validata.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import requests


def validate_file_against_schema(file, schema_url):
# Reset the file pointer to the beginning
file.seek(0)
response = requests.post(
"https://api.validata.etalab.studio/validate",
files={
"file": ("file.csv", file.read(), file.content_type),
},
data={"schema": schema_url, "header_case": True},
)
return response.json()["report"]


def process_errors(report):
Copy link
Member Author

Choose a reason for hiding this comment

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

question à trancher : est-ce qu'on pre-process les erreurs Validata ici (dans le backend), ou coté frontend ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Côté front +1 !

errors = []
for error in report["tasks"][0]["errors"]:
errors.append(
{
"row": error["rowNumber"],
"status": 400,
"column": error["fieldName"],
"cell": error["cell"],
"message": error["message"],
}
)
return errors
Loading