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

Split api_controller actions into their own controllers #2160

Merged
merged 6 commits into from
Feb 28, 2019

Conversation

gravitystorm
Copy link
Collaborator

I've been planning on a refactoring of api-related controllers for a while. For further background reading, see gravitystorm#27.

I want to refactor the api-related controllers to inherit from a common controller (in the same way that regular controllers normally inherit from ApplicationController). The only convention that I found has this base controller named as "ApiController", but we're already using that name. So I thought about renaming our current ApiController, to free up the name for reuse, but I couldn't figure out a good descriptive name for the actions in the current ApiController. And then I realised that's because they are mostly unrelated to each other, and so I could split them into separate controllers instead.

So that's what this PR does - it splits the ApiController into separate controllers for each unrelated action.

I've chosen to put them in an "api" namespace (aka subdirectory) since I plan to do this for the rest of the api-related controllers. It is also the end-goal for other work that I'm doing on supporting multiple API versions (see gravitystorm#28), so creating the new controllers in the subdirectory should avoid further renaming.

@tomhughes
Copy link
Member

One (genuine) test failure:

ExportControllerTest#test_finish_osm:
ActionController::UrlGenerationError: No route matches {:action=>"map", :bbox=>#<BoundingBox:0x0000556f20be2028 @min_lon=0.0, @min_lat=50.0, @max_lon=1.0, @max_lat=51.0>, :controller=>"api", :format=>"osm"}
    app/controllers/export_controller.rb:16:in `finish'
    test/controllers/export_controller_test.rb:20:in `test_finish_osm'


bin/rails test test/controllers/export_controller_test.rb:19

@gravitystorm
Copy link
Collaborator Author

Oops. I'd already fixed that when working on #2161 but had forgotten to update this branch after some rebasing. I've fixed this now with a force-push.

@tomhughes tomhughes merged commit f4e2990 into openstreetmap:master Feb 28, 2019
@gravitystorm gravitystorm deleted the api_namespace branch June 13, 2019 07:43
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.

2 participants