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

Ensure that WooCommerce is actually active [MAILPOET-5613] #5189

Merged

Conversation

websupporter
Copy link
Contributor

@websupporter websupporter commented Sep 29, 2023

Description

In a video you can see how MailPoet crashes during the activation of a lot of plugins. Following the related blog post I tried to replicate the issue. Sometimes it did replicate and I got the fatal error screen, sometimes it didn't.

The error is thrown in the Migrator. It states \WC_Query_Object not found. In the instances where I did not get the fatal error on screen, I was still able to see, that the related migration failed. I couldn't pin it down, but I assume some autoloader of one of the plugins loads the WooCommerce class or the Query class without WooCommerce being active. This PR therefore strenghtens how we determine whether WooCommerce is actually active or not. With this PR I was not able to reproduce the issue and the migration worked as expected.

Code review notes

N/A

QA notes

Not sure if its worth the effort to install 108 plugins and all. I would instead suggest that the normal installation activation still works as expected and that all migrations work without a failure. This PR alters a bit the behavior how we determine whether Woo is active. So it might be worth a quick look whether MailPoet still works as expected with Woo active/inactive.

Linked PRs

N/A

Linked tickets

MAILPOET-5613

After-merge notes

N/A

Tasks

  • I followed best practices for translations
  • I added sufficient test coverage
  • I embraced TypeScript by either creating new files in TypeScript or converting existing JavaScript files when making changes

@websupporter websupporter changed the title Ensure that woocommerce is actually active [MAILPOTE-5613] Ensure that WooCommerce is actually active [MAILPOTE-5613] Sep 29, 2023
@github-actions
Copy link

Pull reviewers stats

Stats of the last 90 days for mailpoet:

User Total reviews Time to review Total comments
Aschepikov
🥇
85
▀▀▀▀
5d 14h 11m
▀▀
0
JanJakes
🥈
21
3d 12h 35m
51
▀▀▀
costasovo
🥉
18
3d 17h 46m
13
johnolek
17
4d 9h 59m
47
▀▀▀
rodrigoprimo
15
3d 9h 28m
26
▀▀
lysyjan
12
4d 9h 21m
9
brezocordero
11
12h 56m
4
websupporter
9
3d 8h 47m
19
triple0t
7
2d 23h 47m
0
alex-mailpoet
6
15h 1m
1
alex-mpoet
6
1d 5h 21m
0
veljkho
3
50m
0
pavel-mailpoet
2
8h 28m
0

Copy link
Contributor

@costasovo costasovo left a comment

Choose a reason for hiding this comment

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

@websupporter I was thinking, why don't we check just by is_plugin_active I see that it looks only to a WP option record. So good that we check both. 👍

@costasovo costasovo assigned veljkho and Aschepikov and unassigned costasovo Oct 2, 2023
Copy link
Collaborator

@Aschepikov Aschepikov left a comment

Choose a reason for hiding this comment

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

GJ!

@Aschepikov Aschepikov merged commit 17ca776 into trunk Oct 11, 2023
22 checks passed
@Aschepikov Aschepikov deleted the MAILPOET-5613-error-when-108-plugins-activate-at-same-time branch October 11, 2023 08:19
@Aschepikov Aschepikov assigned websupporter and unassigned veljkho and Aschepikov Oct 11, 2023
@NeosinneR NeosinneR changed the title Ensure that WooCommerce is actually active [MAILPOTE-5613] Ensure that WooCommerce is actually active [MAILPOET-5613] Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants