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

Filter and form do not honor scope_to :current_user #70

Closed
bigcurl opened this issue May 20, 2011 · 43 comments
Closed

Filter and form do not honor scope_to :current_user #70

bigcurl opened this issue May 20, 2011 · 43 comments
Assignees
Labels
Milestone

Comments

@bigcurl
Copy link

bigcurl commented May 20, 2011

scope_to :current_user is set in ActiveAdmin.register

If I set a filter for an association to filter :folder, :as => :select it will call Folder.all.

If I try to set filter :folder, :as => :select, :collection => current_user.folders.all rails fails with a current_user not found error.

if scope is set the filter method should scope the association to the current_users objects.

@gregbell
Copy link
Contributor

You can set the :collection via a proc which will be called within the view:

  filter :folder, :as => :select, :collection => proc{ current_user.folders.all }

That work for you?

@adie
Copy link

adie commented May 25, 2011

I have the same issue. With :collection => proc { current_user.folders.all } I'm getting an error

undefined local variable or method `current_user' for #<ActiveAdmin::DSL:0x183c6fdc>

@emzeq
Copy link
Contributor

emzeq commented May 26, 2011

Are you using ActiveAdmin's default user naming? If so it's 'current_admin_user', not 'current_user'.

@adie
Copy link

adie commented May 27, 2011

No, I have exactly current_user. And I have scope_to :current_user which works well.

@bigcurl
Copy link
Author

bigcurl commented May 28, 2011

Like adie I tried a lot of combinations (also with proc) but the problem is still there

@sugui
Copy link

sugui commented May 30, 2011

same issue here.

At filter and form DSL block no access to current_admin_user, but scope_to :current_admin_user work fine.

@asancio
Copy link
Contributor

asancio commented Jun 5, 2011

Me three... i can't access current_admin_user from the form block.

@tmilewski
Copy link

Same form issues here.

@samitheberber
Copy link

Same here

@tjogin
Copy link

tjogin commented Oct 5, 2011

Not sure if this can be considered a proper solution, but until that shows up you can use f.template.current_admin_user within form blocks, like so:

form do |f|
   if f.template.current_admin_user.has_permission? :something
       #woot!
   end
   f.inputs
   f.buttons
end

Still haven't figured out how to access current_admin_user from within an ActiveAdmin::DSL though. Anyone else?

@holden
Copy link

holden commented Oct 7, 2011

I have the same issue. filter :space, :collection => proc { current_user.spaces }

It is correctly set: config.current_user_method = :current_user

also tried:

filter :space, :collection => proc { controller.current_user.spaces }

scoped_to :current_user works perfectly 2 lines above it ;-(

@mhuggins
Copy link

mhuggins commented Nov 2, 2011

I'm having the same issue with trying to set a default value on a hidden form element.

Current user config:

config.current_user_method = :current_profile

Formtastic input:

f.input :profile_id, :value => proc { current_profile.id }, :as => :hidden

Resultant HTML:

<input id="advertisement_profile_id" name="advertisement[profile_id]" type="hidden"
       value="#&lt;Proc:0x00000101aceb88@/Users/mhuggins/Sites/dating/app/admin/advertisements.rb:55&gt;" />

@cgunnels
Copy link

can't believe this hasn't been fixed yet. I am having the same issue. @gregbell you said it should work, but doesn't. What makes you think it should work? Have you got it to work before?

@tmilewski
Copy link

Whoa, slow it down there @cgunnels!

This is open source and you are not paying for it. @gregbell (and others) have put in a lot of work to ensure that ActiveAdmin works for the majority of its users. I'm quite thankful for that.

If it's as big of an issue as you're making it out to be then I think it's time to get your hands dirty and contribute some code.

@cgunnels
Copy link

didn't mean any ill will, came across wrong when you read it.

@bdastous
Copy link

Actually, it came out wrong when you wrote it.

@cgunnels
Copy link

So I hacked something together...for the filters in the register block you can do something like this:
filter :columnname, :as => :select, :collection => proc { Model.all(:conditions => {:user_id => Thread.current[:user].id}).each { ... } }

In the applicationcontroller.rb set a before filter like so, before_filter :set_current_user and a method like so,
def set_current_user Thread.current[:user] = current_user if current_user end

If anyone has a better way, I'd love to hear. I think this is a hack and would like a better way to go about using the current user at the scope of ActiveAdmin::DSL

@gabpaladino
Copy link

hello guys

I had the same problem and I am a newbie with Ruby. but I understand that the problem was different from scope_to why the filter is processed when the collection is defined.

so I made a very ugly monkey patch that solved my problem by processing the collection when the filter will be displayed .. then it is in an instance of the object and not in static class.

I set the filter as

filter: client, :label => t(:client), :scope => lambda{ current_user.clients }

and the patch below changes the scope to be the collection when the view is generated

if someone has a better solution .. and can send a pull request I appreciate it. unfortunately I have no knowledge for this.

module ActiveAdmin
  class ResourceController < ::InheritedResources::Base
    module Filters
      protected
      def filters_config
        filters = self.class.filters_config
        new_filters = []
        filters.each do |options|
          options = options.merge(:collection => instance_exec(&options[:scope])) if options[:scope]
          new_filters << options
        end
        new_filters
      end
    end
  end
end

@RobertLowe
Copy link

+1, gabpaladino seems to be working for me

@diasjorge
Copy link
Contributor

+1 gabpaladino, nasty but it works, although I changed the first 3 lines to this:

ActiveAdmin::ResourceController::Filters.module_eval do

@pcreux
Copy link
Contributor

pcreux commented Dec 21, 2011

Hey everybody!

Could you confirm that @samvincent's pull request #847 fixes the issue you were encountering?

@gabpaladino
Copy link

Hi @samvincent and @pcreux,

Our problem is different. I did not know this counts bug, our need scope_to is applied to the values ​​of the filter.

For example, we have a Client model, and user has many clients. So if we have a filter select for clients all they are displayed to user, not only that have relationship with current_user.

The default :collection option of filters does not accept current_user because is processed as config before the instances of objects.

@diasjorge
Copy link
Contributor

I have the same use case than @gabpaladino, it's the list of options for the filter that depends on the current_user

@gabpaladino
Copy link

Hey Guys, i am now with the same problem on new/edit forms! =/

Any sugestion?! I did not get an monkey patch for it!

@jancel
Copy link
Contributor

jancel commented Jan 24, 2012

Pull request 847 had interesting behavior on my application. Some of the "Show" pages were no longer working, though I'm still a little new to this project, attribute that started erroring was (:body). At any rate, it may address the counts (as I couldn't validate the bug via recreation). I'm really interested in seeing that scope_to is applied to relationships (has_one) dropdown specifically in forms, and even filters would be nice, though I guess I can remove that.

Shall I open this a new defect and provide better details and an empty project showing this behavior. (I'm tracking master currently)

@jancel
Copy link
Contributor

jancel commented Jan 25, 2012

I created a base project with a bare minimum config to show this issue. You can see it here, https://github.com/jancel/scoping_active_admin

I'd like to go farther, perhaps creating the tests to recreate this issue, or even a fix. But just knowing that this is in fact not the intended behavior, would help me go farther.

@rayzor
Copy link

rayzor commented Jan 30, 2012

Show Stopper: Bug 70

This BUG 70 is a show stopper for my multi tenant SaaS App.

All models (filters and association selects) must scope to current_account.

Apologies but I have insufficient knowledge of AA to assist.

Any progress on resolving this Bug 70 much appreciated.

Currently using alternative to Active Admin.

@bruno
Copy link

bruno commented Feb 3, 2012

Unfortunately the pull request #847 is not working, we are having both issues mentioned on here, the issue around :scope_to not being respected by the filters and forms as well as the count issue on the resource scopes.

We have a scope on our model scope :pending, where(:state => :pending) which we map in active admin with scope :pending however with this pull request applied to Active Admin 0.3.4 I am getting

undefined method 'pending' for #<Array:0x007f85f1ef5cd0>
lib/active_admin_views_pages_base.rb:10:in 'block (2 levels) in build_page'
lib/active_admin_views_pages_base.rb:7:in 'block in build_page'
lib/active_admin_views_pages_base.rb:6:in 'build_page'

And when I tried to upgrade to 0.4.0, all the filters were back to their old behaviour (previous to using the patch idea from @gabpaladino )

@bruno
Copy link

bruno commented Feb 3, 2012

As an interim measure I am putting the following in my initializer:

ActiveAdmin::Views::Scopes.class_eval do
  protected

  def build_scope(scope)
    span :class => classes_for_scope(scope) do
      begin
        scope_name = I18n.t!("active_admin.scopes.#{scope.scope_method}")
      rescue I18n::MissingTranslationData
        scope_name = scope.name
      end

      if current_scope?(scope)
        em(scope_name)
      else
        a(scope_name, :href => url_for(params.merge(:scope => scope.id, :page => 1)))
      end
    end
  end
end

As you can see, I have removed the following:

text_node(" ")
scope_count(scope)
text_node(" ")

Just as to avoid confusing our users when the counts are not respecting the scope_to

@ghost ghost assigned gregbell Feb 3, 2012
@bruno
Copy link

bruno commented Feb 3, 2012

Of course, my solution sucks and I'm not yet as familiar with the inner workings of ActiveAdmin as to fix this problem. I'll keep on looking though.

@pcreux
Copy link
Contributor

pcreux commented Feb 6, 2012

For the form, I confirm that you can do:

form do |f|
  f.inputs do
    f.input :group_id, :as => :select, :collection => f.template.controller.current_client.groups
  end
end

@jcabas
Copy link

jcabas commented Feb 8, 2012

Thanks @pcreux, it works for me!

gregbell added a commit that referenced this issue Feb 18, 2012
When defining a filter, you can pass a collection that will be
rendered within the context of the view. For example:

ActiveAdmin.register Post do
  filter :some_attr, :as => :select,
                     :collection => proc{ current_admin_user.some_collection }
end
gregbell added a commit that referenced this issue Feb 18, 2012
You can now do:

    ActiveAdmin.register Post do
      form do
        if current_admin_user.super_admin?
          f.inputs "For Super Admins", :super_admin_thing
        end
        f.buttons
      end
    end
@gregbell
Copy link
Contributor

Listed commits above should fix all the issues present here.

@gabpaladino
Copy link

uow it's good! :D thanks @gregbell, i will test

@topperge
Copy link

Has any one actually tested this? I'm still having the same issue with the filters on my index.

@jancel
Copy link
Contributor

jancel commented Mar 17, 2012

I've done it successfully on versions 0.4.0+

On Sat, Mar 17, 2012 at 11:21 AM, Matt Topper <
[email protected]

wrote:

Has any one actually tested this? I'm still having the same issue with the
filters on my index.


Reply to this email directly or view it on GitHub:
#70 (comment)

@topperge
Copy link

Ugh, must be something I'm doing then, putting something like
filter :user
Just gives me a list of all the users in the system and shows the references to the users (#<User.0x00) not their email address or any nice name.

filter :user, :as => :select, :collection => User.where(:company_id => current_user.company_id).map(&:email)
Gets me: undefined local variable or method `current_user' for #ActiveAdmin::ResourceDSL:0x007fc054930dc0

