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

Tool Provider Private Methods #19

Closed
jkm9000 opened this issue May 9, 2019 · 4 comments
Closed

Tool Provider Private Methods #19

jkm9000 opened this issue May 9, 2019 · 4 comments

Comments

@jkm9000
Copy link

jkm9000 commented May 9, 2019

Hello, thanks for this fork. I'm doing my first implementation of LTI and was glad to come across this since the original is now abandoned.

I am trying to integrate this into an existing Symfony application and it's a bit of a challenge using the ToolProvider class. I'm still very early on but what's hanging me up comes down to being unable to override some of these private methods.

For example, result() directly calls header() to redirect, and other times it does inline echo statements. That makes it very hard to try and pass data to views, add logging, etc.

At first I thought about using output buffering to capture the echos but the inline header() calls make that problematic.

I could just fork this repository and handle it that way, but I thought maybe others could benefit. Is there a good reason to keep these private? Has anyone else integrated this into a framework like Symfony and worked around this in other ways?

Thanks much!

@Izumi-kun
Copy link
Owner

In current implementation you can override handleRequest method and replace result call with something you needed.

@jkm9000
Copy link
Author

jkm9000 commented May 21, 2019

Thanks. Unfortunately, authenticate() is also a private method so looks like I would have to implement that as well. If these methods were protected instead, it would be a lot easier to work with.

If that's not something you're willing to do, I can always just make a fork.

@Izumi-kun
Copy link
Owner

Agree. PR is merged. Thanks!

@jkm9000
Copy link
Author

jkm9000 commented May 22, 2019

Great, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants