(PC-31827)[BO] feat: add special event page #14671
Conversation
| NEW = "new" | ||
| VALIDATED = "validated" | ||
| REJECTED = "rejected" | ||
| PRESELECTED = "preselected" |
There was a problem hiding this comment.
Détail : Si on veut que ce soit plus homogène dans le format avec les ValidationStatus ou encore BookingStatus :
| NEW = "new" | |
| VALIDATED = "validated" | |
| REJECTED = "rejected" | |
| PRESELECTED = "preselected" | |
| NEW = "NEW" | |
| VALIDATED = "VALIDATED" | |
| REJECTED = "REJECTED" | |
| PRESELECTED = "PRESELECTED" |
| @@ -58,6 +67,7 @@ class SpecialEventResponse(PcObject, Base, Model): | |||
| dateSubmitted: datetime = sa.Column(sa.DateTime, nullable=False) | |||
| phoneNumber: str = sa.Column(sa.Text(), nullable=True) | |||
| email: str = sa.Column(sa.Text(), nullable=True) | |||
| status: SpecialEventResponseStatus = sa.Column(MagicEnum(SpecialEventResponseStatus), nullable=False) | |||
There was a problem hiding this comment.
Suggestion : pourquoi pas une valeur par défaut à l'insertion (NEW) ?
| case operations_models.SpecialEventResponseStatus.REJECTED: | ||
| return Markup('<span class="badge text-bg-danger">Rejetée</span>') | ||
| case operations_models.SpecialEventResponseStatus.PRESELECTED: | ||
| return Markup('<span class="badge text-bg-warning">Préselectionné</span>') |
There was a problem hiding this comment.
Attention, la couleur warning peut être interprétée comme à moitié négative, alors qu'ici c'est une issue en bonne voie. Un vert clair ? (Mais je ne suis pas designer hein :))
Par contre toutes les étiquettes sont au féminin, donc ce serait «Pré-sélectionnée ».
| special_event_query = operations_models.SpecialEvent.query.filter( | ||
| operations_models.SpecialEvent.id == special_event_id | ||
| ).options( | ||
| sa.orm.joinedload(operations_models.SpecialEvent.questions).load_only( |
There was a problem hiding this comment.
Attention, tu fais une jointure 1..n sur les questions, une jointure 1..n sur les candidatures puis sur les réponses aux questions. Ça peut devenir exponentiel en cas de grand nombre de participations.
Imaginons 5 questions et 1000 candidatures, on aura ici... 5 x 1000 x 5 = 25000 lignes SQL rapatriées.
| ) | ||
|
|
||
|
|
||
| @operations_blueprint.route("/<int:special_event_id>/validate/<int:response_id>", methods=["POST"]) |
There was a problem hiding this comment.
On peut demander plusieurs avis, mais j'aurais mis d'abord l'id de la candidature, puis l'action, par logique dans le chemin. Idem en-dessous.
| @operations_blueprint.route("/<int:special_event_id>/validate/<int:response_id>", methods=["POST"]) | |
| @operations_blueprint.route("/<int:special_event_id>/responses/<int:response_id>/validate", methods=["POST"]) |
| method="post"> | ||
| {{ csrf_token }} | ||
| <button type="submit" | ||
| class="btn btn-sm d-block w-100 text-start px-3">Valider la candidature</button> |
There was a problem hiding this comment.
Débat : Je suis partagé sur le terme « Valider la candidature », car dans mon esprit, cela signifie qu'on valide le fait de postuler. Or là il s'agirait plutôt de « sélectionner », voire de « retenir », non ?
| {% if response.user %} | ||
| <td>{{ links.build_public_user_name_to_details_link(response.user) }}</td> | ||
| <td>{{ response.user.email }}</td> | ||
| <td>{{ response.user.phoneNumber | empty_string_if_null }}</td> |
There was a problem hiding this comment.
Attention, ici tu ignores le numéro de téléphone saisi dans le formulaire, alors que selon moi c'est ce dernier qui devrait être privilégié pour contacter le jeune, au cas où ce ne soit pas le même qu'à l'inscription.
| {% for answer in response.answers %} | ||
| {% if answer.questionId == question.id %} | ||
| {{ answer.text }} | ||
| <br> | ||
| {% endif %} | ||
| {% endfor %} |
There was a problem hiding this comment.
La boucle dans la boucle dans la boucle me gène, et peut-être pour éviter la multiplication des lignes SQL, une agrégation JSON des réponses aux questions, pour chaque candidature, permettrait d'éviter de multiplier les lignes SQL. Ainsi tu accéderais ici aux réponses par clé de dictionnaire.
J'imagine donc deux requêtes dans l'endpoint qui charge cette page :
- l'opération spéciale et les questions en jointure (total 5 lignes SQL dans l'exemple donné plus haut) ou en agrégation (1 ligne)
- les candidatures avec une agrégation des réponses (total 1000 lignes SQL dans le même exemple)
| <li class="dropdown-item p-0"> | ||
| <a href="{{ url_for('backoffice_web.fraud.prepare_blacklist_domain_name', domain=row.domain) }}" | ||
| class="text-decoration-none"> | ||
| <button type="submit" | ||
| class="btn btn-sm d-block w-100 text-start px-3">Répéter l'opération</button> | ||
| </a> | ||
| </li> |
There was a problem hiding this comment.
À quoi sert ce code vers prepare_blacklist_domain_name ??
| operations_factories.SpecialEventAnswerFactory( | ||
| responseId=long_response.id, | ||
| questionId=why_question.id, | ||
| text="Vous savez, moi je ne crois pas qu'il y ait de bonne ou de mauvaise situation. " |
There was a problem hiding this comment.
<rabat-joie>As-tu vérifié qu'il soit autorisé d'inclure cette citation dans du code open-source ? Au minimum je pense que tu dois créditer l'auteur, étant donné que ce n'est pas encore dans le domaine public.</rabat-joie>
But de la pull request
Ticket Jira (ou description si BSR) : https://passculture.atlassian.net/browse/PC-31827
Vérifications