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

Duplicate EmailCommon::buildQuickForm onto the trait #17052

Merged
merged 1 commit into from
Apr 10, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Apr 10, 2020

Overview

Copy EmailCommon::buildQuickForm onto the new trait we are using instead of the static class for shared email form functionality

I have duplicated this code rather than moving it in case any non-core code calls it. We can remove in a bit

Before

buildForm is on the static class - it references many form attributes that are inconsistently defined

After

buildForm is on the trait. The old code remains in the static function with a deprecation notice in case anyone is accessing it in an unsupported way

Technical Details

Comments

I have duplicated this code rather than moving it in case any non-core code calls it. We can remove in a bit
@civibot
Copy link

civibot bot commented Apr 10, 2020

(Standard links)

@civibot civibot bot added the master label Apr 10, 2020
@demeritcowboy
Copy link
Contributor

  • General standards
    • [r-explain] PASS
    • [r-user] PASS
    • [r-doc] PASS
    • [r-run] PASS Works
  • Developer standards
    • [r-tech] PASS
    • [r-code] PASS
      • There's a couple === that snuck in there but otherwise is the same.
    • [r-maint] PASS
    • [r-test] PASS

@eileenmcnaughton
Copy link
Contributor Author

thanks @demeritcowboy

@eileenmcnaughton eileenmcnaughton merged commit 92fc8db into civicrm:master Apr 10, 2020
@eileenmcnaughton eileenmcnaughton deleted the email5 branch April 10, 2020 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants