-
Notifications
You must be signed in to change notification settings - Fork 7
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
Ajustement manuel du score au niveau de la sous-action CAE #3016
Conversation
@@ -55,7 +56,7 @@ export const SelectActionStatut = (props: TSelectActionStatutProps) => { | |||
value={currentValue} | |||
options={options} | |||
onSelect={onChange} | |||
buttonClassName="min-w-5rem w-fit p-0 !bg-transparent" | |||
buttonClassName={`min-w-5rem !w-fit p-0 !bg-transparent ${buttonClassName}`} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ça serait mieux avec l'utilitaire classNames
(évite un potentiel undefined
ajouté à la liste de classes)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
c'est d'ailleurs un undefined
qui fait échouer les storyshots du composant dans le workflow "Earthly test / all"
const isCustomScoreGranted = () => { | ||
const isGranted = tasks.reduce((result, currTask) => { | ||
if ( | ||
(!!tasksStatus[currTask.id] && | ||
tasksStatus[currTask.id] === 'non_renseigne') || | ||
!tasksStatus[currTask.id] | ||
) { | ||
if ( | ||
!!localStatus[currTask.id] && | ||
localStatus[currTask.id].avancement !== 'non_renseigne' | ||
) { | ||
return result; | ||
} else return false; | ||
} else { | ||
if ( | ||
!!localStatus[currTask.id] && | ||
localStatus[currTask.id].avancement === 'non_renseigne' | ||
) { | ||
return false; | ||
} else return result; | ||
} | ||
}, true); | ||
|
||
return isGranted; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Je ne suis pas sûr de saisir toute la logique, mais est-ce que ce ne serait pas plus clair avec un find
plutôt qu'un reduce
?
La fonction renvoi false
pour le 1er item trouvé qui ne rempli pas les conditions, et true
si pas d'item trouvé.
Et ça a aussi peut-être plus de sens à l'extérieur du composant pour bien séparer la partie de rendu de la partie logique de détermination des états.
point_fait: | ||
payload.avancement === 'fait' | ||
? task.point_potentiel | ||
: payload.avancement === 'detaille' && | ||
payload.avancementDetaille | ||
? task.point_potentiel * payload.avancementDetaille[0] | ||
: 0, | ||
point_programme: | ||
payload.avancement === 'programme' | ||
? task.point_potentiel | ||
: payload.avancement === 'detaille' && | ||
payload.avancementDetaille | ||
? task.point_potentiel * payload.avancementDetaille[1] | ||
: 0, | ||
point_pas_fait: | ||
payload.avancement === 'pas_fait' | ||
? task.point_potentiel | ||
: payload.avancement === 'detaille' && | ||
payload.avancementDetaille | ||
? task.point_potentiel * payload.avancementDetaille[2] | ||
: 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Idem. Ce bloc répété 3 fois est peut-être à factoriser dans une fonction externe (dans ./utils.ts
avec les autres traitements de l'avancement) ?
Ça te permettra aussi un return
intermédiaire qui évitera la double condition ternaire toujours un peu difficile à lire je trouve.
collectivite_id: payload.collectivite_id, | ||
action_id: payload.action_id, | ||
texte: payload.commentaire, | ||
modified_at: new Date().toLocaleDateString(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Normalement le modified_at
est rajouté automatiquement par le back, non ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
J'ai une erreur si je ne le mets pas dans la payload envoyée, modified_at
n'est pas optionnel
collectivite_id: payload.collectivite_id, | ||
action_id: payload.action_id, | ||
texte: payload.commentaire, | ||
modified_at: new Date().toLocaleDateString(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Idem pour le modified_at
.
Tu pourrais aussi éventuellement mutualiser la construction de la payload entre les deux appels.
| 'fait' | ||
| 'pas_fait' | ||
| 'programme' | ||
| 'non_renseigne' | ||
| 'detaille'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cette enum devrait être déplacée dans un fichier alias, car elle est maintenant disponible dans le typage exporté depuis la base Enums<'avancement'>
.
Il me semble qu'elle est aussi dupliquée au moins dans ActionStatusDropdown.tsx
| 'fait' | ||
| 'pas_fait' | ||
| 'programme' | ||
| 'non_renseigne' | ||
| 'detaille'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
à remplacer par Enums<'avancement'>
if ( | ||
task.action_id !== payload.actionId && | ||
!localTasksScores[task.action_id] | ||
) { | ||
scoreFait += task.point_fait; | ||
scoreProgramme += task.point_programme; | ||
scorePasFait += task.point_pas_fait; | ||
} else if ( | ||
task.action_id !== payload.actionId && | ||
localTasksScores[task.action_id] | ||
) { | ||
scoreFait += localTasksScores[task.action_id].point_fait; | ||
scoreProgramme += localTasksScores[task.action_id].point_programme; | ||
scorePasFait += localTasksScores[task.action_id].point_pas_fait; | ||
} else { | ||
switch (payload.avancement) { | ||
case 'fait': | ||
scoreFait += task.point_potentiel; | ||
break; | ||
case 'programme': | ||
scoreProgramme += task.point_potentiel; | ||
break; | ||
case 'pas_fait': | ||
scorePasFait += task.point_potentiel; | ||
break; | ||
case 'detaille': | ||
if (payload.avancementDetaille) { | ||
scoreFait += task.point_potentiel * payload.avancementDetaille[0]; | ||
scoreProgramme += | ||
task.point_potentiel * payload.avancementDetaille[1]; | ||
scorePasFait += | ||
task.point_potentiel * payload.avancementDetaille[2]; | ||
} | ||
break; | ||
default: | ||
break; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ce calcul devrait être sorti du composant pour être mis dans une fonction externe, avec un commentaire pour expliquer ce que ça fait.
7f13eeb
to
a63b51d
Compare
cf3a203
to
1daf877
Compare
… Correction d'arrondi
4731652
to
e759148
Compare
No description provided.