Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
49 changes: 45 additions & 4 deletions envergo/moulinette/tests/test_views_haie.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ def test_triage(client):
assert res.status_code == 200
content = res.content.decode()
assert "<h2>Doctrine du département</h2>" in content
assert Event.objects.get(
category="simulateur", event="localisation", metadata__user_type="anonymous"
)


@pytest.mark.urls("config.urls_haie")
Expand All @@ -83,6 +86,9 @@ def test_triage_result(client):
content = res.content.decode()
assert "Votre projet n'est pas encore pris en compte par le simulateur" in content
assert "<h2>kikoo</h2>" in content
assert Event.objects.get(
category="simulateur", event="soumission_autre", metadata__user_type="anonymous"
)

params = "department=44&element=bosquet&travaux=entretien"
full_url = f"{url}?{params}"
Expand Down Expand Up @@ -158,7 +164,7 @@ def test_debug_result(client):
ENVERGO_HAIE_DOMAIN="testserver", ENVERGO_AMENAGEMENT_DOMAIN="otherserver"
)
@patch("envergo.hedges.services.get_replantation_coefficient")
def test_result_p_view_with_R_gt_0(mock_R, client):
def test_result_d_view_with_R_gt_0(mock_R, client):
DCConfigHaieFactory()
hedges = HedgeDataFactory()
data = {
Expand All @@ -179,14 +185,17 @@ def test_result_p_view_with_R_gt_0(mock_R, client):
res = client.get(f"{url}?{query}")

assert "Déposer une demande sans plantation" not in res.content.decode()
assert Event.objects.get(
category="simulateur", event="soumission_d", metadata__user_type="anonymous"
)


@pytest.mark.urls("config.urls_haie")
@override_settings(
ENVERGO_HAIE_DOMAIN="testserver", ENVERGO_AMENAGEMENT_DOMAIN="otherserver"
)
@patch("envergo.hedges.services.get_replantation_coefficient")
def test_result_p_view_with_R_eq_0(mock_R, client):
def test_result_d_view_with_R_eq_0(mock_R, client):
DCConfigHaieFactory()
hedges = HedgeDataFactory()
data = {
Expand Down Expand Up @@ -214,7 +223,7 @@ def test_result_p_view_with_R_eq_0(mock_R, client):
@override_settings(
ENVERGO_HAIE_DOMAIN="testserver", ENVERGO_AMENAGEMENT_DOMAIN="otherserver"
)
def test_result_p_view_non_soumis_with_r_gt_0(client):
def test_result_d_view_non_soumis_with_r_gt_0(client):
DCConfigHaieFactory()
hedge_lt5m = HedgeFactory(
latLngs=[
Expand Down Expand Up @@ -242,6 +251,36 @@ def test_result_p_view_non_soumis_with_r_gt_0(client):
assert "Déposer une demande sans plantation" not in res.content.decode()


@pytest.mark.urls("config.urls_haie")
@override_settings(
ENVERGO_HAIE_DOMAIN="testserver", ENVERGO_AMENAGEMENT_DOMAIN="otherserver"
)
@patch("envergo.hedges.services.get_replantation_coefficient")
def test_result_p_view(mock_R, client):
DCConfigHaieFactory()
hedges = HedgeDataFactory()
data = {
"element": "haie",
"travaux": "destruction",
"motif": "amelioration_culture",
"reimplantation": "remplacement",
"localisation_pac": "oui",
"department": "44",
"haies": hedges.id,
"lineaire_total": 100,
"transfert_parcelles": "non",
"meilleur_emplacement": "non",
}
url = reverse("moulinette_result_plantation")
query = urlencode(data)
mock_R.return_value = 0.0
client.get(f"{url}?{query}")

assert Event.objects.get(
category="simulateur", event="soumission_p", metadata__user_type="anonymous"
)


@pytest.mark.urls("config.urls_haie")
@override_settings(
ENVERGO_HAIE_DOMAIN="testserver", ENVERGO_AMENAGEMENT_DOMAIN="otherserver"
Expand All @@ -260,7 +299,9 @@ def test_moulinette_post_form_error(client):
assert res.status_code == 200
assert HOME_TITLE in res.content.decode()
assert FORM_ERROR in res.content.decode()
error_event = Event.objects.filter(category="erreur", event="formulaire-simu").get()
error_event = Event.objects.get(
category="erreur", event="formulaire-simu", metadata__user_type="anonymous"
)
assert "errors" in error_event.metadata
assert error_event.metadata["errors"] == {
"haies": [
Expand Down
45 changes: 28 additions & 17 deletions envergo/moulinette/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
from envergo.moulinette.forms import TriageFormHaie
from envergo.moulinette.models import ConfigHaie, get_moulinette_class_from_site
from envergo.users.mixins import InstructorDepartmentAuthorised
from envergo.users.models import User
from envergo.utils.urls import copy_qs, remove_from_qs, remove_mtm_params, update_qs


Expand Down Expand Up @@ -251,6 +252,10 @@ def get_result_url(self):
return url_with_params

def log_moulinette_event(self, moulinette, context, **kwargs):
if not moulinette.is_triage_valid():
Copy link
Collaborator

Choose a reason for hiding this comment

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

Je ne comprends pas pourquoi il faut logguer un événement différent en fonction de l'état de la validation de la Moulinette. Un petit commentaire ne serait pas de refus ici.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

J'ai modifié un peu la structure, et ajouté un commentaire. C'est sans doute mieux.

super().log_moulinette_event(moulinette, context)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Est ce que ce comportement est acceptable dans un Mixin ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah ça me paraît aussi bizarre d'appeler une méthode d'une classe parente sur une mixin. Pourquoi ne pas l'avoir laissé dans la classe enfant ?

Il y a un test sur triage ici, qui vérifie avant que moulinette est sur triage et pas form :
https://github.com/MTES-MCT/envergo/pull/933/changes#diff-2eda7161f91966c43de31eb00616f3b0f93d417249644d5000ff61e14d41e2afR178

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

La méthode de la classe enfant n'est pas appelée, elle est cachée par le mixin.
J'ai finalement mis l'ensemble des cas dans le Mixin, c'est de loin l'option la plus cohérente à mes yeux/

return

export = moulinette.summary()
export.update(kwargs)
export["url"] = self.request.build_absolute_uri()
Expand All @@ -268,6 +273,7 @@ def log_moulinette_event(self, moulinette, context, **kwargs):
action,
self.request,
**export,
user_type=User.get_type(self.request.user),
)


Expand Down Expand Up @@ -323,6 +329,7 @@ def form_invalid(self, form):
self.request,
data=form.data,
errors=form_errors,
user_type=User.get_type(self.request.user),
)
return self.render_to_response(context)

Expand Down Expand Up @@ -518,20 +525,18 @@ def get(self, request, *args, **kwargs):
return res

def log_moulinette_event(self, moulinette, context):
if moulinette.is_triage_valid():
super().log_moulinette_event(moulinette, context)
else:
# TODO Why is matomo param cleanup only happens here?
# Matomo parameters are stored in session, but some might remain in the url.
# We need to prevent duplicate values
params = get_matomo_tags(self.request)
params.update(self.request.GET.dict())
log_event(
"simulateur",
"soumission_autre",
self.request,
**params,
)
# TODO Why is matomo param cleanup only happens here?
# Matomo parameters are stored in session, but some might remain in the url.
# We need to prevent duplicate values
params = get_matomo_tags(self.request)
params.update(self.request.GET.dict())
log_event(
"simulateur",
"soumission_autre",
self.request,
**params,
user_type=User.get_type(self.request.user),
)


class MoulinetteAmenagementResult(
Expand Down Expand Up @@ -648,13 +653,19 @@ def get(self, request, *args, **kwargs):
if not self.moulinette.department:
return HttpResponseRedirect(reverse("home"))

event_params = {
"department": self.moulinette.department.department,
"user_type": User.get_type(request.user),
}
is_alternative = bool(request.GET.get("alternative", False))
if is_alternative:
event_params["alternative"] = "true"

log_event(
"simulateur",
"localisation",
self.request,
**{
"department": self.moulinette.department.department,
},
**event_params,
**get_matomo_tags(self.request),
)
return self.render_to_response(self.get_context_data())
Expand Down
6 changes: 3 additions & 3 deletions envergo/pages/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
from envergo.moulinette.models import ConfigAmenagement
from envergo.moulinette.views import MoulinetteMixin
from envergo.pages.models import NewsItem
from envergo.users.models import User

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -79,9 +80,8 @@ def post(self, request, *args, **kwargs):
"simulateur",
"localisation",
self.request,
**{
"department": department.department,
},
department=department.department,
user_type=User.get_type(request.user),
)
return self.render_to_response(context)

Expand Down
9 changes: 9 additions & 0 deletions envergo/petitions/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,9 @@ def test_petition_project_detail(mock_post, client, site):
response = client.get(petition_project_url)
assert response.status_code == 200
assert "moulinette" in response.context
assert Event.objects.get(
category="simulateur", event="consultation", metadata__user_type="anonymous"
)
# default PetitionProjectFactory has hedges near Aniane but is declared in department 44
assert response.context["has_hedges_outside_department"]
assert "Le projet est hors du département sélectionné" in response.content.decode()
Expand Down Expand Up @@ -977,6 +980,9 @@ def test_petition_project_alternative(client, haie_user, haie_instructor_44, sit
in content
)
assert "Partager cette page par email" not in content
assert Event.objects.get(
category="simulateur", event="soumission_d", metadata__alternative="true"
)

# WHEN the user visit the result plantation page of an alternative
result_url = alternative_url.replace("/formulaire", "/resultat-plantation")
Expand All @@ -992,6 +998,9 @@ def test_petition_project_alternative(client, haie_user, haie_instructor_44, sit
assert "Partager cette page par email" not in content
assert "La demande d'autorisation est prête à être complétée" not in content
assert "Copier le lien de cette page" in content
assert Event.objects.get(
category="simulateur", event="soumission_p", metadata__alternative="true"
)


def test_instructor_view_with_hedges_outside_department(client, haie_instructor_44):
Expand Down
4 changes: 4 additions & 0 deletions envergo/petitions/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
send_message_dossier_ds,
update_demarches_simplifiees_status,
)
from envergo.users.models import User
from envergo.utils.mattermost import notify
from envergo.utils.tools import generate_key
from envergo.utils.urls import extract_param_from_url, remove_mtm_params, update_qs
Expand Down Expand Up @@ -199,6 +200,7 @@ def form_valid(self, form):
"creation",
self.request,
**petition_project.get_log_event_data(),
user_type=User.get_type(self.request.user),
**get_matomo_tags(self.request),
)

Expand Down Expand Up @@ -529,6 +531,7 @@ def get(self, request, *args, **kwargs):
"consultation",
self.request,
**self.object.get_log_event_data(),
user_type=User.get_type(self.request.user),
**get_matomo_tags(self.request),
)

Expand Down Expand Up @@ -780,6 +783,7 @@ def log_event_action(self, request):
self.event_action,
self.request,
**self.get_log_event_data(),
user_type=User.get_type(request.user),
**get_matomo_tags(self.request),
)

Expand Down
11 changes: 11 additions & 0 deletions envergo/users/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,3 +74,14 @@ class Meta:

def __str__(self):
return f"{self.name}"

@classmethod
def get_type(cls, user):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ça me semble peu clair et mal défini ce que signifie un « user type ». Par exemple le terme « administrator » veut-il dire avoir accès au backend (si on se réfère à la version technique du terme) ou avoir accès aux dossiers (version métier) ?

Par ailleurs, ça n'a pas vraiment de sens si on est côté Aménagement.

Cette fonction n'est utilisée que dans un cadre d'analytics, et n'est pas vraiment lié à la gestion fonctionnelle des utilisateurs. Pour cette raison, je l'aurais bien sortie de la classe User pour la déporter quelque part vers analytics.utils.get_user_type par exemple.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Effectivement, ces labels n'ont pas beaucoup de sens nu d'un point de vue technique, ni d'un point de vue métier. Mais, à ce jour, on n'a pas vraiment d'équivalent métier ou technique qui m'aurait permis de répondre au besoin de tracking.
J'ai déplacé le tout (tel quel) dans l'app analytics pour que l'on ne soit pas tenter de réutiliser ce type en lui donnant plus de valeur que ce qu'il a vraiement.

if not user or not user.is_authenticated:
return "anonymous"
if user.is_superuser or user.is_staff:
return "administrator"
elif user.is_instructor:
return "instructor"
else:
return "guest"
Loading