Skip to content

Sinatra::Namespace does not prefix redirect() nor url() #27

Open
metakeule opened this Issue Dec 17, 2011 · 7 comments

4 participants

@metakeule

I would expect the redirects to be prefixed like the routes.

namespace '/admin' do
    get '/a' do
        redirect '/b'  # is not redirecting to '/admin/b' but to '/b'
    end

    get '/a' do
        redirect to('/b')  # is not redirecting to '/admin/b' but to '/b'
    end
 end

:prefixed_redirects raises an error

namespace '/admin' do
    enable :prefixed_redirects
    get '/somewhere'
 end

 # => may not set prefixed_redirects (ArgumentError)

I don't know where this should be fixed

module Sinatra::Namespace::NamespacedMethods
    prefixed :to
end

didn't help either.

@metakeule

ok, I digged into it. It seems the following should do the trick:

module Sinatra::Namespace::InstanceMethods
  def redirect(uri, *args)
    prefix = @namespace.instance_variable_get("@pattern")
    super("#{prefix}#{uri}", *args)
  end
end

But I guess you would organize it some other way to fit into your code.

@metakeule

same thing for url()

module Sinatra::Namespace::InstanceMethods
  def url(uri, *args)
    prefix = @namespace.instance_variable_get("@pattern")
    super("#{prefix}#{uri}", *args)
  end
end
@metakeule

ok, so it seems that if we use redirect to() we could have the choice:

module Sinatra::Namespace
  module InstanceMethods
    def url(uri, *args)
      super("#{@namespace.pattern}#{uri}", *args)
    end

    def to(uri, *args)
      super("#{@namespace.pattern}#{uri}", *args)
    end
  end

  module NamespacedMethods
    def pattern
      @pattern
    end
  end
end

and then we could use it like this:

class App < Sinatra::Base
  register Sinatra::Namespace

  namespace '/admin' do
    get '/a' do
      "A"
    end

    get '/c' do
      redirect to('/a')   # goes to /admin/a
    end
  end

  namespace  '/badmin' do
    get '/b' do
      "B: #{url('/d')}"
    end

    get '/c' do
      redirect '/admin/a'  # goes to /admin/a
    end
  end
end
@TrevorBramble
Sinatra member

Thanks for this! I'll see about adding some specs to cover the cases you've identified and adding support for them. Any other weirdness you've noticed with using namespaces? (Excluding what's covered in #28.)

@metakeule

@TrevorBramble please have a look at

https://github.com/metakeule/sinatra-namespace-fix

for fixes, tests and inspirations. I find the mount concept interesting even if it might conflict with some routing options (patterns, :agent), see my ugly __replace_params_in_pattern hack

I had a hard time not being allowed to set options from within a namespace. I think a clean concept would be needed.

What about this:

  • settings from the outer environment are merged into the current namespace
  • settings set in the namespaced environment are only available in the namespaced environment and overwrite settings from the outer env with the same name

there were also problems with using a Sinatra::Extension in a namespace. But it was too weird and I gave up on Sinatra::Extension

@zzak
Sinatra member
zzak commented May 23, 2015

This needs an (actual) failing test and patch before it can be fixed.

That said, it seems the following comments could lead to a potential patch:

@stjhimy
stjhimy commented Dec 4, 2015

@zzak I added a method called "redirect_to" in the #180 that allows redirects within the namespace.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.