rollbar-php-symfony-bundle icon indicating copy to clipboard operation
rollbar-php-symfony-bundle copied to clipboard

Possibility to use service as person_fn rather than a static function

Open zinkovskiy opened this issue 1 year ago • 4 comments

Description of the change

This PR allow to use service as person_fn

I created PersonProvider service that is used by default

To use custom person provider you need to define your own service with alias rollbar.person-provider in services.yml of your application, for example:

  rollbar.person-provider:
    class: App\Service\Rollbar\Provider\PersonProvider

No need to define person_fn option (thx to DI), the service will be used automatically, but if person_fn is defined, then it will be used

Logic of obtaining user info is not changed.

I've also discovered, that security.token_storage and serializer are private and cannot be obtained from container, so I passed it to Rollbar\Symfony\RollbarBundle\Factories\RollbarHandlerFactory constructor

Type of change

  • [ ] Bug fix (non-breaking change that fixes an issue)

Related issues

  • Fix #52

Checklists

Development

  • [ ] All tests related to the changed code pass in development

Code review

I tested these changes on symfony 5.4 and symfony 6.4, it works well

@danielmorell could you please take a look on it?

zinkovskiy avatar Dec 07 '23 00:12 zinkovskiy

@zinkovskiy, we are looking for a maintainer or owner of the repo. Would you like to participate?

zdavis-rollbar avatar Feb 11 '24 17:02 zdavis-rollbar

@zinkovskiy, we are looking for a maintainer or owner of the repo. Would you like to participate?

Hi @zdavis-rollbar yes, I'm ready to maintain. contact me anytime when you have time, we are interested in keeping the project up to date

zinkovskiy avatar Feb 15 '24 17:02 zinkovskiy

@zinkovskiy Thank you for the PR. Please add tests for the new functionality, and run tests locally to ensure CI will pass.

waltjones avatar Feb 23 '24 18:02 waltjones

@zinkovskiy Thank you for the PR. Please add tests for the new functionality, and run tests locally to ensure CI will pass.

Hi @waltjones, yes, I missed it. Now it's added Hope it helps to accept changes :slightly_smiling_face:

zinkovskiy avatar Mar 10 '24 00:03 zinkovskiy