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

Adds the calling class to the string for the ajax hash to prevent collisions #794

Merged
merged 3 commits into from
Feb 15, 2021

Conversation

mslinnea
Copy link
Member

Addresses #621

Copy link
Member

@dlh01 dlh01 left a comment

Choose a reason for hiding this comment

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

🌳 I wondered whether the post, term, and user datasources should get the same fix, since they override the default get_ajax_action(). But I see that each has a distinct leading string like fm_datasource_post and fm_datasource_term...except for the user datasource, which also uses fm_datasource_post (oops!). Can we switch that to fm_datasource_user while we're here? (Or drop the _type_ prefix from each and use get_called_class() like in the parent class.)

@mboynes
Copy link
Contributor

mboynes commented Feb 15, 2021

except for the user datasource, which also uses fm_datasource_post

😆

@mslinnea
Copy link
Member Author

@dlh01 I updated the other classes to all use get_called_class() since that will prevent this same issue if those classes were extended.

@dlh01
Copy link
Member

dlh01 commented Feb 15, 2021

@mslinnea Unfortunately, it seems that using get_called_class() on its own will cause datasources to break when the class name is under a namespace: The action name in $_POST will get slashed and will no longer match the action name generated by Fieldmanager, like \\My\\Datasource instead of \My\Datasource.

I'm wondering whether the class name should be part of the hashed string instead, so maybe fm_datasource_{$hash}.

@mslinnea
Copy link
Member Author

@dlh01 Valid point, I've updated these so that the called class is part of the hash, and I also fixed the preface for the user datasource.

Copy link
Member

@dlh01 dlh01 left a comment

Choose a reason for hiding this comment

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

🌳 Now we're cookin' with get_called_class()!

@mslinnea mslinnea merged commit 45e0505 into master Feb 15, 2021
@dlh01 dlh01 deleted the 627-ajax-action-name branch February 15, 2021 21:59
@dlh01 dlh01 added this to the 1.4.0 milestone Jan 6, 2022
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.

3 participants