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

Compte épargne : initialisation #767

Merged
merged 4 commits into from
Feb 27, 2023
Merged

Conversation

raphodn
Copy link
Member

@raphodn raphodn commented Feb 22, 2023

Quoi ?

  • nouveau type TimeLog.TYPE_SAVING
  • nouveau paramètre use_time_log_saving
  • dans l'entité Membership :
    • créé getShiftTimeLogs() (tous les timeLogs qui ne sont pas TYPE_SAVING)
    • créé getSavingTimeLogs() (tous les timeLogs qui sont TYPE_SAVING)
    • renommé getTimeCount() en getShiftTimeCount (impact sur d'autres fichiers & templates)
    • créé getSavingTimeCount()
  • dans BeneficiaryService :
    • renommé getTimeCount() en getCycleShiftDurationSum() pour éviter de confondre avec celles de Membership
  • dans TimeLogEventListener :
    • faire appel au nouveau initCycleEndRegulateOptionalShiftsTimeLog()

Pourquoi ?

Pour séparer la notion de "compteur-temps" et de "compteur-épargne"

Captures d'écran

Aucun impact sur l'interface dans cette PR

@raphodn raphodn linked an issue Feb 22, 2023 that may be closed by this pull request
@raphodn raphodn force-pushed the raphodn/time-log-saving-feature branch from 837d86c to 54ff720 Compare February 26, 2023 16:36
@raphodn raphodn changed the title Compte épargne Compte épargne : initialisation Feb 26, 2023
@raphodn raphodn marked this pull request as ready for review February 26, 2023 16:45
@raphodn raphodn force-pushed the raphodn/time-log-saving-feature branch from 54ff720 to e6492c9 Compare February 26, 2023 19:05
@raphodn raphodn self-assigned this Feb 26, 2023
return $this->createdAt;
return $this->timeLogs->filter(function (TimeLog $log) {
return ($log->getType() != TimeLog::TYPE_SAVING);
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

Il faudrait le faire directement en SQL. C’est pas très performant de le faire de cette manière. Surtout que ce bout de code est appelé à beaucoup d’endroits.

Copy link
Member Author

@raphodn raphodn Feb 27, 2023

Choose a reason for hiding this comment

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

yes t'as raison. le hic c'est qu'on ne peut pas faire appel à Repository depuis une Entity. il faudrait passer par le Service (un peu comme BeneficiaryService.getTimeCount())

j'ai mis un commentaire dans l'issue, en optimisations à faire après-coup (ca mérite une PR dédiée)

return $this->timeLogs->filter(function (TimeLog $log) {
return ($log->getType() == TimeLog::TYPE_SAVING);
});
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Même remarque ici

return ($log->getCreatedAt() < $before);
});
else
$logs = $this->getTimeLogs();
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Est ce que le before est toujours utilisé dans le code ?

cf mes commentaires précédents ça vaudrait peut être le coup là aussi de faire directement en SQL

Copy link
Member Author

Choose a reason for hiding this comment

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

yes c'est utilisé à pas mal d'endroits..

$log->setTime(-1 * ($counter_today - ($this->due_duration_by_cycle + $allowed_cumul)));
$log->setType(TimeLog::TYPE_CYCLE_END_REGULATE_OPTIONAL_SHIFTS);
Copy link
Collaborator

Choose a reason for hiding this comment

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

J’ai l’impression qu’on perd plus en lisibilité à déplacer ce code dans le service ?

Copy link
Member Author

@raphodn raphodn Feb 27, 2023

Choose a reason for hiding this comment

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

c'est vrai j'ai opté pour répliquer l'usage que j'avais fait pour initShiftValidatedTimeLog qui lui était utilisé à d'autres endroits.
Après je trouve que la fonction initCycleEndRegulateOptionalShiftsTimeLog permet de savoir quels sont les paramètres à fournir, vs devoir tout re-set + faire appel à TYPE_CYCLE_END_REGULATE_OPTIONAL_SHIFTS si l'inhéritance n'est pas utilisée

@raphodn
Copy link
Member Author

raphodn commented Feb 27, 2023

je vais merger pour rebaser / maj les PR par dessus.
j'ai noté tes commentaires pour les requêtes SQL, je le ferais dans une PR dédiée en utilisant les Repository

@raphodn raphodn merged commit 082f222 into master Feb 27, 2023
@raphodn raphodn deleted the raphodn/time-log-saving-feature branch February 27, 2023 17:54
quot17 pushed a commit to quot17/gestion-compte that referenced this pull request Mar 28, 2023
* New parameter: use_time_log_saving

* Add TYPE_SAVING

* Rename getTimeCount to getShiftTimeCount. Create getSavingTimeCount & initCycleEndRegulateOptionalShiftsTimeLog

* fixes
OursDesCavernes pushed a commit to Les400Coop/gestion-compte that referenced this pull request Jan 20, 2024
* New parameter: use_time_log_saving

* Add TYPE_SAVING

* Rename getTimeCount to getShiftTimeCount. Create getSavingTimeCount & initCycleEndRegulateOptionalShiftsTimeLog

* fixes
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.

Création d'un deuxième compteur "épargne"
2 participants