test(message-templates): review fixes left out of #140 squash (EVO-1716)

[daniloleonecarneiro]

Summary

Follow-up a #140. As correções da adversarial review foram commitadas depois que #140 já tinha sido mergeado (squash levou só o primeiro commit, head f7b4440), então ficaram fora do develop. Este PR traz exatamente esse delta.

• Authz happy-path com user real: todo verde anterior rodava via service-token, que bypassa os dois gates (require_permissions + Pundit). Autentica um user e stuba check_user_permission por key para travar o contrato do resource novo (create → 201, index → 200, negado → 403).
• Cobertura de sync restaurada no sync_with_whatsapp_cloud (preservado, não movido): 422 para canal não-WhatsApp-Cloud e 404 para template id inexistente.
• MessageTemplatePolicy: removida a classe Scope (dead code — policy_scope nunca é chamado sobre MessageTemplate; o controller filtra via base_scope).
• 404 enganoso: quando o caller manda channel_id puro, a mensagem agora nomeia o alvo certo ("Channel not found") em vez de "Inbox not found".

Test plan

• message_templates_spec.rb + message_templates_service_token_spec.rb: 26 examples, 0 failures.
• Rubocop limpo nos arquivos de produção (controller + policy).

Summary by Sourcery

Align message template authorization, sync behavior, and error responses with the intended contract and restore missing test coverage from a previous squash merge.

Bug Fixes:

• Return 404 errors that correctly distinguish between missing inboxes and channels when resolving message template targets.
• Ensure sync_with_whatsapp_cloud returns 422 for non-WhatsApp-Cloud channels and 404 for unknown template IDs instead of silently proceeding.

Enhancements:

• Remove the unused Pundit Scope from MessageTemplatePolicy since templates are not account-scoped and the controller does not call policy_scope.
• Expand authorization tests to exercise the real-user path using EvoAuthService permission checks for message_templates.create and message_templates.read.

Tests:

• Add request specs covering real-user authorization for message template create and index endpoints, including allowed and forbidden scenarios.
• Add request specs verifying sync_with_whatsapp_cloud behavior for non-WhatsApp-Cloud channels and unknown template IDs.

[sourcery-ai]

Reviewer's Guide

Adds real-user authorization tests for message template endpoints, restores sync_with_whatsapp_cloud coverage and behavior, removes unused MessageTemplatePolicy::Scope, and fixes a misleading 404 error message when channel_id is used instead of inbox_id.

Sequence diagram for render_inbox_not_found 404 message selection

sequenceDiagram
  participant Caller
  participant MessageTemplatesController

  Caller->>MessageTemplatesController: render_inbox_not_found
  alt [params[:inbox_id].present?]
    MessageTemplatesController->>MessageTemplatesController: set target = Inbox
  else [no inbox_id param]
    MessageTemplatesController->>MessageTemplatesController: set target = Channel
  end
  MessageTemplatesController->>Caller: error_response(ApiErrorCodes::RESOURCE_NOT_FOUND, "<target> not found")

Loading

Flow diagram for sync_with_whatsapp_cloud response codes

flowchart TD
  A["sync_with_whatsapp_cloud called"] --> B{"channel is WhatsApp Cloud?"}
  B -- no --> C["respond 422 Unprocessable Entity"]
  B -- yes --> D{"template id exists?"}
  D -- no --> E["respond 404 Not Found"]
  D -- yes --> F["perform sync and respond 200"]

Loading

File-Level Changes

