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

Refactor AJAX controller to use plugin manager #1138

Merged
merged 67 commits into from Mar 21, 2018
Merged

Conversation

demiankatz
Copy link
Member

@demiankatz demiankatz commented Mar 9, 2018

The AJAX controller is currently overcomplicated and has too many dependencies. This PR is intended to refactor it to use a plugin manager so that the separate AJAX routines can be isolated from one another, and new actions can be more easily added.

TODO

  • Discuss whether there is a better way to handle the "no session write" functionality
  • Finish refactoring existing methods
  • Make sure all tests still pass
  • Write documentation

@demiankatz
Copy link
Member Author

@EreMaijala, I believe that this refactoring is now complete, and all tests are passing. Since you've had a fairly significant hand in some of the past AJAX work, I'm interested to hear your thoughts on this final version. If you have any suggestions for changing the interface of the AjaxHandler classes or if you dislike any of the changes to the AjaxController, please let me know.

I know there are a ton of files in this diff, but you can ignore most of the code in the AjaxHandler namespace -- once you've looked at one or two of those, you'll get the basic idea of how they work.

Copy link
Contributor

@EreMaijala EreMaijala left a comment

Choose a reason for hiding this comment

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

Looks good to me. I tested by converting one of our Ajax methods to the plugin manager style and got it to work with relatively little fuzz.

@demiankatz
Copy link
Member Author

Thank, @EreMaijala! I'll merge it today.

@demiankatz demiankatz merged commit 587f141 into master Mar 21, 2018
@demiankatz demiankatz deleted the ajax-refactor branch October 22, 2019 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants