8000 [Notifier] Add FakeSMS Logger transport by noniagriconomie · Pull Request #42123 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Notifier] Add FakeSMS Logger transport #42123

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 31, 2021
Merged

[Notifier] Add FakeSMS Logger transport #42123

merged 1 commit into from
Jul 31, 2021

Conversation

noniagriconomie
Copy link
Contributor
Q A
Branch? 5.4
Bug fix? no
New feature? yes
Deprecations? no
Tickets Sub part of #40625
License MIT
Doc PR WIP

Friendly ping @OskarStark

As commented here I use mainly the sms transport, thus wanted to work on the fake sms. This PR adds the logger.

For the part an optional channel, how can we get to here? dymanically retreiving the proper logger based on the dsn config?

@carsonbot carsonbot changed the title Add FakeSMS Logger transport [Notifier] Add FakeSMS Logger transport Jul 15, 2021
@nicolas-grekas nicolas-grekas added this to the 5.4 milestone Jul 15, 2021
@noniagriconomie
Copy link
Contributor Author

@OskarStark done, if wanted, I can create a second PR for FakeChatLoggerTransport

@OskarStark
Copy link
Contributor
OskarStark commented Jul 16, 2021 •
edited
Loading

@OskarStark done, if wanted, I can create a second PR for FakeChatLoggerTransport

Yes please do 👍🏻

@noniagriconomie noniagriconomie mentioned this pull request Jul 19, 2021
Merged
@noniagriconomie noniagriconomie changed the title [Notifier] Add FakeSMS Logger transport [WIP] [Notifier] Add FakeSMS Logger transport Jul 19, 2021
@noniagriconomie noniagriconomie changed the title [WIP] [Notifier] Add FakeSMS Logger transport [Notifier] Add FakeSMS Logger transport Jul 19, 2021
@Nyholm
Copy link
Member
Nyholm commented Jul 19, 2021

Sorry for asking. But why can't we just add a logger to the FakeSMS transport?

@OskarStark
Copy link
Contributor

In this case one need to configure the email stuff just to get the logging.

If you have no mailing in your project, you would need to set it up just for local dev to have the entry point via DSN

OskarStark referenced this pull request Jul 21, 2021
@nicolas-grekas
[Notifier] cs
ef84e66
@OskarStark
Copy link
Contributor

Please add a test case to UnsupportedSchemeExceptionTest

Thanks

@noniagriconomie
Copy link
Contributor Author
noniagriconomie commented Jul 30, 2021 •
edited
Loading

@OskarStark

PR rebased, also on

Please add a test case to UnsupportedSchemeExceptionTest

I am sorry I really do not understand this, when I check into this class Notifier\UnsupportedSchemeExceptionTest, I already see logic on FakeSmsTransportFactory

can you be more precise on the way to do the required test? thank you

edit: appveyor failure is not related to this addition

@OskarStark
Copy link
Contributor

You are right, it is just a new transport for an existing bridge 👍

@OskarStark
Copy link
Contributor

Thanks for your work on this new feature!

@OskarStark OskarStark merged commit 350674e into symfony:5.4 Jul 31, 2021
@OskarStark OskarStark mentioned this pull request Jul 31, 2021
Closed
@noniagriconomie noniagriconomie deleted the ft-40625 branch August 1, 2021 18:14
OskarStark added a commit that referenced this pull request Aug 4, 2021
@OskarStark
feature #42188 [Notifier] Add FakeChat Logger transport (noniagricono…
7c1820d 
…mie)

This PR was merged into the 5.4 branch.

Discussion
----------

[Notifier] Add FakeChat Logger transport

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | Sub part of #40625
| License       | MIT
| Doc PR        | WIP

Refs #42123 for Notifier FakeChat

Commits
-------

2bfe06f Add FakeChat Logger transport
symfony-splitter pushed a commit to symfony/framework-bundle that referenced this pull request Aug 4, 2021
@OskarStark
feature #42188 [Notifier] Add FakeChat Logger transport (noniagricono…
c9ed8d1 
…mie)

This PR was merged into the 5.4 branch.

Discussion
----------

[Notifier] Add FakeChat Logger transport

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | Sub part of #40625
| License       | MIT
| Doc PR        | WIP

Refs symfony/symfony#42123 for Notifier FakeChat

Commits
-------

2bfe06fac7 Add FakeChat Logger transport
This was referenced Nov 5, 2021
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@OskarStark OskarStark OskarStark approved these changes

@derrabus derrabus derrabus approved these changes

Assignees
No one assigned
Labels
Feature Notifier Status: Reviewed
Projects
None yet
Milestone
5.4
Development

Successfully merging this pull request may close these issues.
6 participants
@noniagriconomie @OskarStark @Nyholm @derrabus @nicolas-grekas @carsonbot
0