Skip to content

Design view: relax permissions for updateImageDiffNote mutation

Problem to solve

We scope updateImageDiffNote mutation to users with admin_note permission for individual notes.

On the frontend at GitLab, we currently only use this mutation to update the notes position.

The current behaviour is that most users are unable to move another user's note's position. The desired behaviour is that all users of design management are able to move all pins (even those that are not their own).

Intended users

All users of design management

Backend Proposal

  1. Add a new permission on Note called :reposition_note. This would be the same as :admin_note but would additionally be true if noteable is a Design and user can :create_note on the Design.
  2. Expose new :reposition_note permission in GraphQL NotePermissions type.
  3. Add a new repositionImageDiffNote mutation, that takes note_id, x, y, width, height args. note_id must be the Global ID of a DiffNote. User must have :reposition_note permission on the note. (The name of the mutation has been chosen to be consistent with the existing note mutation naming conventions)

Frontend Proposal

frontend will need to update note-moving logic to cater for the new mutation and/or scope

Availability & Testing

  • frontend will need to update jest specs that check for inability to move a note without adminNote permission.
  • Unit tests of new mutation (including authorization, error messages, and what happens when not passed a note global id)

Links / references

Testing Activity

@alexkalderimis

  • Yes, at the service, mutation and feature level

@.luke

  • Unit tests + request test of mutation, including authorization

MR Breakdown

@alexkalderimis

  • The issue is well specified. The intent is to add a new mutation solely for repositioning notes given the :create_note permission. This will require a new mutation, possibly a new service and new request and feature specs
    • Could be done in one, but it will be on the chunky side. May grow to two if splitting is necessary

@.luke

Documentation

@.luke

  • FE docs will be required
Edited by Luke Duncalfe