Option pour remplacer un ingrédient demandé par un ingrédient existant #1334

hfroot · 2024-12-02T17:44:04Z

Dans le scope : fonctionnalité pour trouver un ingrédient de même type et remplacer une demande avec.

Hors du scope : ajout de synonyme en même temps; remplacement d'une demande par un ingrédient d'autre type (par ex microorganisme avec plante), à faire à la main pour l'instant

Control de la demande

Il y a une explication quand le remplacement est bloqué au cause du type

Quand la demande a été remplacée :

MAJ du badge pour les instructrices

NB: Les déclarants vont voir le nom du remplacement si ils vont regarder la déclaration.

Calculation de substances

Le tableau de substances est déjà automatiquement recalculé. Cette PR ajoute un avertissement au dessus si il y a des remplacements qui ont des substances.

NB: pour l'instant, cet avertissement est toujours affiché pour l'instructrice. Question: est-ce que c'est acceptable en premier temps ?

This is to cut down on the size of the file as more actions are added

… button

This is a simplification of the needs to reduce the scope of the PR

…ing with element of wrong type

…tion summary view

This is an initial simple implementation that needs to be updated to hide alert once recalculation has happened

hfroot · 2024-12-19T21:39:06Z

api/serializers/declaration.py

@@ -103,7 +103,23 @@ def update(self, instance, validated_data):
 )


-class DeclaredIngredientCommonSerializer(PrivateFieldsSerializer):
+class DeclaredElementNestedField:


Cette refacto avait du sens quand j'ai commencé la PR. Maintenant, elle paraît un peu hors scope mais je l'ai laissée dedans car je trouve que c'est cool quand même de ne pas dupliquer la logique de create

hfroot · 2024-12-19T21:42:54Z

api/tests/test_declaration.py

@@ -1638,7 +1638,9 @@ def test_get_single_declared_plant(self):
        declaration = DeclarationFactory()
        plant = DeclaredPlantFactory(declaration=declaration)

-        response = self.client.get(reverse("api:declared_plant", kwargs={"pk": plant.id}), format="json")
+        response = self.client.get(
+            reverse("api:declared_element", kwargs={"pk": plant.id, "type": "plant"}), format="json"


Dans cette PR, j'ai du changer l'API de REST vers RPC pour avoir un meilleur contrôle sur les actions de demande de plus d'infos (fait dans une PR avant), refus (aussi dans une autre PR), et le remplacement (le cible de cette PR).

Surtout, la refacto nous donne la possibilité de ne pas envoyer le statut et aussi de modifier des champs liés, comme mettre le champ new à False

Pendant ce changement, j'ai remarqué que c'est possible de changer l'API pour reduire la duplication de code entre 4 types d'element et 4 actions par demande.

Alors ici, on voit que j'ai remplacé l'api:declared_plant avec api:declared_element... "type": "plant"

D'accord avec toi, ces actions semblent être plus transactionnelles qu'autre chose, du coup une approche RPC me semble plus adapté 👍

hfroot · 2024-12-19T21:44:06Z

api/tests/test_declaration.py

-            reverse("api:declared_microorganism", kwargs={"pk": microorganism.id}),
-            {"requestStatus": DeclaredMicroorganism.AddableStatus.INFORMATION, "requestPrivateNotes": "some notes"},
+        response = self.client.post(
+            reverse("api:declared_element_request_info", kwargs={"pk": microorganism.id, "type": "microorganism"}),


avec la refacto, un appel REST vers api:declared_x est devenu un appel vers api:declared_x_action

hfroot · 2024-12-19T21:44:39Z

api/tests/test_declaration.py

+        self.assertEqual(microorganism.request_status, DeclaredMicroorganism.AddableStatus.INFORMATION)
+
+    @authenticate
+    def test_reject_declared_element(self):


Il y avait pas un test pour le refus d'ingrédient avant

hfroot · 2024-12-19T21:45:20Z

api/tests/test_declaration.py

@@ -1753,3 +1808,46 @@ def test_status_visible_to_instructor(self):
        m = declared_microorganisms[0]
        self.assertIn("requestStatus", m)
        self.assertIn("requestPrivateNotes", m)
+
+    @authenticate
+    def test_replace_declared_plant(self):


enfin on arrive aux tests ajoutés pour vérifier la vraie nouvelle fonctionnalité de cette PR 🎉

hfroot · 2024-12-19T21:47:07Z

api/urls.py

@@ -21,18 +21,22 @@
    path("elements/autocomplete/", views.AutocompleteView.as_view(), name="substance_autocomplete"),
    # Declared elements
    path("new-declared-elements/", views.DeclaredElementsView.as_view(), name="list_new_declared_elements"),
-    path("declared-elements/plants/<int:pk>", views.DeclaredPlantView.as_view(), name="declared_plant"),
+    path("declared-elements/<str:type>/<int:pk>", views.DeclaredElementView.as_view(), name="declared_element"),


maintenant le nouveau api c'est:

pour lire: declared-elements/<type>/<id>
pour prendre une decision: declared-elements/<type>/<id>/<action name>

(avant c'etait declared-elements/<type en pluriel>/<id> REST)

hfroot · 2024-12-19T21:51:13Z

api/views/declaration/declared_element.py

-    permission_classes = [(IsInstructor | IsVisor)]
-    serializer_class = DeclaredPlantSerializer
-    queryset = DeclaredPlant.objects.all()
+class ElementMappingMixin:


au lieu d'avoir une vue dédiée pour chaque type d'element, ce mixin nous donne les clés pour avoir une vue par besoin (Retrieve, Refus, Demande d'infos, Remplacement, etc...)

hfroot · 2024-12-19T21:52:44Z

frontend/src/components/DeclarationSummary/index.vue

@@ -77,6 +77,15 @@
    />

    <p class="font-bold mt-8">Substances contenues dans la composition :</p>
+    <DsfrAlert v-if="replacedRequestsWithSubstances.length" type="warning" class="mb-4">


est-ce que c'est un pb que cet avertissement va rester même après l'aller-retour instructrice - déclarant - instructrice ?

Peut-être on pourra ajuster le message 🤔

« Les ingrédients suivantes, ajoutés pour remplacer une demande, rajoutent des substances dans la composition.
Veuillez vérifier que les doses totales des substances restent pertinentes. »

Ou enfin qqchose du genre, sans forcément dire qu'il faut toujours contacter le pro.

hfroot · 2024-12-19T21:59:44Z

frontend/src/components/ElementAutocomplete.vue

-        @blur="hasFocus = false"
-        @keydown="checkKeyboardNav($event)"
-      />
+    <div :class="hideSearchButton ? '' : 'flex items-end'">


avant, quand hideSearchButton est false le label du champ DsfrInput (quand donné) est affiché à côté du champ et non pas au dessus. Ce changement repare ça

hfroot · 2024-12-19T22:01:13Z

frontend/src/views/DeclaredElementPage/ElementAlert.vue

@@ -0,0 +1,48 @@
+<template>


Pendant une refacto, j'ai créé ce fichier pour améliorer le focus du code principal dans index.vue

alemangui · 2024-12-27T15:04:34Z

api/serializers/declaration.py

+                validated_data[self.nested_field_name] = self.nested_model.objects.get(pk=element.get("id"))
+            except self.nested_model.DoesNotExist:
+                raise ProjectAPIException(
+                    field_errors={f"declared_{self.nested_field_name}s": "L'ingrédient spécifiée n'existe pas."}


Il faudra adapter aussi le message de l'exception non ? Genre

field_errors={f"declared_{self.nested_field_name}s": f"L'objet « ${self.nested_field_name} » spécifiée n'existe pas."}

alemangui · 2024-12-27T15:07:08Z

api/tests/test_declaration.py

@@ -1638,7 +1638,9 @@ def test_get_single_declared_plant(self):
        declaration = DeclarationFactory()
        plant = DeclaredPlantFactory(declaration=declaration)

-        response = self.client.get(reverse("api:declared_plant", kwargs={"pk": plant.id}), format="json")
+        response = self.client.get(
+            reverse("api:declared_element", kwargs={"pk": plant.id, "type": "plant"}), format="json"


D'accord avec toi, ces actions semblent être plus transactionnelles qu'autre chose, du coup une approche RPC me semble plus adapté 👍

alemangui · 2024-12-27T15:10:39Z

api/tests/test_declaration.py

@@ -1753,3 +1808,46 @@ def test_status_visible_to_instructor(self):
        m = declared_microorganisms[0]
        self.assertIn("requestStatus", m)
        self.assertIn("requestPrivateNotes", m)
+


Est-ce que ça vaut la peine d'ajouter un test sur les permissions pour ces nouveaux endpoints ? Similaire à test_get_declared_element_not_allowed_not_instructor

alemangui · 2024-12-27T15:13:19Z

frontend/src/components/DeclarationSummary/index.vue

@@ -77,6 +77,15 @@
    />

    <p class="font-bold mt-8">Substances contenues dans la composition :</p>
+    <DsfrAlert v-if="replacedRequestsWithSubstances.length" type="warning" class="mb-4">


Peut-être on pourra ajuster le message 🤔

« Les ingrédients suivantes, ajoutés pour remplacer une demande, rajoutent des substances dans la composition.
Veuillez vérifier que les doses totales des substances restent pertinentes. »

Ou enfin qqchose du genre, sans forcément dire qu'il faut toujours contacter le pro.

alemangui · 2024-12-27T15:15:29Z

frontend/src/components/ElementAutocomplete.vue

@@ -85,6 +87,10 @@ const props = defineProps({
    type: Boolean,
    default: false,
  },
+  optional: {


Est-ce que ça serait plus cohérent avec l'API des inputs de le nommer required ? et donc par la suite dans la ligne 11 :required="required".

Surtout que de cette façon les autres composants utilisant ElementAutocomplete et n'envoyant pas explicitement la prop se retrouveront avec un changement de comportement : leur champ deviendra obligatoire.

alemangui · 2024-12-27T15:18:16Z

frontend/src/views/DeclaredElementPage/ResultCard.vue

+        <DsfrBadge v-if="result.novelFood" label="Novel Food" small type="new" />
+        <ElementStatusBadge v-if="result.status" :text="result.status" />
+      </div>
+      <!-- TODO: make this a tag and align with badges, check DSFR -->


D'ailleurs on essaie de faire les commentaires en français dans le code. On n'est pas non plus hyper strictes mais p-e pour les prochains mieux de le faire en FR pour avoir une certaine cohérence.

alemangui · 2024-12-27T15:19:20Z

frontend/src/views/DeclaredElementPage/index.vue

+              <div v-if="modalToOpen === 'replace'">
+                <p v-if="cannotReplace">
+                  Ce n'est pas possible pour l'instant de remplacer une demande avec un ingrédient d'un type different.
+                  Veuillez contacter l'équipe Compl'Alim pour résoudre le substitution.


Suggested change

Veuillez contacter l'équipe Compl'Alim pour résoudre le substitution.

Veuillez contacter l'équipe Compl'Alim pour effectuer la substitution.

alemangui · 2024-12-27T15:20:19Z

frontend/src/views/DeclaredElementPage/index.vue

    .json()
+  // TODO: is this handling errors nicely?


On peut le tester en envoyant un BadRequest programmatiquement, si tu veux on peut regarder ensemble

hfroot linked an issue Dec 3, 2024 that may be closed by this pull request

ETQ instructrice, je veux remplacer une demande d'ajout d'ingrédient avec un ingrédient qui existe en base #1320

Open

2 tasks

hfroot added 6 commits December 3, 2024 17:21

Move alert handling to separate file

dab1c7e

This is to cut down on the size of the file as more actions are added

Add first elements of UI for search of ingredient

b4594b1

Fetch element detail, display synonyms

ad3dd7c

Activate replace button when ingredient selected

e569633

Ability to add an existing ingredient to a declared one

fbc13dc

Add success replace alert with element link

803d024

hfroot force-pushed the replace-ingredient-request branch from 276a7b6 to 803d024 Compare December 3, 2024 16:43

hfroot added 2 commits December 3, 2024 18:57

Proper alignment when showing label of autocomplete as well as search…

3449deb

… button

Front: prevent demand being replaced with ingredient of different type

459cc1c

This is a simplification of the needs to reduce the scope of the PR

Base automatically changed from declared-element-note to staging December 5, 2024 10:30

hfroot added 12 commits December 5, 2024 15:24

Merge branch 'staging' into replace-ingredient-request

567f87f

Add merge migration

d595bb7

Refactor API to reduce code repetition

93fac1d

Refactor from REST to RPC

58246dc

Require passing replacement element type to avoid accidentally replac…

7a081f0

…ing with element of wrong type

Prevent the replacement cross-type

f88a103

Update front with new API and add request info and notes to response

705bdc1

When a demand is replaced, it is no longer new

60c77b3

Minimal solution for explaining the ingredient name change in declara…

c1371e4

…tion summary view

Recommend instructor to return declaration to recalculate substances

8863961

This is an initial simple implementation that needs to be updated to hide alert once recalculation has happened

Only show warning if replacement adds substances to composition

b5a217a

Merge branch 'staging' into replace-ingredient-request

5786381

hfroot commented Dec 19, 2024

View reviewed changes

Merge migration

e554e92

hfroot commented Dec 19, 2024

View reviewed changes

hfroot marked this pull request as ready for review December 19, 2024 22:02

alemangui added 2 commits December 27, 2024 15:59

Merge branch 'staging' into replace-ingredient-request

9f8b2ff

Adds merge migration

1e2566e

alemangui requested changes Dec 27, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Option pour remplacer un ingrédient demandé par un ingrédient existant #1334

Option pour remplacer un ingrédient demandé par un ingrédient existant #1334

hfroot commented Dec 2, 2024 •

edited

Loading

hfroot Dec 19, 2024

hfroot Dec 19, 2024 •

edited

Loading

alemangui Dec 27, 2024

hfroot Dec 19, 2024

hfroot Dec 19, 2024

hfroot Dec 19, 2024

hfroot Dec 19, 2024

hfroot Dec 19, 2024

hfroot Dec 19, 2024

alemangui Dec 27, 2024

hfroot Dec 19, 2024

hfroot Dec 19, 2024

alemangui Dec 27, 2024

alemangui Dec 27, 2024

alemangui Dec 27, 2024

alemangui Dec 27, 2024

alemangui Dec 27, 2024

alemangui Dec 27, 2024

alemangui Dec 27, 2024

alemangui Dec 27, 2024

	Veuillez contacter l'équipe Compl'Alim pour résoudre le substitution.
	Veuillez contacter l'équipe Compl'Alim pour effectuer la substitution.

Option pour remplacer un ingrédient demandé par un ingrédient existant #1334

Are you sure you want to change the base?

Option pour remplacer un ingrédient demandé par un ingrédient existant #1334

Conversation

hfroot commented Dec 2, 2024 • edited Loading

Control de la demande

MAJ du badge pour les instructrices

Calculation de substances

Choose a reason for hiding this comment

hfroot Dec 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hfroot commented Dec 2, 2024 •

edited

Loading

hfroot Dec 19, 2024 •

edited

Loading