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

Routing specs and cleanup #1477

Merged
merged 12 commits into from Sep 16, 2012
Merged

Routing specs and cleanup #1477

merged 12 commits into from Sep 16, 2012

Conversation

rspeicher
Copy link
Contributor

First, this adds routing specs. Why? Because I was going to modify the
routes and wanted to make sure the current ones still worked after my
changes.

Then I made a few changes:

  • Adds an AdminController base controller for the other Admin
    controllers to subclass. It currently handles setting the layout and
    authorizing an admin and just removes some duplication from those
    controllers.
  • Style changes in routes.rb, mostly 1.9 Hash syntax and alignment and
    spacing.
  • Removes a few actions that are no longer used from various
    controllers.
  • Removes unused actions from various routes ("logs" had all the default
    actions but only used "show", for instance.)
  • Removes the 'team' action from the Projects controller and instead
    uses the 'index' action in TeamMembers.

It still works, and it cleans up the output of `rake routes`.
Also cleans up some alignment and removes unnecessary "to: " arguments
Handles stuff that's shared across admin controllers.
We were already calling `authorize_admin_issue!` in a before filter with
the same permission checks, so this deleted check wasn't actually doing
anything.
Uses TeamMembers#index instead, to be more RESTful
@riyad
Copy link
Contributor

riyad commented Sep 16, 2012

Wouldn't it be better to split the routing specs into one file per controller?

@rspeicher
Copy link
Contributor Author

Yeah, probably. I can do that if we want.

@rspeicher
Copy link
Contributor Author

I dunno, while there's definitely a downside to a file that big, I think there's definitely a positive side to seeing them all in one file; so many of them are related since they belong to Projects.

There's probably a better way to split it up than just one file per controller. I'll think on it some more.

@riyad
Copy link
Contributor

riyad commented Sep 16, 2012

I'll trust your jundgement. ;)

@rspeicher
Copy link
Contributor Author

That split them up a bit.

dzaporozhets added a commit that referenced this pull request Sep 16, 2012
@dzaporozhets dzaporozhets merged commit 26fb55c into gitlabhq:master Sep 16, 2012
@dzaporozhets
Copy link
Member

@tsigo great cleanup. You make my dreams :)

maxlazio added a commit that referenced this pull request Sep 16, 2014
API MR ordering

Fixes #1477

See merge request !1076
dzaporozhets pushed a commit that referenced this pull request Apr 23, 2015
dzaporozhets added a commit that referenced this pull request Apr 23, 2015
Use `search_text_nodes` helper in our custom filters

Closes #1477

See merge request !561
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants