Feature/1 #2

zappaciato · 2023-09-08T11:47:16Z

first pull request.
ORM installed
migration for User
UserServices
*to verify the expected output form UserController

…ed - OK. User findAll and User Find by ID initially tested - OK. Routing Requires fixing - only works on root path: localhost.

lukaszozim

Poza drobnymi uwagami to bardzo fajny poczatek

lukaszozim · 2023-09-08T11:55:36Z

.env

+# DATABASE_URL="sqlite:///%kernel.project_dir%/var/data.db"
+# DATABASE_URL="mysql://app:[email protected]:3306/app?serverVersion=8.0.32&charset=utf8mb4"
+# DATABASE_URL="mysql://app:[email protected]:3306/app?serverVersion=10.11.2-MariaDB&charset=utf8mb4"
+  DATABASE_URL="mysql://root:@127.0.0.1:3306/php_symfony_db?serverVersion=mariadb-10.4.22"


tu był moj błąd ze wrzucilem do repo plik .env, ogólnie podczas git add pomijaj plik .env, ja go wrzuce później do gitignora. bo w ten sposób kazdy przejmuje Twoje ustawienia bazy. To bazy lokalne ale gyby tam były dane dostępowe do prawdziwej bazy, to za to można nawet stracić robote.

faktycznie, byłem przekonany, że z automatu go ignoruje. Przynajmniej tak było z Laravelem. Nie zwróciłem uwagi kompletnie.

lukaszozim · 2023-09-08T11:58:03Z

migrations/Version20230908082627.php

+    public function up(Schema $schema): void
+    {
+        // this up() migration is auto-generated, please modify it to your needs
+        $this->addSql('ALTER TABLE user CHANGE phone_number phone_number INT DEFAULT NULL');


Z migracjami coś jest nie tak. Nie ma skryptu tworzącego tabele user. Wygląda tak jakbyś albo nie załączył pliku z migracją lub utworzył tabele bezpośrednio na bazie dnaych. Migracje nie zadziałają, bo od razu robi altera na tabeli której nie ma. Podpowiedź taka, ze trzeba wyrzucić tą migracje, zrobić doctrine:migration:diff, usunać tabelke z bazy dancyh i przeprowadzić ponowną migracje. z tabelki na bazie danych migrations należy usunac wpis w razie potrzeby

Poprawiłem to. Mój błąd wynikał z tego, że postawilem te migracje, a potem chcialem dodac emaila i uruchomilem jeszcze raz, zanim skasowalem poprzednią, a tam z automatu wstawiło alter zamiast create.

Nie wiem czy to ma znaczenie, ale wyrzuca mi takie info przy migracjach np.

2023-09-08T15:07:24+02:00 [info] User Deprecated: Method "Symfony\Component\HttpKernel\Bundle\Bundle::build()" might add "void" as a native return type declaration in the future. Do the same in child class "Sensio\Bundle\FrameworkExtraBundle\SensioFrameworkExtraBundle" now to avoid errors or add an explicit @return annotation to suppress this message.
2023-09-08T15:07:24+02:00 [info] User Deprecated: Version detection logic for MySQL will change in DBAL 4. Please specify the version as the server reports it, e.g. "10.9.3-MariaDB" instead of "mariadb-10.9". (AbstractMySQLDriver.php:130 called by AbstractMySQLDriver.php:37, doctrine/dbal#5779, package doctrine/orm)
2023-09-08T15:07:24+02:00 [info] User Deprecated: Context: trying to commit a transaction
Problem: the transaction is already committed, relying on silencing is deprecated.
Solution: override AbstractMigration::isTransactional() so that it returns false.
Automate that by setting transactional to false in the configuration.
More details at https://www.doctrine-project.org/projects/doctrine-migrations/en/stable/explanation/implicit-commits.html (TransactionHelper.php:26 called by
DbalExecutor.php:214, doctrine/migrations#1169, package doctrine/migrations)

lukaszozim · 2023-09-08T11:58:55Z

src/Controller/HelloWorldController.php

+
+class HelloWorldController extends AbstractController
+{
+    #[Route('/hello-world ', name: 'app_hello_world')]


to nie było cześćią zadania, rozumiem, ze ocś próbowałeś. Ogólnie byłoby do usunięcia z MR jako kod niemający związku z zadaniem, ale tutaj moze zostać

tak, chciałem zoabczyc jak cos tam zadziała. Do usunięcia zdecydowanie.

lukaszozim · 2023-09-08T11:59:25Z

src/Controller/UserController.php

+class UserController extends AbstractController
+{
+
+    #[Route(' ', name: 'app_users')]


dlatego działa Ci tlko na pierwszej stronie bo tak jest to zmapowane. Tu powinien być url = /users

Tutaj zrobilem tak, bo jesli jest users, to właśnie wywala mi 404. zmienilem wiec na pusty, zeby przetestowac na localhost czy w ogole reszta kodu zadziałała.

Jeszcze raz sprawdziłem i wywala 404.

lukaszozim · 2023-09-08T12:00:37Z

src/Controller/UserController.php

+    public function index(UserServices $userServices): JsonResponse
+    {
+        $users= $userServices->getAllUsers();
+        print_r($users);


odpowiedzią jest to co w return czyli json. tego print_r już nie stosujemy.
Jak bedziesz chcial się dowiedzieć np co będzie zwrócone lub prontować sobie poszczególne etapy procesu to pokaże Ci przydatne triki. w kazdym razie print_r do usunięcia bo zwraca dwa razy ten sam wynik.

Niedopatrzenie. Przyzwyczaiłem się ciągle coś tam drukować. A triki na tego typu debugging jak najbardziej, im wiecej tym lepiej dla mnie.

lukaszozim · 2023-09-08T12:01:03Z

src/Controller/UserController.php

+    public function getUserById(UserServices $userServices): JsonResponse
+    {
+        $user = $userServices->getUserById(1);
+        print_r($user);


do usunięcia

tutaj też dla testów stworzyłem. Będzie usunięte.

lukaszozim · 2023-09-08T12:05:50Z

src/Controller/UserController.php

+
+
+    #[Route('/create-user ', name: 'create_user')]
+    public function createUser(EntityManagerInterface $entityManager) : Response {


tu tak jak każe dokumentacja symfony, a to jest to co mówiłem, tak się nie robi. Tutaj możesz poczytać o zasadzie "thin controller" głównie w projektach opartych na MVC. Taka logika powinna zanelźć się w serwisie. A takie na sztywno wpisywane dane to fixtury, je robi się w innym miejscu. Dodawanie usera będzie trzeba mocno przerobić, ale póki co niech to zostanie. Zrobimy zadanie na refactor.

raczej z dodawaniem usera to się spodziewalem, ze nie jest dobrze, bo jedynie chcialem przetestowac czy da rade wbic do bazy w ten sposob jak napisalem. Faktycznie korzystalem z dokumentacji, i nawet nie spodziewalem sie, ze od razu wpadne w pułapkę ;)

lukaszozim · 2023-09-08T12:06:42Z

src/Entity/User.php

+    #[ORM\Id]
+    #[ORM\GeneratedValue]
+    #[ORM\Column]
+    private ?int $id = null;


od jakiegoś czau m in ze względów bezpieczeństa nie stosuje się już id tylko UUid. To może poruszymy jak się zdzwonimy.

Tak, jasne, wspomnij na pewno o tym. Ja wiem, ze szybciej sie wyszukuje, i duzo lepiej to chodzi. Wczoraj wieczorem jeszcze myślałęm właśnie, żeby tak zrobic.

Na razie to zostawiam jak jest, do momentu jak sie spotkamy.

lukaszozim · 2023-09-08T12:08:07Z

src/Service/UserServices.php

+
+    public function getAllUsers() : array
+    {
+        $users = $this->userRepository->findAll();


Tu ten temat o którym mówiłem, czy potrafisz czytać i korzystac z kodu już napisanego. Bardzo dobrze ze wykorzystałeś już dostępne metody a nie pisałeś ich samodzielnie.

lukaszozim · 2023-09-08T12:10:28Z

src/Repository/UserRepository.php

+ */
+class UserRepository extends ServiceEntityRepository
+{
+    public function __construct(ManagerRegistry $registry)


To co tu zrobiłes jest jak najbardziej OK, jest to klasyczne DI, natomiast zgodnie ze sztuką najlepiej jest opierać się o wzorzec IoC (Inversion of controll) ściśle związane z DI ale daje jedną kluczową zalete nie towrzysz zależności do klas rzeczywistych tylko opierasz się na abstrakcji, czyli interfejsach. To niech zostanie póki co , w zadaniu z refaktorem to poprawisz jak doczytasz na czym to polega lub jak sobie o tym opowiemy.

to temat grubszy dla mnie i pewnie trzeba będzie to ze mną przeorać porzadniej cos czuję. To zrobiłem tak lekko na czuja prawde mówiąc a wolałbym wiedziec co robie.

Ogólnie tak jak zrobiłeś to jest akceptowalne, natomiast można to zrobić lepiej

lukaszozim · 2023-09-08T14:13:34Z

Dobra robota!

zappaciato added 2 commits September 8, 2023 10:04

User entity created/Db_migrtion done/Additional packaged added -> ORM.

e61f812

User: controler,service,Repository - done. User create initially test…

931da0e

…ed - OK. User findAll and User Find by ID initially tested - OK. Routing Requires fixing - only works on root path: localhost.

lukaszozim changed the base branch from master to develop September 8, 2023 11:53

lukaszozim reviewed Sep 8, 2023

View reviewed changes

Drobne poprawki

74812d1

lukaszozim force-pushed the feature/1 branch from 1705933 to 74812d1 Compare September 8, 2023 14:08

Łukasz Ozimek added 2 commits September 8, 2023 16:10

Dodanie migracji

a48522c

usunięcie niepotrzebnej migracji:

7d88dcf

lukaszozim merged commit 7766aca into develop Sep 8, 2023



		#[Route('/create-user ', name: 'create_user')]
		public function createUser(EntityManagerInterface $entityManager) : Response {

Feature/1 #2

Feature/1 #2

Uh oh!

Conversation

zappaciato commented Sep 8, 2023

Uh oh!

lukaszozim left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lukaszozim commented Sep 8, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants