diff options
author | Sean McGivern <sean@mcgivern.me.uk> | 2017-02-13 12:00:29 +0000 |
---|---|---|
committer | Sean McGivern <sean@mcgivern.me.uk> | 2017-02-13 12:00:29 +0000 |
commit | d8345a5d4359ec2783216b3b4896e43fbe731c6d (patch) | |
tree | 9321be75709887e69bf88eab1fb19b510f3b13b9 /lib | |
parent | 2cb70671f3c90a4ade7b4d04dcffc927b6cee75c (diff) | |
parent | 88d610c60e9064f92419481a9df6453b3c8079b3 (diff) | |
download | gitlab-ce-d8345a5d4359ec2783216b3b4896e43fbe731c6d.tar.gz |
Merge branch '20732_member_exists_409' into 'master'
Add member: Always return 409 when a member exists
Closes #20732
See merge request !9093
Diffstat (limited to 'lib')
-rw-r--r-- | lib/api/api.rb | 1 | ||||
-rw-r--r-- | lib/api/members.rb | 13 | ||||
-rw-r--r-- | lib/api/v3/members.rb | 134 |
3 files changed, 138 insertions, 10 deletions
diff --git a/lib/api/api.rb b/lib/api/api.rb index eb9792680ff..7ec089b9c29 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -7,6 +7,7 @@ module API version 'v3', using: :path do mount ::API::V3::DeployKeys mount ::API::V3::Issues + mount ::API::V3::Members mount ::API::V3::MergeRequests mount ::API::V3::Projects mount ::API::V3::ProjectSnippets diff --git a/lib/api/members.rb b/lib/api/members.rb index d85f1f78cd6..d1d78775c6d 100644 --- a/lib/api/members.rb +++ b/lib/api/members.rb @@ -56,16 +56,9 @@ module API member = source.members.find_by(user_id: params[:user_id]) - # We need this explicit check because `source.add_user` doesn't - # currently return the member created so it would return 201 even if - # the member already existed... - # The `source_type == 'group'` check is to ensure back-compatibility - # but 409 behavior should be used for both project and group members in 9.0! - conflict!('Member already exists') if source_type == 'group' && member - - unless member - member = source.add_user(params[:user_id], params[:access_level], current_user: current_user, expires_at: params[:expires_at]) - end + conflict!('Member already exists') if member + + member = source.add_user(params[:user_id], params[:access_level], current_user: current_user, expires_at: params[:expires_at]) if member.persisted? && member.valid? present member.user, with: Entities::Member, member: member diff --git a/lib/api/v3/members.rb b/lib/api/v3/members.rb new file mode 100644 index 00000000000..4e6cb2e3c52 --- /dev/null +++ b/lib/api/v3/members.rb @@ -0,0 +1,134 @@ +module API + module V3 + class Members < Grape::API + include PaginationParams + + before { authenticate! } + + helpers ::API::Helpers::MembersHelpers + + %w[group project].each do |source_type| + params do + requires :id, type: String, desc: "The #{source_type} ID" + end + resource source_type.pluralize do + desc 'Gets a list of group or project members viewable by the authenticated user.' do + success ::API::Entities::Member + end + params do + optional :query, type: String, desc: 'A query string to search for members' + use :pagination + end + get ":id/members" do + source = find_source(source_type, params[:id]) + + users = source.users + users = users.merge(User.search(params[:query])) if params[:query] + + present paginate(users), with: ::API::Entities::Member, source: source + end + + desc 'Gets a member of a group or project.' do + success ::API::Entities::Member + end + params do + requires :user_id, type: Integer, desc: 'The user ID of the member' + end + get ":id/members/:user_id" do + source = find_source(source_type, params[:id]) + + members = source.members + member = members.find_by!(user_id: params[:user_id]) + + present member.user, with: ::API::Entities::Member, member: member + end + + desc 'Adds a member to a group or project.' do + success ::API::Entities::Member + end + params do + requires :user_id, type: Integer, desc: 'The user ID of the new member' + requires :access_level, type: Integer, desc: 'A valid access level (defaults: `30`, developer access level)' + optional :expires_at, type: DateTime, desc: 'Date string in the format YEAR-MONTH-DAY' + end + post ":id/members" do + source = find_source(source_type, params[:id]) + authorize_admin_source!(source_type, source) + + member = source.members.find_by(user_id: params[:user_id]) + + # We need this explicit check because `source.add_user` doesn't + # currently return the member created so it would return 201 even if + # the member already existed... + # The `source_type == 'group'` check is to ensure back-compatibility + # but 409 behavior should be used for both project and group members in 9.0! + conflict!('Member already exists') if source_type == 'group' && member + + unless member + member = source.add_user(params[:user_id], params[:access_level], current_user: current_user, expires_at: params[:expires_at]) + end + if member.persisted? && member.valid? + present member.user, with: ::API::Entities::Member, member: member + else + # This is to ensure back-compatibility but 400 behavior should be used + # for all validation errors in 9.0! + render_api_error!('Access level is not known', 422) if member.errors.key?(:access_level) + render_validation_error!(member) + end + end + + desc 'Updates a member of a group or project.' do + success ::API::Entities::Member + end + params do + requires :user_id, type: Integer, desc: 'The user ID of the new member' + requires :access_level, type: Integer, desc: 'A valid access level' + optional :expires_at, type: DateTime, desc: 'Date string in the format YEAR-MONTH-DAY' + end + put ":id/members/:user_id" do + source = find_source(source_type, params[:id]) + authorize_admin_source!(source_type, source) + + member = source.members.find_by!(user_id: params[:user_id]) + attrs = attributes_for_keys [:access_level, :expires_at] + + if member.update_attributes(attrs) + present member.user, with: ::API::Entities::Member, member: member + else + # This is to ensure back-compatibility but 400 behavior should be used + # for all validation errors in 9.0! + render_api_error!('Access level is not known', 422) if member.errors.key?(:access_level) + render_validation_error!(member) + end + end + + desc 'Removes a user from a group or project.' + params do + requires :user_id, type: Integer, desc: 'The user ID of the member' + end + delete ":id/members/:user_id" do + source = find_source(source_type, params[:id]) + + # This is to ensure back-compatibility but find_by! should be used + # in that casse in 9.0! + member = source.members.find_by(user_id: params[:user_id]) + + # This is to ensure back-compatibility but this should be removed in + # favor of find_by! in 9.0! + not_found!("Member: user_id:#{params[:user_id]}") if source_type == 'group' && member.nil? + + # This is to ensure back-compatibility but 204 behavior should be used + # for all DELETE endpoints in 9.0! + if member.nil? + { message: "Access revoked", id: params[:user_id].to_i } + else + ::Members::DestroyService.new(source, current_user, declared_params).execute + + present member.user, with: ::API::Entities::Member, member: member + end + end + end + end + end + end +end |