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

Changes property access of product id to public. #2618

Merged
merged 1 commit into from
Aug 16, 2023

Conversation

krutidugade
Copy link
Contributor

Changes proposed in this Pull Request:

Private access of $id property done in PR 2606, breaks the product feed functionality. The Product feed doesn't populate variable products. This is happening because the id can't be accessed by fbproductfeed.php file at

$product_data['default_product'] = $parent_attribute_values['default_variant_id'] == $woo_product->id ? 'default' : '';

This PR changes the access from private to public which restores the original product feed functionality.

  • Do the changed files pass phpcs checks? Please remove phpcs:ignore comments in changed files and fix any issues, or delete if not practical.

Screenshots:

Before:

Screenshot 2023-08-16 at 1 22 35 PM

After:

Screenshot 2023-08-16 at 1 22 13 PM

Detailed test instructions:

  1. Install and Activate the plugin
  2. Switch to the branch in this PR
  3. Go to WooCommerce > Status > Scheduled Actions > Pending
  4. Run wc_facebook_regenerate_feed action
  5. Go to wp-content/uploads/facebook_for_woocommerce folder
  6. Open the product feed csv file
  7. Verify you see variable products along with all other products

Changelog entry

Fix - Changes property access of product id from private to public

@krutidugade krutidugade self-assigned this Aug 16, 2023
@github-actions github-actions bot added changelog: fix Took care of something that wasn't working. type: bug The issue is a confirmed bug. labels Aug 16, 2023
Copy link
Contributor

@ibndawood ibndawood left a comment

Choose a reason for hiding this comment

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

Thank you, @krutidugade, for the changes. Excellent work with the smoke testing and debugging.

@ibndawood ibndawood mentioned this pull request Aug 16, 2023
29 tasks
@krutidugade krutidugade merged commit ebf90ee into develop Aug 16, 2023
6 checks passed
@krutidugade krutidugade deleted the fix/dynamic-property-access branch August 16, 2023 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: fix Took care of something that wasn't working. type: bug The issue is a confirmed bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants