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

Personnalisation du GUH par département #408

Merged
merged 7 commits into from
Sep 23, 2024
Merged

Personnalisation du GUH par département #408

merged 7 commits into from
Sep 23, 2024

Conversation

thibault
Copy link
Collaborator

@thibault thibault requested a review from pyDez September 10, 2024 08:58
@thibault thibault marked this pull request as ready for review September 10, 2024 08:58
@thibault
Copy link
Collaborator Author

@pyDez Je me suis pris la tête à essayer de rajouter des paramètres dans l'url du simulateur, et j'ai recontré pas mal de difficultés techniques.

Finalement j'ai rétropédalé et je suis resté sur une approche extrêmement lean jusqu'à la caricature, mais le découpe en carte est fait de telle façon que j'ai finalement l'impression que sur ce ticket spécifique, on n'a pas besoin de plus.

@@ -5,5 +5,8 @@

urlpatterns = [
path("", include("envergo.pages.urls_haie")),
path(_("moulinette/"), include("envergo.moulinette.urls")),
path(
"indre/",
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 hésité à rendre le département paramétrable, mais à ce stade il n'y a rien dans la carte qui décrirait un comportement différent en fonction du département, donc je suis allé au plus direct.

Copy link
Collaborator

@pyDez pyDez left a comment

Choose a reason for hiding this comment

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

Pour être lean, c'est lean ! J'espere que cela nous emmene quand meme un peu dans la bonne direction :)

@@ -1,7 +1,7 @@
section#simulateur,
div#moulinette {
h2 {
color: var(--text-action-high-blue-france);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Le titre en bleu avait été une demande explicite à un moment donné. Je suis étonné de voir que ce n'est plus le cas.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oui j'avoue…

for url in HAIE_URLS + COMMON_URLS:
response = client.get(url)
assert response.status_code < 400, f"Failed for URL: {url}"
def test_haie_can_access_haie_pages(client, url):
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@pyDez

Suite à la modif de cette PR, le résultat de reverse("moulinette_home") et reverse("moulinette_result") n'est plus le même entre haie et aménagement.

Par conséquent, ces tests ne passaient plus. J'ai été obligé de déporter l'appel à reverse à l'intérieur du test.

Par ailleurs, malgré l'override de settings, reverse ne semble pas prendre en compte l'urlconf de haie. J'ai été obligé d'utiliser @pytest.mark.urls("config.urls_haie"). Tu as une idée pourquoi ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Waouh ! L'utilisation de parametrize est tellement plus élégante 🤩

Par ailleurs, malgré l'override de settings, reverse ne semble pas prendre en compte l'urlconf de haie. J'ai été obligé d'utiliser @pytest.mark.urls("config.urls_haie"). Tu as une idée pourquoi ?

L'override de ENVERGO_HAIE_DOMAIN permet de définir l'"urlconf" de la requete grâce à un middleware. Cet urlConf à la préséance sur celui qui vient des settings dans le cadre de la requete. Mais comme on appel reverse en dehors d'une requete, il se retranche sur l'urlConf par défaut qui provient des settings.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, merci pour l'explication. Donc effectivement surcharger le domain n'est pas suffisant pour faire fonctionner reverse ici.

@thibault
Copy link
Collaborator Author

@pyDez J'ai du modifier le fonctionnement des tests pour les faire passer. Je te laisse jeter un dernier coup d'œil si tu veux.

Copy link
Collaborator

@pyDez pyDez left a comment

Choose a reason for hiding this comment

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

👍

@pyDez pyDez merged commit 07a2900 into main Sep 23, 2024
4 of 5 checks passed
@pyDez pyDez deleted the haie_department branch September 23, 2024 07:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants