Guard /internal_note quick action on a persisted target

What does this MR do and why?

A Sentry alert on gitlab.com (gprd) reports NoMethodError: undefined method 'widget_classes' for nil, raised at app/models/issue.rb:945 in Issue#has_widget?. The failing request is GET /-/autocomplete_sources/commands.

The call chain from the stacktrace:

  1. Projects::AutocompleteSourcesController#commands
  2. Projects::AutocompleteService#commands
  3. QuickActions::InterpretService#available_commands
  4. the /internal_note quick action condition (lib/gitlab/quick_actions/issuable_actions.rb:266)
  5. current_user.can?(:mark_note_as_internal, quick_action_target)
  6. IssuePolicy's notes_widget_enabled condition (app/policies/issue_policy.rb:37)
  7. Issue#has_widget?(:notes)
  8. work_item_type.widget_classes(...)

When the autocomplete request has no type_id, QuickActions::TargetService builds a non-persisted issue with container.issues.build. work_item_type is only assigned by the ensure_work_item_type before_validation callback, so on an unvalidated new record it is still nil, and has_widget? raises.

This surfaced because the /internal_note quick action was added in !243383 (merged). Unlike the other quick action conditions in lib/gitlab/quick_actions/, its condition did not guard on quick_action_target.persisted?, so it was the only one evaluating a notes-widget policy against an unsaved issue.

The fix adds a quick_action_target.persisted? guard to the /internal_note condition, matching the convention already used by the other conditions in lib/gitlab/quick_actions/issuable_actions.rb, issue_actions.rb, and issue_and_merge_request_actions.rb.

Issue#has_widget? is deliberately left unchanged. Making it fall back to a default work item type would let it answer true for widgets based on a type the record does not actually have yet. Keeping has_widget? strict and guarding at the call site preserves that.

Behavior change: /internal_note no longer appears in the autocomplete command list for an issuable that has not been created yet. That matches how the other quick actions that need a saved record behave, and the action was already a no-op there, since it applies to a note on an existing noteable.

Tests: a when the target is not persisted context was added to the /internal_note block in spec/services/quick_actions/interpret_service_spec.rb. It asserts the command is absent from available_commands and that executing /internal_note produces no updates. Both examples fail without the fix. The existing /internal_note specs still pass (26 examples, 0 failures).

No migration, no feature flag, no UI change, no documentation change.

This change and this merge request description were created with the help of an AI agent. The author reviewed the change.

References

Screenshots or screen recordings

Not applicable. This change has no user interface component. The only visible difference is that /internal_note is no longer listed in the autocomplete commands for an issuable that does not exist yet.

How to set up and validate locally

  1. Check out the branch and run the added specs:

    bundle exec rspec spec/services/quick_actions/interpret_service_spec.rb -e '/internal_note'
  2. To confirm the original error, stash the change to lib/gitlab/quick_actions/issuable_actions.rb and run the same command. The two examples in the when the target is not persisted context fail with NoMethodError: undefined method 'widget_classes' for nil.

  3. To check the request path, request the autocomplete commands for a project without a type_id parameter, for example http://127.0.0.1:3000/<group>/<project>/-/autocomplete_sources/commands. It returns a command list rather than a 500 response, and the list does not include internal_note.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Mario Celi

Merge request reports

Loading
Loading