Hello,

thanks for this PR

Any chance to add a spec to cover this scenario?

@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.

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

Asking @javierjulio for confirmation

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

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

@mgrunberg @tagliala what do you think of the this approach and the tests? I think it would help to have as if the feature is not used then the routes should not exist and this seems to handle the case where it's only enabled for a specific namespace or resource. Any improvements or concerns?

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.08%. Comparing base (f79eabd) to head (d81db47).

@@           Coverage Diff           @@
##           master    #8710   +/-   ##
=======================================
  Coverage   99.08%   99.08%           
=======================================
  Files         139      139           
  Lines        4046     4047    +1     
=======================================
+ Hits         4009     4010    +1     
  Misses         37       37           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Sorry, I'm not familiar with the comments implementation.

I can see there is a global switch and a per-resource switch, so this is why it is not checking for config.comments = false.

I also assume that the global config can be override at namespace / model level, so you can have comments false at global level, except for models or namespaces which explicitly set comments to true

I do not have suggestions for the specs

I'm not very familiar with the comments code either, but in a quick review, I'm ok with the approach and specs.

I'm just wondering if it's possible to reduce the usage of stubs/doubles. It's not a strong opinion. It's just that the spec has Namespace & Resource creation examples. I'm aware that we are trying to call a hook and the setup for that could not be easy.

I reduced excessive stubs (thanks to @mgrunberg !).

Could anyone run tests?

Oops, tests which I created look flaky...
I'll fix them 🙇

Hello, this is still failing. Can you also please rabse on master?