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

[README] Avoid method usage when propertiesexist #1

Closed
wants to merge 1 commit into from

Conversation

lyrixx
Copy link
Contributor

@lyrixx lyrixx commented Jan 15, 2015

No description provided.

@javiereguiluz
Copy link
Collaborator

What if the getter method is like the following?

class User
{
    // ...

    public function getFirstName()
    {
        return ucfirst(trim(preg_replace("/[^[:alpha:][:space:]]/ui", '', $this->firstName)));
    }
}

@lyrixx
Copy link
Contributor Author

lyrixx commented Jan 15, 2015

I don't know ;) Feel free to close this PR if it does not match.

@javiereguiluz
Copy link
Collaborator

@lyrixx thanks for opening the first issue of EasyAdminBundle!

I appreciate your issue because I always wonder the same with the getters. From what I've seen, in Symfony applications getters are stupid and useless, because they only return the property. But I'm not sure if other people are using real getters or not.

I'm closing it because it's too early for these documentation details but I'll keep it in mind. Thanks.

javiereguiluz pushed a commit that referenced this pull request Feb 18, 2016
Added option to add target attribute to menu links
javiereguiluz added a commit that referenced this pull request Feb 20, 2016
…for the menu links (maldoinc, javiereguiluz)

This PR was merged into the master branch.

Discussion
----------

Added a "target" option to specify the target attribute for the menu links

This pull request finishes the work done originally by @maldoinc in #845.

Commits
-------

5a00dae Added tests for the feature
13c1e58 Fixed tests
84584b3 Tweaked the new feature
af018e0 Merge pull request #1 from maldoinc/maldoinc-patch-menu-target
142531f Added option to add target attribute to menu links
javiereguiluz pushed a commit that referenced this pull request Feb 7, 2018
javiereguiluz added a commit that referenced this pull request Feb 10, 2018
…imecolin, javiereguiluz)

This PR was merged into the 1.17.x-dev branch.

Discussion
----------

Allow to search by related entities fields

This finishes #1991.

Commits
-------

7ed99c6 Added docs for the new feature
851fce9 Added tests for the new feature
f136e6f Fix tests
6248c60 Fixed tests
6f201dd Fixes
db3b4b4 Code syntax fixes and tweaks
b60b986 Merge pull request #1 from maximecolin/patch-3
e73afa4 Fix multiple join
984018f Searching by related entities fields
@lyrixx lyrixx deleted the patch-1 branch January 21, 2019 16:54
javiereguiluz added a commit that referenced this pull request Mar 18, 2021
… (liarco)

This PR was merged into the 3.0.x-dev branch.

Discussion
----------

Fixing minor PHPDoc error causing static analysis to fail

Hi,
I'm getting errors from PHPStan when I use the `setMenuItems(...)` method from `EasyCorp\Bundle\EasyAdminBundle\Config\UserMenu` like this:
```php
// My dashboard controller
// ...
    public function configureUserMenu(UserInterface $user): UserMenu
    {
        return parent::configureUserMenu($user)
            ->setName($user->getUsername())
            ->setGravatarEmail($user->getUsername())
            ->setMenuItems([MenuItem::linkToLogout('__ea__user.sign_out', 'fa fa-sign-out')])
        ;
    }
// ...
```

The analysis output says:
```
Parameter #1 $items of method EasyCorp\Bundle\EasyAdminBundle\Config\UserMenu::setMenuItems() expects array<EasyCorp\Bundle\EasyAdminBundle\Config\MenuItem>, array<int, EasyCorp\Bundle\EasyAdminBundle\Config\Menu\LogoutMenuItem> given.
```

I think that this minor PHPDoc change should fix that.

What do you think?

Commits
-------

befe94b Fixing minor PHPDoc error causing static analysis to fail
JeroenMoonen added a commit to JeroenMoonen/EasyAdminBundle that referenced this pull request Feb 10, 2023
Fix:
Symfony\Component\Form\Extension\Core\Type\EnumType::Symfony\Component\Form\Extension\Core\Type\{closure}(): Argument EasyCorp#1 ($choice) must be of type ?BackedEnum, string given, called in /vendor/symfony/form/ChoiceList/ArrayChoiceList.php on line 182
javiereguiluz added a commit that referenced this pull request Apr 5, 2023
…einonen)

This PR was merged into the 4.x branch.

Discussion
----------

The error page can't be rendered without the context

```
Uncaught PHP Exception EasyCorp\Bundle\EasyAdminBundle\Exception\EntityNotFoundException: "The "App\Entity\Order" entity with "orderId = " does not exist in the database. The entity may have been deleted by mistake or by a "cascade={"remove"}" operation executed by Doctrine." at /app/vendor/easycorp/easyadmin-bundle/src/Factory/EntityFactory.php line 153
Uncaught Error: Symfony\Bridge\Twig\Extension\AssetExtension::getAssetUrl(): Argument #1 ($path) must be of type string, null given, called in /app/var/cache/prod/twig/ff/ffe222c63463562719428a9eb08ba9a2.php on line 206
Uncaught PHP Exception TypeError: "Symfony\Bridge\Twig\Extension\AssetExtension::getAssetUrl(): Argument #1 ($path) must be of type string, null given, called in /app/var/cache/prod/twig/ff/ffe222c63463562719428a9eb08ba9a2.php on line 206" at /app/vendor/symfony/twig-bridge/Extension/AssetExtension.php line 46
```

In production `EntityNotFoundException` turns from 404 into 500 because the rendering failed here. Pretty annoying because 404s can just be ignored but 500 is something that should probably be looked at.

There's also #5477 which fixes this but this is should be a lot simpler

Commits
-------

eef7c68 The error page can't be rendered without the context
javiereguiluz added a commit that referenced this pull request Jun 22, 2023
This PR was squashed before being merged into the 4.x branch.

Discussion
----------

[Chore] Fix phpstan issues

This PR will fix several `PHPStan` issues:
  - [x] Fix `Cannot unset offset string on array<int, EasyCorp\Bundle\EasyAdminBundle\Dto\ActionDto>.`
  - [x] Fix `Variable method call on Twig\Extension\RuntimeExtensionInterface.`
      - [x] Refactor `EasyAdminTwigExtension::applyFilterIfExists`
      - [x] Add integration tests for `applyFilterIfExists`. Issues #3762 and #4808 have been taken into consideration.
  - [x] Update parameter type declaration of `Action::new`. We can't use ``@var`` to annotate a parameter. See: https://phpstan.org/r/7695e922-db7f-42b3-ac4f-8ff65a8a37b9
  - [x] Modify incorrect PHPDoc. See: https://phpstan.org/r/df4eb1c2-7286-4aa7-b276-f36ff2c0c82c

These issues are a little bit tricky. Both `TranslatableMessageBuilder::withParameters` and `TranslatableChoiceMessage::__toString` are not compatible with `TranslatableInterface`. Those functions should depend on `TranslatableMessage`.
```txt
 ------ -----------------------------------------------------------------------
 Line   Field/Configurator/ChoiceConfigurator.php
 ------ -----------------------------------------------------------------------
  124    Parameter #1 $message of class
         EasyCorp\Bundle\EasyAdminBundle\Translation\TranslatableChoiceMessage
         constructor expects
         Symfony\Component\Translation\TranslatableMessage,
         Symfony\Contracts\Translation\TranslatableInterface given.
 ------ -----------------------------------------------------------------------

 ------ -----------------------------------------------------------------------
 Line   Translation/TranslatableMessageBuilder.php
 ------ -----------------------------------------------------------------------
  28     Call to an undefined method
         Symfony\Contracts\Translation\TranslatableInterface::getMessage().
  29     Call to an undefined method
         Symfony\Contracts\Translation\TranslatableInterface::getParameters().
  30     Call to an undefined method
         Symfony\Contracts\Translation\TranslatableInterface::getDomain().
 ------ -----------------------------------------------------------------------
```

Commits
-------

053f62c [Chore] Fix phpstan issues
javiereguiluz added a commit that referenced this pull request Nov 4, 2023
This PR was squashed before being merged into the 4.x branch.

Discussion
----------

Fix a deprecation in IntlFormatter

Fixes this direct deprecation:

```
Remaining self deprecation notices (1)

  1x: NumberFormatter::format(): Passing null to parameter #1 ($num) of type int|float is deprecated
    1x in IntlFormatterTest::testFormatNumber from EasyCorp\Bundle\EasyAdminBundle\Tests\Intl
``

Commits
-------

c7973cd Fix a deprecation in IntlFormatter
javiereguiluz pushed a commit that referenced this pull request Jan 5, 2024
* Update Portuguese translation
javiereguiluz added a commit that referenced this pull request Jan 5, 2024
This PR was merged into the 4.x branch.

Discussion
----------

Update Portuguese translation

Updated the translation for the European Portuguese language since we use this locale for ourselves and have someone on the team who knows the language :)

Commits
-------

677e591 Update EasyAdminBundle.pt.php (#1)
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.

None yet

2 participants