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

Make flexmailer mandatory #25110

Merged
merged 1 commit into from Dec 8, 2022
Merged

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Make flexmailer mandatory

Before

  • flexmailer optional
  • setting permits fallback to 'traditional_mode`

After

  • flexmailer mandatory
  • fallback setting removed

Technical Details

The next challenge will be to figure out how to remove the chunks of code that this makes fully obsolete

Comments

@civibot
Copy link

civibot bot commented Dec 3, 2022

(Standard links)

@civibot civibot bot added the master label Dec 3, 2022
@eileenmcnaughton eileenmcnaughton force-pushed the mandatory_flex branch 2 times, most recently from 67575d3 to 1ec07e6 Compare December 5, 2022 23:01
@demeritcowboy
Copy link
Contributor

This seems ok. I know there was some discussion in chat but I didn't follow it so not sure if there's something to look out for here.

I did have a weird experience on one run because I had happened to visit the extension page first before the upgrade (and it oddly looked installed but that's not the issue). When I then went to the upgrade screen it crashed since it couldn't find \Civi\Services\Flexmailer until I cleared cache. But I'm chalking it up to just clicking around too much.

@demeritcowboy demeritcowboy added the merge ready PR will be merged after a few days if there are no objections label Dec 6, 2022
@demeritcowboy
Copy link
Contributor

demeritcowboy commented Dec 6, 2022

Also I do still have flexmailer settings in the Admin - CiviMail menu, but not a blocker. It just takes you to the admin page since it doesn't go anywhere.

@seamuslee001
Copy link
Contributor

My only slight hesitation on this is have we had any notices to those that have Flexmailer enabled but the setting set to bao?

@eileenmcnaughton
Copy link
Contributor Author

@seamuslee001 we removed the last reason to have it as BAO with the double https thing

@seamuslee001
Copy link
Contributor

@eileenmcnaughton my point being is what if people had previously deliberately set it to be BAO for whatever reason this would now completely change things for them I would assume.

@eileenmcnaughton
Copy link
Contributor Author

@seamuslee001 yeah - the bao option is gone for both those people & those who don't have flexmailer - personally I only ever encountered sites with bao set by accident who had bugs that were fixed by turning it on - I never saw anyone suggest it as a solution to a problem ....

@colemanw colemanw merged commit f7d3c9d into civicrm:master Dec 8, 2022
@colemanw colemanw deleted the mandatory_flex branch December 8, 2022 02:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
master merge ready PR will be merged after a few days if there are no objections
Projects
None yet
4 participants