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

Dynamic callback function name cannot include hyphens #1468

Closed
shadi-sharaf-nuk opened this issue Nov 20, 2023 · 6 comments · Fixed by #1469
Closed

Dynamic callback function name cannot include hyphens #1468

shadi-sharaf-nuk opened this issue Nov 20, 2023 · 6 comments · Fixed by #1469
Labels
Milestone

Comments

@shadi-sharaf-nuk
Copy link

shadi-sharaf-nuk commented Nov 20, 2023

Bug Report

Dynamic callback methods in custom connectors, eg: callback_ACTION_NAME cannot have hyphens in their method name, however the callback discovery routine does not convert hyphens to an underscore, hence connectors cannot target actions with hyphens like load-xxx_xxx unless they override the callback method and target the action manually.

Expected Behavior

For an action like load-something, I should be able to use callback_load_something to log that action.

Actual Behavior

I can't use a dynamic callback to target that action, only way is to override the callback method.

System Information

  • Stream plugin version: ALL, including current 3.10.0
  • WordPress version: ANY
  • PHP version: ANY
  • Browser: ANY
  • Computer operating system: ANY
@shadi-sharaf-nuk
Copy link
Author

Hey @schlessera @kasparsd, trying to issue a PR against this issue, but seems like external contributions might be disabled?

image

@kasparsd
Copy link
Contributor

@shadi-sharaf-nuk Can you please link to the branch you're creating the pull request from? I'm seeing external pull request being created very recently such as #1462.

@shadi-sharaf-nuk
Copy link
Author

@kasparsd
Copy link
Contributor

Interesting. This is what I'm seeing -- looks like it would allow me to create a pull request:

pr

Can you please share what you are seeing?

@shadi-sharaf-nuk
Copy link
Author

I'm seeing the same as you, but when I submit, I see the error I attached above 🤷

Unsure if this is related to me being a previous contributor maybe?

@shadyvb
Copy link
Contributor

shadyvb commented Nov 20, 2023

My bad, I stupidly was trying to create the PR from another GitHub account who doesn't have access to my fork 🤦 Sorry for the confusion.

@delawski delawski added this to the 4.0.1 milestone Jul 16, 2024
@delawski delawski added the bug label Jul 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants