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

Phpstan level 2 #245

Open
wants to merge 16 commits into
base: development
Choose a base branch
from
Open

Phpstan level 2 #245

wants to merge 16 commits into from

Conversation

Art4
Copy link
Contributor

@Art4 Art4 commented Jan 30, 2024

This PR fixes all errors for PHPStan level 2. This is a follow-up of #244.

To teach PHPStan about Doctrine repositories, I've installed the phpstan-doctrine extension as dev dependency.

This PR fixes all reported 58 errors, including #242.

@holema
Copy link
Collaborator

holema commented Mar 21, 2024

Hallo @Art4,
die anderen beiden PRs habe ich gemergt. Diese sind noch als Draft eingestellt.

@Art4
Copy link
Contributor Author

Art4 commented Mar 21, 2024

Prima, vielen Dank. Ich habe #244 freigegeben und prüfe noch, ob es mergen Conflicts gibt, die ich beheben muss.

@Art4
Copy link
Contributor Author

Art4 commented Sep 11, 2024

Merge Conflicts sind behoben. Dieser PR ist bereit für das Review.

@Art4 Art4 marked this pull request as ready for review September 11, 2024 10:41
Comment on lines 132 to +134
try {
// FIXME: ResourceOwnerInterface cannot have method getEmail()
return $keycloakUser->getEmail();
if ($keycloakUser instanceof User) {
Copy link
Contributor Author

@Art4 Art4 Oct 11, 2024

Choose a reason for hiding this comment

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

@holema Ich bin mir nicht sicher, wie hier verfahren werden soll. Im Prinzip kann die ganze Methode reduziert werden:

    private function getEmailForKeycloakUser(ResourceOwnerInterface $keycloakUser): string {
        try {
            return $keycloakUser->toArray()['preferred_username'];
        } catch (Exception $e) {
            return '';
        }
    }

Copy link
Collaborator

Choose a reason for hiding this comment

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

Die Idee ist, das versucht wird ob auf die email als Attribut zugegriffen werden kann.
prefered suername muss nicht unbedingt die email Adresse sein sondern kann auch ein username sein.

@Art4 Art4 requested a review from holema October 11, 2024 11:08
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.

2 participants