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:add check and replace warn instead of error for setCustomNameStan… #55

Conversation

mmustafa-tse
Copy link
Contributor

…dardization

Summary

#54

Testing Plan

Added code to local overrides and tested the warn instead error and then added the if statement to check the callback.

Master Issue

Closes https://go.mparticle.com/work/REPLACE

'Error calling setCustomNameStandardization callback. Check your callback. Data will still be sent without user-defined standardization. See our docs for proper use - https://docs.mparticle.com/integrations/google-analytics-4/event/',
e
);
if (window.GoogleAnalytics4Kit.setCustomNameStandardization(name)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If you're checking if setCustomNameStandardization exists, you might be better off using hasOwnProperty instead of calling the function directly.

If the function does not exist, calling the function would throw an error before the try/catch block. Also, if the function does exist, you're essentially calling it twice.

Suggested change
if (window.GoogleAnalytics4Kit.setCustomNameStandardization(name)) {
if (window.GoogleAnalytics4Kit.hasOwnProperty(setCustomNameStandardization)) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alexs-mparticle thanks for correction, applying the change

Copy link
Collaborator

@rmi22186 rmi22186 left a comment

Choose a reason for hiding this comment

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

Alex's comment is the only comment I have.

'Error calling setCustomNameStandardization callback. Check your callback. Data will still be sent without user-defined standardization. See our docs for proper use - https://docs.mparticle.com/integrations/google-analytics-4/event/',
e
);
if (window.GoogleAnalytics4Kit.hasOwnProperty(setCustomNameStandardization)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

setCustomNameStandardization is not a variable, but rather, a string

Suggested change
if (window.GoogleAnalytics4Kit.hasOwnProperty(setCustomNameStandardization)) {
if (window.GoogleAnalytics4Kit.hasOwnProperty('setCustomNameStandardization')) {

@rmi22186
Copy link
Collaborator

rmi22186 commented Nov 20, 2023

It looks like this test already does not have the callback when testing standardization, so the fact that this test doesn't break is sufficient and no additional test needs to be added.

@rmi22186 rmi22186 requested a review from samdozor November 20, 2023 22:57
@rmi22186 rmi22186 merged commit 7184c2b into mparticle-integrations:development Nov 29, 2023
4 checks passed
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.

4 participants