Skip to content

Commit

Permalink
Rendre obligatoire les champs required du message
Browse files Browse the repository at this point in the history
Ce commit corrige une combinaison de deux problèmes qui fait qu'un
message pouvait être validé sans aucun champs de remplis (causant une ou
des erreurs côté backend).

- Au niveau de JS le form n'était pas validé avant l'envoi
- Au niveau de ChoicesJS, le champ initial est bien required mais masqué
  complétement par la bibliothèque (display: none !important) ce qu'il
fait qu'il ne rentre pas en compte dans la validation du formulaire.

Pour corriger le deuxième problème j'ai opté pour une méthode de
masquage différente qui garde le champ actif mais le cache de
l'affichage, il est ainsi consévé dans la validation. Ce changement de
comportement oblige a beaucoup de changement dans les tests pour être
sur de sélectionner la bonne option.

Choices-js/Choices#1061
  • Loading branch information
Anto59290 committed Mar 4, 2025
1 parent 4b8a106 commit 5d30a2c
Show file tree
Hide file tree
Showing 4 changed files with 113 additions and 28 deletions.
8 changes: 6 additions & 2 deletions conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,15 @@ def mocked(self, request):

@pytest.fixture
def choice_js_fill(db, page):
def _choice_js_fill(page, locator, fill_content, exact_name):
def _choice_js_fill(page, locator, fill_content, exact_name, use_locator_as_parent_element=False):
page.query_selector(locator).click()
page.wait_for_selector("input:focus", state="visible", timeout=2_000)
page.locator("*:focus").fill(fill_content)
page.get_by_role("option", name=exact_name, exact=True).click()
if use_locator_as_parent_element:
list_element = page.locator(locator).locator(".choices__list")
list_element.get_by_role("option", name=exact_name, exact=True).click()
else:
page.get_by_role("option", name=exact_name, exact=True).click()

return _choice_js_fill

Expand Down
11 changes: 11 additions & 0 deletions core/static/core/message.css
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,14 @@ label[for=id_recipients], label[for=id_recipients_copy], label[for=id_recipients
display: flex;
justify-content: space-between;
}