Change	Details	Files
Strengthen authorization tests to exercise real-user paths and permission keys for message_templates.*.	Switch authorization specs from a generic forbidden user to a concrete agent user authenticated through the controller Stub EvoAuthService.check_user_permission per permission key (message_templates.create/read) to assert allowed create (201) and index (200) flows Keep and clarify the forbidden path spec by stubbing check_user_permission to return false and expecting 403	spec/requests/api/v1/message_templates_spec.rb
Expand sync_with_whatsapp_cloud request specs to cover non-WhatsApp-Cloud channels and unknown template IDs.	Add spec ensuring sync_with_whatsapp_cloud returns 422 and does not enqueue the job for non-WhatsApp-Cloud channels Add spec ensuring sync_with_whatsapp_cloud returns 404 and does not enqueue the job when the template id is unknown	spec/requests/api/v1/message_templates_spec.rb
Remove dead Pundit Scope logic from MessageTemplatePolicy.	Delete MessageTemplatePolicy::Scope implementation since templates are not policy_scoped and the controller filters via base_scope instead	app/policies/message_template_policy.rb
Fix misleading 404 error messaging when resolving targets by inbox_id or channel_id in message_templates controller.	Update render_inbox_not_found to derive the target label (Inbox vs Channel) based on whether inbox_id is present in params Return a more accurate 'Channel not found' message when only channel_id is provided and lookup fails	app/controllers/api/v1/message_templates_controller.rb

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've left some high level feedback:

• The new authorization specs stub EvoAuthService#check_user_permission inline in each example; consider extracting a small helper or shared context to centralize the key-based stubbing and reduce duplication/maintenance overhead across these tests.
• In render_inbox_not_found, the selection of Inbox vs Channel based solely on the presence of inbox_id could become misleading if both params are ever present or lookups change; it may be more robust to branch on which lookup actually failed or on a more explicit flag from the calling code.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The new authorization specs stub `EvoAuthService#check_user_permission` inline in each example; consider extracting a small helper or shared context to centralize the key-based stubbing and reduce duplication/maintenance overhead across these tests.
- In `render_inbox_not_found`, the selection of `Inbox` vs `Channel` based solely on the presence of `inbox_id` could become misleading if both params are ever present or lookups change; it may be more robust to branch on which lookup actually failed or on a more explicit flag from the calling code.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[dpaes]

Reviewed — approved ✅

Verified the follow-up delta against the real code on danilocarneiro/evo-1716-review-fixes. CRM-community runs no RSpec in CI, so I traced each spec's logic by hand rather than trusting the self-reported green.

• Real-user authz happy-path — traced the full chain: require_permissions → check_<action>_permission! → check_permission!(key) → EvoAuthService#check_user_permission(user_id, key) (2-arg signature, matches .with(anything, 'message_templates.create')). Exactly one call per request with the action's key; Pundit goes through User#has_permission? (always true on community), not EvoAuthService, so the strict .with stub can't trip "unexpected arguments". 201 / 200 / 403 are all sound.
• Sync coverage restored — sync_template_with_whatsapp_cloud does a global MessageTemplate.find → unknown uuid raises RecordNotFound → 404; the Channel::Whatsapp && provider == 'whatsapp_cloud' guard rejects the evolution channel → 422 (the cross-channel template is still found globally, hence 422 and not 404 — the fixtures line up).
• MessageTemplatePolicy::Scope removal — confirmed zero callers of policy_scope(MessageTemplate) anywhere; the only policy_scope reference is the generic Pundit.policy_scope! in application_policy.rb. The controller authorizes instances + filters via base_scope, so the Scope class was dead code. Safe to drop.
• channel_id surface removal — gone from the controller entirely; base_scope / channel_bound? / resolve_channel collapse to the single inbox_id axis. Confirmed no consumer ever sent channel_id (FE global service sends neither filter, channel-bound sends inbox_id; evo-flow reads the table directly), so the "Inbox not found" 404 is now always accurate.

Non-blocking nit: the PR description's "404 → 'Channel not found'" bullet describes commit 1's intermediate state — commit 2 dropped channel_id entirely, so the net diff only keeps the always-correct "Inbox not found". The code itself is coherent.

The one open item from the card (show / index global without channel scope) is an accepted by-design consequence of the account-less, instance-wide catalog — not a blocker.