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

Fix Contact 'email' property filtered due to 'Emails' HubSpot type #21

Merged
merged 2 commits into from
Jul 1, 2024

Conversation

leolll
Copy link
Contributor

@leolll leolll commented Jun 30, 2024

This PR addresses an issue where the email property was being incorrectly filtered out due to a conflict with the Emails HubSpot type. This was causing a sales-email-read scope error when attempting to access $contact->email, as the code was inadvertently trying to access the $contact->emails associated type instead.

Changes:

  • Added an isAllowedProperty method to centralize property filtering logic
  • Updated fill and __set methods to use isAllowedProperty
  • Specifically allowed email property while maintaining filtering for other HubSpot types

This fix ensures that:

  1. The email property is correctly retained and accessible
  2. The code no longer attempts to access the Emails associated type when simply trying to get the email property from a Contact
  3. The sales-email-read scope error is avoided for email property access

The original intent of filtering out most HubSpot types is maintained, while solving this specific edge case.

This change should resolve issues for users who were encountering unexpected scope errors when working with contact email addresses.

Bug Demonstration

Currently, if you run this code:

$contact = Contact::find(555);
echo $contact->email;

email is not defined. This is because email when pluralized (src/Api/Model.php line 85) is a HubSpot associated type of Contact. It also triggers an attempt to download the Contact's associated Emails, which can lead to unexpected behavior and potential sales-email-read scope errors.

- Add isAllowedProperty method for centralized filtering
- Update fill and __set to use isAllowedProperty
- Allow 'email' while keeping other HubSpot type filtering
- Fix caused by pluralization check ('email' vs 'Emails')

Stops 'email' from being filtered out due to conflict with
'Emails' HubSpot type. Keeps original filtering for other types.
@jszobody
Copy link
Member

This makes sense, I'm surprised I haven't run into it myself.

Can you take a quick look at the failing tests and see about fixing those?

@jszobody jszobody merged commit ea005a0 into stechstudio:main Jul 1, 2024
16 checks passed
@jszobody
Copy link
Member

jszobody commented Jul 1, 2024

Thanks!

@leolll
Copy link
Contributor Author

leolll commented Jul 1, 2024

No problem! Thanks for the great package.

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