#message-form .choices [hidden]{
display: block !important;
position: absolute;
width: 100%;
height: 100%;
opacity: 0;
pointer-events: none;
top: 0;
left: 0;
}
8 changes: 8 additions & 0 deletions core/static/core/message_form.js
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,14 @@ document.addEventListener('DOMContentLoaded', function () {
document.getElementById("message-send-btn").addEventListener("click", event =>{
event.preventDefault()
event.target.disabled = true
const messageForm = document.getElementById("message-form")
messageForm.reportValidity()

if (!messageForm.checkValidity()) {
event.target.disabled = false
return
}

const isDocumentBlockVisible = !document.querySelector(".document-form").classList.contains("fr-hidden")
const hasFile = !!document.getElementById('id_file').files[0]

Expand Down
114 changes: 88 additions & 26 deletions sv/tests/test_evenement_message.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,10 @@ def test_can_add_and_see_message_without_document(live_server, page: Page, choic

choice_js_fill(
page,
"#id_recipients ~ input",
'label[for="id_recipients"] ~ div.choices',
active_contact.nom,
active_contact.contact_set.get().display_with_agent_unit,
use_locator_as_parent_element=True,
)
expect(page.locator("#message-type-title")).to_have_text("message")
page.locator("#id_title").fill("Title of the message")
Expand Down Expand Up @@ -66,16 +67,18 @@ def test_can_add_and_see_demande_intervention(live_server, page: Page, choice_js

choice_js_fill(
page,
"#id_recipients_structures_only ~ input",
'label[for="id_recipients_structures_only"] ~ div.choices',
active_contact.display_with_agent_unit,
active_contact.display_with_agent_unit,
use_locator_as_parent_element=True,
)
page.keyboard.press("Escape")
choice_js_fill(
page,
"#id_recipients_copy_structures_only ~ input",
'label[for="id_recipients_copy_structures_only"] ~ div.choices',
other_active_contact.display_with_agent_unit,
other_active_contact.display_with_agent_unit,
use_locator_as_parent_element=True,
)
expect(page.locator("#message-type-title")).to_have_text("demande d'intervention")
page.locator("#id_title").fill("Title of the message")
Expand Down Expand Up @@ -120,9 +123,10 @@ def test_can_add_and_see_message_multiple_documents(live_server, page: Page, cho

choice_js_fill(
page,
"#id_recipients ~ input",
'label[for="id_recipients"] ~ div.choices',
active_contact.nom,
active_contact.contact_set.get().display_with_agent_unit,
use_locator_as_parent_element=True,
)
page.locator("#id_title").fill("Title of the message")
page.locator("#id_content").fill("My content \n with a line return")
Expand Down Expand Up @@ -182,30 +186,36 @@ def test_can_add_and_see_message_with_multiple_recipients_and_copies(live_server
# Add multiple recipients
choice_js_fill(
page,
"#id_recipients ~ input",
'label[for="id_recipients"] ~ div.choices',
agents[0].nom,
agents[0].contact_set.get().display_with_agent_unit,
use_locator_as_parent_element=True,
)
page.keyboard.press("Escape")
choice_js_fill(
page,
"#id_recipients ~ input",
'label[for="id_recipients"] ~ div.choices',
agents[1].nom,
agents[1].contact_set.get().display_with_agent_unit,
use_locator_as_parent_element=True,
)
page.keyboard.press("Escape")

# Add multiples recipients as copy
choice_js_fill(
page,
"#id_recipients_copy ~ input",
'label[for="id_recipients_copy"] ~ div.choices',
agents[2].nom,
agents[2].contact_set.get().display_with_agent_unit,
use_locator_as_parent_element=True,
)
page.keyboard.press("Escape")
choice_js_fill(
page,
"#id_recipients_copy ~ input",
'label[for="id_recipients_copy"] ~ div.choices',
agents[3].nom,
agents[3].contact_set.get().display_with_agent_unit,
use_locator_as_parent_element=True,
)

page.locator("#id_title").fill("Title of the message")
Expand Down Expand Up @@ -393,7 +403,7 @@ def test_cant_pick_inactive_user_in_message(live_server, page: Page, choice_js_c
page.get_by_test_id("element-actions").click()
page.get_by_role("link", name="Message").click()

choice_js_cant_pick(page, "#id_recipients ~ input", agent.nom, str(agent))
choice_js_cant_pick(page, 'label[for="id_recipients"] ~ div.choices', agent.nom, str(agent))


def test_cant_only_pick_structure_with_email(live_server, page: Page, choice_js_fill, choice_js_cant_pick):
Expand All @@ -406,8 +416,8 @@ def test_cant_only_pick_structure_with_email(live_server, page: Page, choice_js_
page.get_by_test_id("element-actions").click()
page.get_by_role("link", name="Message").click()

choice_js_fill(page, "#id_recipients ~ input", "FOO", "FOO")
choice_js_cant_pick(page, "#id_recipients ~ input", "BAR", "BAR")
choice_js_fill(page, 'label[for="id_recipients"] ~ div.choices', "FOO", "FOO", use_locator_as_parent_element=True)
choice_js_cant_pick(page, 'label[for="id_recipients"] ~ div.choices', "BAR", "BAR")


@pytest.mark.parametrize("message_type, message_label", Message.MESSAGE_TYPE_CHOICES)
Expand Down Expand Up @@ -444,20 +454,28 @@ def test_can_see_more_than_4_search_result_in_recipients_and_recipients_copy_fie
page.get_by_role("link", name="Message").click()

# Test le champ Destinataires
page.locator("#id_recipients ~ input").click()
page.locator('label[for="id_recipients"] ~ div.choices').click()
page.wait_for_selector("input:focus", state="visible", timeout=2_000)
page.locator("*:focus").fill("Structure")
for i in range(nb_structure):
expect(page.get_by_role("option", name=f"Structure {i + 1}", exact=True)).to_be_visible()
expect(
page.locator('label[for="id_recipients"] ~ div.choices')
.locator(".choices__list")
.get_by_role("option", name=f"Structure {i + 1}", exact=True)
).to_be_visible()

page.locator(".fr-select").first.press("Escape")

# Test le champ Copie
page.locator("#id_recipients_copy ~ input").click()
page.locator('label[for="id_recipients_copy"] ~ div.choices').click()
page.wait_for_selector("input:focus", state="visible", timeout=2_000)
page.locator("*:focus").fill("Structure")
for i in range(nb_structure):
expect(page.get_by_role("option", name=f"Structure {i + 1}", exact=True)).to_be_visible()
expect(
page.locator('label[for="id_recipients_copy"] ~ div.choices')
.locator(".choices__list")
.get_by_role("option", name=f"Structure {i + 1}", exact=True)
).to_be_visible()


def test_create_message_adds_agent_and_structure_contacts(
Expand Down Expand Up @@ -485,10 +503,22 @@ def test_create_message_adds_agent_and_structure_contacts(
page.get_by_role("link", name="Message").click()

# Ajout du destinataire
choice_js_fill(page, "#id_recipients ~ input", contact.agent.nom, contact.display_with_agent_unit)
choice_js_fill(
page,
'label[for="id_recipients"] ~ div.choices',
contact.agent.nom,
contact.display_with_agent_unit,
use_locator_as_parent_element=True,
)
page.keyboard.press("Escape")
# Ajout de la copie
choice_js_fill(page, "#id_recipients_copy ~ input", contact_copy.agent.nom, contact_copy.display_with_agent_unit)
choice_js_fill(
page,
'label[for="id_recipients_copy"] ~ div.choices',
contact_copy.agent.nom,
contact_copy.display_with_agent_unit,
use_locator_as_parent_element=True,
)
page.keyboard.press("Escape")
page.locator("#id_title").fill("Title of the message")
page.locator("#id_content").fill("Message de test")
Expand Down Expand Up @@ -540,7 +570,13 @@ def test_create_multiple_messages_adds_contacts_once(
page.get_by_test_id("element-actions").click()
page.get_by_role("link", name="Message").click()

choice_js_fill(page, "#id_recipients ~ input", contact.agent.nom, contact.display_with_agent_unit)
choice_js_fill(
page,
'label[for="id_recipients"] ~ div.choices',
contact.agent.nom,
contact.display_with_agent_unit,
use_locator_as_parent_element=True,
)
page.locator("#id_title").fill("Message 1")
page.locator("#id_content").fill("Message de test 1")
page.get_by_test_id("fildesuivi-add-submit").click()
Expand All @@ -549,7 +585,13 @@ def test_create_multiple_messages_adds_contacts_once(
page.get_by_test_id("element-actions").click()
page.locator(".message-actions").get_by_role("link", name="Message", exact=True).click()

choice_js_fill(page, "#id_recipients ~ input", contact.agent.nom, contact.display_with_agent_unit)
choice_js_fill(
page,
'label[for="id_recipients"] ~ div.choices',
contact.agent.nom,
contact.display_with_agent_unit,
use_locator_as_parent_element=True,
)
page.locator("#id_title").fill("Message 2")
page.locator("#id_content").fill("Message de test 2")
page.get_by_test_id("fildesuivi-add-submit").click()
Expand Down Expand Up @@ -593,7 +635,13 @@ def test_create_message_from_locale_changes_to_limitee_and_add_structures_in_all
page.get_by_role("link", name="Message").click()

# Envoi du message
choice_js_fill(page, "#id_recipients ~ input", contact.agent.nom, contact.display_with_agent_unit)
choice_js_fill(
page,
'label[for="id_recipients"] ~ div.choices',
contact.agent.nom,
contact.display_with_agent_unit,
use_locator_as_parent_element=True,
)
page.locator("#id_title").fill("Title of the message")
page.locator("#id_content").fill("Message de test")
page.get_by_test_id("fildesuivi-add-submit").click()
Expand All @@ -619,7 +667,13 @@ def test_create_message_from_locale_from_same_structure_does_not_changes_visibil
page.get_by_role("link", name="Message").click()

# Envoi du message
choice_js_fill(page, "#id_recipients ~ input", contact.agent.nom, contact.display_with_agent_unit)
choice_js_fill(
page,
'label[for="id_recipients"] ~ div.choices',
contact.agent.nom,
contact.display_with_agent_unit,
use_locator_as_parent_element=True,
)
page.locator("#id_title").fill("Title of the message")
page.locator("#id_content").fill("Message de test")
page.get_by_test_id("fildesuivi-add-submit").click()
Expand Down Expand Up @@ -649,7 +703,13 @@ def test_create_message_from_visibilite_limitee_add_structures_in_allowed_struct
page.get_by_role("link", name="Message").click()

# Envoi du message
choice_js_fill(page, "#id_recipients ~ input", contact.agent.nom, contact.display_with_agent_unit)
choice_js_fill(
page,
'label[for="id_recipients"] ~ div.choices',
contact.agent.nom,
contact.display_with_agent_unit,
use_locator_as_parent_element=True,
)
page.locator("#id_title").fill("Title of the message")
page.locator("#id_content").fill("Message de test")
page.get_by_test_id("fildesuivi-add-submit").click()
Expand Down Expand Up @@ -730,9 +790,10 @@ def test_message_with_document_exceeding_max_size_shows_validation_error(live_se
page.get_by_role("link", name="Message").click()
choice_js_fill(
page,
"#id_recipients ~ input",
'label[for="id_recipients"] ~ div.choices',
active_contact.nom,
active_contact.contact_set.get().display_with_agent_unit,
use_locator_as_parent_element=True,
)
page.locator("#id_title").fill("Message avec fichier trop volumineux")
page.locator("#id_content").fill("Test de validation de taille de fichier")
Expand Down Expand Up @@ -766,9 +827,10 @@ def test_can_add_message_with_document_confirmation_modal_reject(live_server, pa

choice_js_fill(
page,
"#id_recipients ~ input",
'label[for="id_recipients"] ~ div.choices',
active_contact.nom,
active_contact.contact_set.get().display_with_agent_unit,
use_locator_as_parent_element=True,
)
page.locator("#id_title").fill("Title of the message")
page.locator("#id_content").fill("My content \n with a line return")
Expand All @@ -780,7 +842,6 @@ def test_can_add_message_with_document_confirmation_modal_reject(live_server, pa
page.get_by_test_id("fildesuivi-add-submit").click()
expect(page.locator("#fr-modal-document-confirmation")).to_be_visible()
page.locator("#send-without-adding-document").click()

page.wait_for_url(f"**{evenement.get_absolute_url()}#tabpanel-messages-panel")
message = Message.objects.get()
assert message.documents.count() == 0
Expand All @@ -795,9 +856,10 @@ def test_can_add_message_with_document_confirmation_modal_confirm(live_server, p

choice_js_fill(
page,
"#id_recipients ~ input",
'label[for="id_recipients"] ~ div.choices',
active_contact.nom,
active_contact.contact_set.get().display_with_agent_unit,
use_locator_as_parent_element=True,
)
page.locator("#id_title").fill("Title of the message")
page.locator("#id_content").fill("My content \n with a line return")
Expand Down

0 comments on commit 5d30a2c

Please sign in to comment.