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

[WIP] Removing SendGrid UI #3615

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

[WIP] Removing SendGrid UI #3615

wants to merge 1 commit into from

Conversation

riverBanjo
Copy link
Collaborator

Removes SendGrid UI and add a hook to control the SendGrid email settings.

Fixes #3614

What has changed?

  • Removed SendGrid UI
  • Added a filter change_email_settings

How to test?

  1. Check Email settings
  2. No visual representation of SendGrid
  3. Hook into filter
  4. Use xdebug to see if data is being overwritten correctly

… Option from the Email System setting. Also, adds filter to allow users to change email settings should they so choose to use SendGrid.
@riverBanjo riverBanjo requested review from Shelob9 and New0 August 31, 2020 19:28
Copy link
Collaborator

@Shelob9 Shelob9 left a comment

Choose a reason for hiding this comment

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

@riverBanjo This looks good overall. I left some inline comments with some concerns about the filter.


return self::$settings;
return apply_filters( 'change_email_settings', self::$settings, $arg1, $arg2 );
Copy link
Collaborator

Choose a reason for hiding this comment

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

@riverBanjo This filter is in the right place. A few concerns:

  1. Filter names should be prefeixed with caldera_forms_
  2. $arg1 and $arg2 are undefined. Please delete those.
  3. There are no inline docs

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