8000 skip registering ActiveAdmin::Comment if namespace.comments is false by FeLvi-zzz · Pull Request #8710 · activeadmin/activeadmin · GitHub
[go: up one dir, main page]
Skip to content

skip registering ActiveAdmin::Comment if namespace.comments is false #8710

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
Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

skip registering ActiveAdmin::Comment if namespace.comments is false #8710

wants to merge 2 commits into from

Conversation

FeLvi-zzz
Copy link
@FeLvi-zzz 8000 FeLvi-zzz commented May 6, 2025 •
edited
Loading

resolve #5117
resolve #5311

background

we disable comments, but routes are still drawn.

what to do

  • skip registering ActiveAdmin::Comment if namespace.comments is disabled
    • this prevents drawing routes for comments
    • something for model (scope, before_save) are unneeded as well

@FeLvi-zzz FeLvi-zzz marked this pull request as draft May 6, 2025 05:18
@FeLvi-zzz FeLvi-zzz marked this pull request as ready for review May 6, 2025 05:24
@tagliala
Copy link
Contributor
tagliala commented May 6, 2025

Hello,

thanks for this PR

Any chance to add a spec to cover this scenario?

@FeLvi-zzz
Copy link
Author

@tagliala
Hello,

yes, but which file should I add a spec into?
I couldn't find an appropriate one, it looks like there is nothing testing like namespace.register ActiveAdmin::Comment.
I would appreciate if you could let me know where to add.

@tagliala
Copy link
Contributor
tagliala commented May 6, 2025

This looks like a routes spec. There is one under resource

Asking @javierjulio for confirmation

@javierjulio
Copy link
Member

@tagliala I'm not sure actually. @FeLvi-zzz let's give that a try please and we'll go from there.

@FeLvi-zzz
Copy link
Author

@tagliala @javierjulio
I tried to add a spec in spec/unit/comments_spec.rb.
What do you two think about this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
type enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Bug: comment routes appear even when comments are disabled
3 participants
@FeLvi-zzz @tagliala @javierjulio
0