Skip to content
Snippets Groups Projects

GraphQL mutations for add, remove and toggle emoji

Merged Luke Duncalfe requested to merge 62826-graphql-emoji-mutations into master

What does this MR do?

GraphQL mutations for add, remove and toggle emoji

Adding new AddAwardEmoji, RemoveAwardEmoji and ToggleAwardEmoji GraphQL mutations.

Does this MR meet the acceptance criteria?

Conformity

Performance and testing

Edited by 🤖 GitLab Bot 🤖

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Luke Duncalfe
  • Luke Duncalfe
  • Luke Duncalfe
  • Author Maintainer

    @reprazent I hope you don't mind if I ask you to also review this MR? It's particularly because I'm interested to know your opinion on this https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/29919#note_184446765. The thinking is that mutation errors could be presented the same as when we raise an error due to authorization checks. The exception turns into:

    {
      errors: 'Some error'
    }

    Rather than how mergeRequestWip currently presents its error:

    {
      data: {
        errors: 'Some error'
      }
    }

    The benefit would be the front-end would only need to check for errors in one structure. If the mutation was dealing with a collection and was allowed to mutate some in the collection but not the records that were invalid, it could probably in that case respond with the errors in its normal data.errors response.

    :point_right: Edit: See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/29919#note_184446765

    Edited by Luke Duncalfe
  • Author Maintainer

    Note: This MR mentions in many places that it will leverage service classes in https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/29782. For now, to allow this to be merged ahead of that MR it will be introducing some ~"technical debt" to be cleaned up when the service classes are available.

  • Bob Van Landuyt
  • Luke Duncalfe added 545 commits

    added 545 commits

    Compare with previous version

  • Luke Duncalfe
  • Luke Duncalfe added 1 commit

    added 1 commit

    • 725923f9 - GraphQL mutations for add, remove and toggle emoji

    Compare with previous version

  • Luke Duncalfe added 1 commit

    added 1 commit

    • 2ff04615 - GraphQL mutations for add, remove and toggle emoji

    Compare with previous version

  • Author Maintainer

    @reprazent Thank you for your review! I've updated this MR with your feedback. I agree that the pre_check is leaky of information about the resource. I haven't yet changed that behaviour because I was wondering whether it's important to have a "nice message" when the resource isn't Awardable https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/29919#note_184888751.

  • Bob Van Landuyt
  • Bob Van Landuyt
  • Bob Van Landuyt
  • unassigned @.luke

  • Luke Duncalfe assigned to @.luke and unassigned @reprazent

    assigned to @.luke and unassigned @reprazent

  • Luke Duncalfe added 81 commits

    added 81 commits

    Compare with previous version

  • Luke Duncalfe added 1 commit

    added 1 commit

    • 1ad5887b - GraphQL mutations for add, remove and toggle emoji

    Compare with previous version

  • Bob Van Landuyt
  • Luke Duncalfe changed the description

    changed the description

  • mentioned in issue #63797 (moved)

  • mentioned in issue #63798 (moved)

  • Luke Duncalfe added 1 commit

    added 1 commit

    • 97ff00fe - GraphQL mutations for add, remove and toggle emoji

    Compare with previous version

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading