Skip to content
This repository was archived by the owner on Dec 22, 2022. It is now read-only.

Nuffer update add membre form#116

Open
nuffer wants to merge 3 commits intomasterfrom
nuffer_update_add_membre_form
Open

Nuffer update add membre form#116
nuffer wants to merge 3 commits intomasterfrom
nuffer_update_add_membre_form

Conversation

@nuffer
Copy link
Collaborator

@nuffer nuffer commented Jan 19, 2017

Bon j'aimerais merger ca avant d'aller plus loin car sinon y aura un million de conflit avec des truc que j'ai deja modifier ailleurs.

En gros y a:

  • la mise a jours de l'ajout de membre
  • la mise en place de l'edition "en gros" des membre et famille. (page d'edition pour chacun)

@nuffer nuffer requested a review from mullerch January 19, 2017 16:18
$form->handleRequest($request);

if ($form->isValid()) {
if ($form->isSubmitted() && $form->isValid()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Inverser les cas


$form->handleRequest($request);

if($form->isValid())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Inverser les cas

->add('numero', TextType::class, array(
'required' => false,
'label' => 'Numéro',
'attr'=>array('placeholder'=>'Numéro')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ca mériterait presque un petit helper dans le Type parent de tout ça pour dire que le placeholder c'est le label, ça éviterai d'avoir tous les labels tapés deux fois

data: $('#membre_add_form form').serialize(),
success: function (response) {
$('#membre_add_form').html(response);
semantic_init();
Copy link
Collaborator

Choose a reason for hiding this comment

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

ça c'est quand même un truc bine bourrin, qui va pas marche toto ou tard, quand on aura une truc qui doit pas être init deux fois. Je propose d'ajouter un param à la fonction qui permet de définir le selecteur root pour jquery. Commme ça on passe semantic_init('#membre_add_form') et ça init que le contenu de la modal (avec adaptation de la fonction en question bien sur). Pour la retrocompatibilité, on peut garder l'appel sans param.

type: 'GET',
success: function (response) {
$('#membre_add_form').html(response);
semantic_init();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Idem que dessus


{% endmacro %}

{% import _self as macral %}
Copy link
Collaborator

Choose a reason for hiding this comment

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

macral c'est macro au singulier c'est bien ça ?

$prenom = $form->get('prenom')->getData();

//save nom and prenom in session for futurer use.
$session->set(AddMembreController::MEMBRE_NOM, $nom);
Copy link
Owner

Choose a reason for hiding this comment

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

Il faudrait créer un modèle simple qui contiendrait les différentes valeurs à transmettre entre état qui implémente \Serializable


//search famille with same name
$familleSearch = new FamilleSearch();
$familleSearch->nom = $nom;
Copy link
Owner

Choose a reason for hiding this comment

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

$familleSearch->setNom($nom); utiliser le moins de propriétés publiques possible

$prenom = $session->get(AddMembreController::MEMBRE_PRENOM);

//search homonyme
$membreSearch = new MembreSearch();
Copy link
Owner

Choose a reason for hiding this comment

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

Plutot que de créer plusieurs classes comme ca à la volée, ca vaudrait pas le coup de les réunir en un service ?


if(is_null($nom))
{
//restart process
Copy link
Owner

Choose a reason for hiding this comment

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

Il faudrait qu'on prenne l'habitude à chaque fois qu'on fait une action de créer un flash message ($this->addFlash('success', "bravo!"); depuis le controller) histoire que lorsqu'on implémentera un système qui affiche ces alertes, elles soient fonctionnelles et pertinentes directement


$form->handleRequest($request);

if($form->isValid())
Copy link
Owner

Choose a reason for hiding this comment

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

$form->isSubmitted() && $form->isValid()

Copy link
Collaborator

Choose a reason for hiding this comment

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

bien que ça n'ajoute rien à la fonctionnalité, c'est plus lisible



{% block message %}
Aller maintenant compléter le membre et sa famille:
Copy link
Owner

Choose a reason for hiding this comment

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

Allez

Copy link
Collaborator

Choose a reason for hiding this comment

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

Veuillez

<div class="item">
<i class="right arrow icon"></i>
<div class="content">
Aller voir le <a href="{{ path('app_membre_show',{'membre':membre.id}) }}">membre</a>
Copy link
Owner

Choose a reason for hiding this comment

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

Allez


{% block content %}

{{ form_start(form,{'attr': {'class': 'ui form'}}) }}
Copy link
Owner

Choose a reason for hiding this comment

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

Il faut définir le class: ui form directement dans le templating



{% block titre %}
{{ membre.prenom }} {{ membre.nom }}
Copy link
Owner

Choose a reason for hiding this comment

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

membre toString() ?

{% else %}
{% set aeowner = ', aucune idée en vrai.' %}
{% endif %}
<div class="content">Actuellement, l'adresse de facturation de ce membre est <b>{{ aeowner|raw }}</b></div>
Copy link
Owner

Choose a reason for hiding this comment

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

Le |raw est-il requis ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

En passant je me permet de dire que aeowner n'est pas très parlant

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants