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

[BE] Small details on LoginControllerTest and AdminController #41

Merged
merged 6 commits into from
Dec 25, 2023

Conversation

FranEnLaNube
Copy link
Contributor

@FranEnLaNube FranEnLaNube commented Dec 20, 2023

@elpupas , these are some contributions to fix #39, maybe it fixes it.

Changes

I just made changes in these files, the rest has been PINT 🤷‍♂️

LoginControllerTest.php

  • Added statement use DatabaseTransactions to avoid refresh data base on each testing or using use refreshDatabase, which resets database in every testing, leaving it totally empty.
  • Delete some unused declared variables.
  • Changed confusing names on some methods
  • Changed routes from $response = $this->json('POST', '/api/v1/login', $credentials); to $response = $this->post(route('login'), $credentials); to avoid SonarCloud messages given that there were string repetition.

AdminController.php

  • Improve indentation to solve SonarCloud messages and take the variable $admin of from findAdmin()

@FranEnLaNube FranEnLaNube self-assigned this Dec 20, 2023
@SectionOne
Copy link
Contributor

Revisat, aparentment s'han corregit els punts d'optimització i alineació del codi.

@SectionOne SectionOne merged commit 495ed47 into feature/fixlogin Dec 25, 2023
@FranEnLaNube FranEnLaNube linked an issue Jan 26, 2024 that may be closed by this pull request
@FranEnLaNube FranEnLaNube changed the title Small details on LoginControllerTest and AdminController [BE] Small details on LoginControllerTest and AdminController Jan 26, 2024
@FranEnLaNube FranEnLaNube deleted the franenlanube/review_of_PR#39 branch January 26, 2024 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[BE] Fix login, DNI instead email
2 participants