8000 Bug: comment routes appear even when comments are disabled · Issue #5117 · activeadmin/activeadmin · GitHub
[go: up one dir, main page]
Skip to content

Bug: comment routes appear even when comments are disabled #5117

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.

Sign up for GitHub

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

Jump to bottom
Closed
shaldengeki opened this issue Aug 2, 2017 · 5 comments · May be fixed by #8710
Closed

Bug: comment routes appear even when comments are disabled #5117

shaldengeki opened this issue Aug 2, 2017 · 5 comments · May be fixed by #8710
Labels
bug

Comments

@shaldengeki
Copy link
shaldengeki commented Aug 2, 2017 •
edited
Loading

Expected behavior

When disabling comments through config.comments = false, the comments-related routes should not be registered.

Actual behavior

Comments-related routes (e.g. /admin/comments) appear in routes even if comments are disabled in configuration.

How to reproduce

  1. Add config.comments = false to config/initializers/active_admin.rb
  2. Add ActiveAdmin.routes(self) to config/routes.rb
  3. Run rake routes | grep comments

Comment-related routes appear.

(cc @alainbloch)
@Fivell
Copy link
Member
Fivell commented Aug 3, 2017

looks like this can be handled here
https://github.com/activeadmin/activeadmin/blob/master/lib/active_admin/router.rb#L81

@Fivell Fivell added the bug label Aug 3, 2017
kavithaRajagopalan added a commit to kavithaRajagopalan/activeadmin that referenced this issue Aug 4, 2017
@kavithaRajagopalan
[bugFix] Remove comment routes when disabled activeadmin#5117
443b37d
@ghilead
Copy link
ghilead commented Nov 8, 2017

Perhaps registering the Comment resource should be avoided altogether for namespaces that disable it?

activeadmin/lib/active_admin/orm/active_record/comments.rb

Line 23 in 4b49319

namespace.register ActiveAdmin::Comment, as: namespace.comments_registration_name do

@ghilead ghilead mentioned this issue Nov 13, 2017
Closed
@kavithaRajagopalan kavithaRajagopalan mentioned this issue Nov 24, 2017
Closed
@varyonic varyonic mentioned this issue Dec 5, 2017
Closed
@vanegomez
Copy link

I have the same issue. I'm a bit confused on how you were able to solve it. Thank you

@ghilead
Copy link
ghilead commented Dec 5, 2017 •
edited
Loading

@vanegomez I ended up doing a workaround: created an empty custom resource page for ActiveAdmin::Comment. The routes are still there, but the page is blank.

ActiveAdmin.register ActiveAdmin::Comment, as: 'Comment' do
  index do
  end

  show do
  end
....
end

I traced back to where this was introduced and tried to understand the rationale, I don't think the fix as I proposed above is going to be effective. I tried to re-ignite the conversation here:

#3695

Hope this helps!

@varyonic varyonic mentioned this issue Jan 8, 2018
Closed
@mef79
Copy link
mef79 commented Feb 28, 2020 •
edited
Loading

Found a workaround that prevents the apps routes from being generated that works for my purposes. It's not ideal, but I really wanted them gone.

# in config/intializers/active_admin.rb
# or anywhere else after ActiveAdmin is loaded but before ActiveAdmin.routes gets called

module NamespaceWithoutComments
  def register(resource_class, options = {}, &block)
    super unless resource_class == ActiveAdmin::Comment
  end
end

module ActiveAdmin
  class Namespace
    prepend NamespaceWithoutComments
  end
end

Consider before using:

  • future compatibility concerns - if any of the active admin code that this references changes and you update, this snippet might cause problems in your app
  • It's at odds with the maintainers' stated intention of allowing comments to be turned on for an individual resource
  • actually, it invalidates the config.comments setting - if you later want to have comments, this needs to go

@javierjulio javierjulio closed this as not planned Won't fix, can't repro, duplicate, stale Mar 12, 2023
@FeLvi-zzz FeLvi-zzz mentioned this issue May 6, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

skip registering ActiveAdmin::Comment if namespace.comments is false FeLvi-zzz/activeadmin
6 participants
@Fivell @javierjulio @ghilead @shaldengeki @mef79 @vanegomez
0