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

Pilotage : Mise en place d'une route pour accéder aux tableaux de bord par leur nom #4466

Merged
merged 2 commits into from
Jul 31, 2024
Merged
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
1 change: 1 addition & 0 deletions itou/www/stats/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
# Public stats.
path("", views.stats_public, name="stats_public"),
path("pilotage/<int:dashboard_id>", views.stats_pilotage, name="stats_pilotage"),
path("redirect/<str:dashboard_name>", views.stats_redirect, name="redirect"),
# Employer stats.
path("siae/aci", views.stats_siae_aci, name="stats_siae_aci"),
path("siae/etp", views.stats_siae_etp, name="stats_siae_etp"),
Expand Down
13 changes: 13 additions & 0 deletions itou/www/stats/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from django.conf import settings
from django.contrib.auth.decorators import login_required
from django.core.exceptions import PermissionDenied
from django.http import HttpResponseNotFound, HttpResponseRedirect
from django.shortcuts import render
from django.urls import reverse
from django.views.decorators.clickjacking import xframe_options_exempt
Expand All @@ -31,6 +32,7 @@
)
from itou.companies import models as companies_models
from itou.prescribers.enums import PrescriberOrganizationKind
from itou.users.enums import UserKind
from itou.utils import constants as global_constants
from itou.utils.apis import metabase as mb
from itou.utils.perms.company import get_current_company_or_404
Expand Down Expand Up @@ -185,6 +187,17 @@ def stats_pilotage(request, dashboard_id):
return render_stats(request=request, context=context, template_name="stats/stats_pilotage.html")


@login_required
def stats_redirect(request, dashboard_name):
match request.user.kind:
case UserKind.LABOR_INSPECTOR:
normalized_organization_kind = request.current_organization.kind.lower().replace(" ", "_")
Copy link
Contributor

Choose a reason for hiding this comment

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

Ça donne envie de virer l’espace de la valeur pour kind, mais j’ai un peu peur qu’on l’utilise directement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

J'ai pas regardé si c'était la merde (faut faire des migrations) ou LA MERDE (faut toucher à des requêtes SQL jusque dans le pilotage) mais oui l'espace m'a aussi un peu étonné.

case _:
return HttpResponseNotFound()
Copy link
Contributor

Choose a reason for hiding this comment

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

Je pense au cas d'un membre DDETS_IAE/DDETS_GEIQ connecté en GEIQ qui se prendrait une 404:
je me demande si on ne peut pas améliorer le message de la 404 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Je pense au cas d'un membre DDETS_IAE/DDETS_GEIQ connecté en GEIQ

Pas sûr de comprendre 🤔.
Un compte LABOR_INSPECTOR ne peux normalement pas se lier à un CompanyMembership, non ?

Et pour le coup j'ai pris le parti de faire la fonction très simple quitte à avoir quelques erreurs au début, plutôt que commencer à mettre des vérifications dans tout les sens.

Copy link
Contributor

Choose a reason for hiding this comment

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

Je parle de https://github.com/gip-inclusion/les-emplois/blob/master/itou/institutions/enums.py#L7-L10
Si sa current_organization est DDETS_GEIQ et qu'il va sur un tableau de bord IAE, il aura une 404 sans forcément faire le lien avec son institution ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, compris ! 😫 🥵

Dans ce cas là il aura pas la 404 mais la 500 [1] car son type d'utilisateur est bon mais la route stats_ddets_geiq_{dashboard_name} n'existera sans doute pas.

C'est toujours la même problématique de prendre current_organization ou organizations pour les ACL, et la perméabilité entre les deux, mais de toute manière vu comment son fait les ACL pour les TB ça serais pas vraiment possible de lui dire "Change d'organisation pour voir ce TB".

[1] Je suis parti sur cette idée car une 500 fera une erreur sentry et donc on verra les erreurs réelles plutôt qu'ajouter du code défensif dés le départ sur un truc qui petit et non-critique

Copy link
Contributor

Choose a reason for hiding this comment

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

Oui pas de super solution à proposer 😬


return HttpResponseRedirect(reverse(f"stats:stats_{normalized_organization_kind}_{dashboard_name}"))


@login_required
def stats_siae_aci(request):
"""
Expand Down
13 changes: 13 additions & 0 deletions tests/www/stats/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -430,3 +430,16 @@ def test_get_params_aci_asp_ids_for_department_when_only_the_antenna_is_in_the_d
assert get_params_aci_asp_ids_for_department(company.department) == {
"id_asp_de_la_siae": [company.convention.asp_id]
}


@pytest.mark.parametrize("dashboard_name", ["ph_prescription", "state", "tension"])
@pytest.mark.parametrize(
"institution_kind", [InstitutionKind.DGEFP_IAE, InstitutionKind.DREETS_IAE, InstitutionKind.DDETS_IAE]
)
@override_settings(METABASE_SITE_URL="http://metabase.fake", METABASE_SECRET_KEY="foobar")
def test_stats_redirect_for_institution(client, institution_kind, dashboard_name):
institution = InstitutionWithMembershipFactory(kind=institution_kind)
client.force_login(institution.members.get())

response = client.get(reverse("stats:redirect", kwargs={"dashboard_name": dashboard_name}), follow=True)
assert response.status_code == 200