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

Content changes to track in Altis #506

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

robindevitt
Copy link
Contributor

@robindevitt robindevitt self-assigned this Jul 1, 2022
@robindevitt robindevitt marked this pull request as draft July 1, 2022 09:36
Comment on lines 514 to 516
if ( defined( 'DOING_AUTOSAVE' ) && DOING_AUTOSAVE || 'auto-draft' === $post->post_status ) {
return;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have had to include the auto-draft status here in the return as it seems to apply when a new post ( the exact post_type ) is created but not other post types. Not returning then fires off the tracking event. Essentially firing two tracking events when creating a new post.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would do this in a separate if statement. Read through the logic here, I don't think it will do what you in every scenario. What if DOING_AUTOSAVE is defined but false. You should never mix && operators directly with ||.

@robindevitt robindevitt marked this pull request as ready for review July 18, 2022 08:03
@robindevitt robindevitt marked this pull request as draft July 18, 2022 08:03
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