Ajout de room_id dans timeSlot#2
Open
JulienSauguesEyraud wants to merge 4 commits into
Open
Conversation
Pierstoval
reviewed
Apr 21, 2026
| $room = $request->request->get('room_id'); | ||
| $event = $request->request->get('event_id'); | ||
| if($booth && $room) { | ||
| if($room != $booth->get('room_id')) { |
Member
There was a problem hiding this comment.
- Cette ligne renverra toujours
false, et une erreur, parce qu'à ce stade du code,$boothcontient le contenu de$request->request->get('booth_id'), qui est un scalaire, et pas un objet. - Il faut toujours utiliser
===ou!==pour comparer la valeur ET le type, ça évite de faire du type juggling : https://www.php.net/manual/fr/language.types.type-juggling.php
Pierstoval
reviewed
Apr 21, 2026
Comment on lines
+35
to
+41
| $this->addSql('DROP INDEX IDX_75EA56E0FB7336F0E3BD61CE16BA31DBBF396750 ON messenger_messages'); | ||
| $this->addSql('CREATE INDEX IDX_75EA56E016BA31DB ON messenger_messages (delivered_at)'); | ||
| $this->addSql('CREATE INDEX IDX_75EA56E0FB7336F0 ON messenger_messages (queue_name)'); | ||
| $this->addSql('CREATE INDEX IDX_75EA56E0E3BD61CE ON messenger_messages (available_at)'); | ||
| $this->addSql('ALTER TABLE time_slot DROP FOREIGN KEY FK_1B3294A54177093'); | ||
| $this->addSql('DROP INDEX IDX_1B3294A54177093 ON time_slot'); | ||
| $this->addSql('ALTER TABLE time_slot DROP room_id'); |
Member
There was a problem hiding this comment.
Ici, tu peux enlever tout ça : on ne va pas rollback une BDD par ce biais, on créera une nouvelle migration (et donc de nouveaux commits) pour corriger le bug.
Dans d'autres projets, le rollback avec la fonction down() peut être utile, mais en 15 ans je ne l'ai jamais vu utilisé en prod pour le moment 😁
Pierstoval
reviewed
Apr 21, 2026
Comment on lines
+26
to
+29
| $this->addSql('DROP INDEX IDX_75EA56E016BA31DB ON messenger_messages'); | ||
| $this->addSql('DROP INDEX IDX_75EA56E0FB7336F0 ON messenger_messages'); | ||
| $this->addSql('DROP INDEX IDX_75EA56E0E3BD61CE ON messenger_messages'); | ||
| $this->addSql('CREATE INDEX IDX_75EA56E0FB7336F0E3BD61CE16BA31DBBF396750 ON messenger_messages (queue_name, available_at, delivered_at, id)'); |
Member
There was a problem hiding this comment.
Ça concerne la table de Symfony/Messenger, il faudrait supprimer ces lignes, et créer une autre classe de migration juste pour ce besoin-là (qui me paraît étrange quand même)
Pierstoval
reviewed
Apr 21, 2026
Comment on lines
+56
to
+57
| } | ||
| else if($booth) { |
Member
There was a problem hiding this comment.
Suggested change
| } | |
| else if($booth) { | |
| } elseif ($booth) { |
(ce genre de problème peut être corrigé avant de commit avec la commande make cs)
Pierstoval
reviewed
Apr 21, 2026
Comment on lines
+141
to
+170
|
|
||
| /** | ||
| * @return Collection<int, TimeSlot> | ||
| */ | ||
| public function getTimeSlots(): Collection | ||
| { | ||
| return $this->timeSlots; | ||
| } | ||
|
|
||
| public function addTimeSlot(TimeSlot $timeSlot): static | ||
| { | ||
| if (!$this->timeSlots->contains($timeSlot)) { | ||
| $this->timeSlots->add($timeSlot); | ||
| $timeSlot->setRoom($this); | ||
| } | ||
|
|
||
| return $this; | ||
| } | ||
|
|
||
| public function removeTimeSlot(TimeSlot $timeSlot): static | ||
| { | ||
| if ($this->timeSlots->removeElement($timeSlot)) { | ||
| // set the owning side to null (unless already changed) | ||
| if ($timeSlot->getRoom() === $this) { | ||
| $timeSlot->setRoom(null); | ||
| } | ||
| } | ||
|
|
||
| return $this; | ||
| } |
Member
There was a problem hiding this comment.
Inutile ici : on n'accède pas à un TimeSlot depuis la classe Room. Y'a pas de raison de le faire, donc ce code peut être enlevé direct 👍
Pierstoval
reviewed
Apr 21, 2026
Comment on lines
+44
to
+47
| #[ORM\ManyToOne(inversedBy: 'timeSlots')] | ||
| #[ORM\JoinColumn(nullable: false)] | ||
| private Room $room; | ||
|
|
Member
There was a problem hiding this comment.
Est-ce que tu peux déplacer cette propriété juste au-dessus de la propriété $booth ?
Pierstoval
reviewed
Apr 21, 2026
Pierstoval
reviewed
Apr 21, 2026
Pierstoval
requested changes
Apr 29, 2026
Member
Pierstoval
left a comment
Pierstoval
left a comment
There was a problem hiding this comment.
Il faut également supprimer le dossier .idea du versioning
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.