If I hard code the company_id I get the results I expect, but that isn't going to work obviously
i.e. filter :user, :as => :select, :collection => User.where(:company_id => 4).map(&:email)

@jancel
Copy link
Contributor

jancel commented Mar 17, 2012

Ok. Here is what I have in my collection...

:collection => f.template.controller.current_user.signatures

i'd imagine that you want to do...

:collection => f.template.controller.current_user.company_id ... and then
your stuff. You should have access to it there.

On Sat, Mar 17, 2012 at 12:15 PM, Matt Topper <
[email protected]

wrote:

Ugh, must be something I'm doing then, putting something like
filter :user
Just gives me a list of all the users in the system and shows the
references to the users (#<User.0x00) not their email address or any nice
name.

filter :user, :as => :select, :collection => User.where(:company_id =>
current_user.company_id).map(&:email)
Gets me: undefined local variable or method `current_user' for
#ActiveAdmin::ResourceDSL:0x007fc054930dc0

If I hard code the company_id I get the results I expect, but that isn't
going to work obviously
i.e. filter :user, :as => :select, :collection => User.where(:company_id
=> 4).map(&:email)


Reply to this email directly or view it on GitHub:
#70 (comment)

@topperge
Copy link

@jancel do you happen to have the project you're referring to on github?

I'm getting an " undefined local variable or method 'f' " with your example but could probably figure it out with a code example

@jancel
Copy link
Contributor

jancel commented Mar 17, 2012

Sorry, I don't. Here's my form block though:

form do |f|
f.inputs "Email Templates" do
...
f.input :signature, :hint => "Blank for default signature",
:collection => f.template.controller.current_user.signatures
...
end
f.buttons
end

On Sat, Mar 17, 2012 at 2:45 PM, Matt Topper <
[email protected]

wrote:

@jancel do you happen to have the project you're referring to on github?

I'm getting an " undefined local variable or method 'f' " with your
example but could probably figure it out with a code example


Reply to this email directly or view it on GitHub:
#70 (comment)

@topperge
Copy link

@jancel Mine works on a form as well, its when I'm using the collection as part of a filter like on a show or index page that I'm getting the error

@jancel
Copy link
Contributor

jancel commented Mar 17, 2012

Ok, may want to move the question over to google groups where we can get more visibility. I'm not sure I've seen a need for this yet, so I'm unfamiliar with using select drop downs in index/show methods.

@cgunnels
Copy link

@topperge
filter :user
Just gives me a list of all the users in the system and shows the references to the users (#<User.0x00) not their email address or any nice name.

This should be fixed by placing this in the user model

def display_name
  self.email_address
end

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

No branches or pull requests