Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

API fixes #1735

Merged
merged 6 commits into from

2 participants

@NARKOZ
Collaborator

No description provided.

@vsizov vsizov commented on the diff
spec/requests/api/projects_spec.rb
((6 lines not shown))
describe "PUT /projects/:id/hooks/:hook_id" do
it "should update an existing project hook" do
put api("/projects/#{project.code}/hooks/#{hook.id}", user),
- url: 'http://example.com'
+ url: 'http://example.org'
@vsizov Collaborator
vsizov added a note

what's the point?

@NARKOZ Collaborator
NARKOZ added a note

to test the changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@vsizov
Collaborator

fix mass-assignment error in user create API

What's the error?

@vsizov

Why projects_limit: 3 ???

@vsizov vsizov commented on the diff
lib/api/users.rb
((19 lines not shown))
# Example Request:
# POST /users
post do
authenticated_as_admin!
- attrs = attributes_for_keys [:email, :name, :password, :password_confirmation, :skype, :linkedin, :twitter, :projects_limit]
- user = User.new attrs
+ attrs = attributes_for_keys [:email, :name, :password, :skype, :linkedin, :twitter, :projects_limit]
+ user = User.new attrs, as: :admin
@vsizov Collaborator
vsizov added a note

Why as :admin ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@vsizov vsizov commented on the diff
lib/api/users.rb
((11 lines not shown))
# password (required) - Password
- # password_confirmation (required) - Password confirmation
@vsizov Collaborator
vsizov added a note

Good idea :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@vsizov vsizov commented on the diff
spec/requests/api/users_spec.rb
@@ -42,9 +42,9 @@
end
it "should create user" do
- expect{
- post api("/users", admin), Factory.attributes(:user)
- }.to change{User.count}.by(1)
+ expect {
+ post api("/users", admin), Factory.attributes(:user, projects_limit: 3)
@vsizov Collaborator
vsizov added a note

Why projects_limit: 3 ??
I've already commented on your fork, so one more time did it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@vsizov vsizov merged commit d6a5e3d into gitlabhq:master

1 check passed

Details default The Travis build passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
2  CHANGELOG
@@ -4,7 +4,7 @@ master
- Fixed bug with gitolite keys
- [API] list one project hook
- [API] edit project hook
- - [API] add project snippets list
+ - [API] list project snippets
- [API] allow to authorize using private token in HTTP header
- [API] add user creation
View
6 doc/api/users.md
@@ -74,14 +74,12 @@ POST /users
Parameters:
+ `email` (required) - Email
-+ `name` (required) - Name
+ `password` (required) - Password
-+ `password_confirmation` (required) - Password confirmation
++ `name` - Name
+ `skype` - Skype ID
+ `linkedin` - Linkedin
+ `twitter` - Twitter account
-+ `projects_limit` - Limit projects wich user can create
-
++ `projects_limit` - Number of projects user can create
Will return created user with status `201 Created` on success, or `404 Not
found` on fail.
View
11 lib/api/users.rb
@@ -23,24 +23,23 @@ class Users < Grape::API
@user = User.find(params[:id])
present @user, with: Entities::User
end
-
+
# Create user. Available only for admin
#
# Parameters:
# email (required) - Email
- # name (required) - Name
# password (required) - Password
- # password_confirmation (required) - Password confirmation
@vsizov Collaborator
vsizov added a note

Good idea :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ # name - Name
# skype - Skype ID
# linkedin - Linkedin
# twitter - Twitter account
- # projects_limit - Limit projects wich user can create
+ # projects_limit - Number of projects user can create
# Example Request:
# POST /users
post do
authenticated_as_admin!
- attrs = attributes_for_keys [:email, :name, :password, :password_confirmation, :skype, :linkedin, :twitter, :projects_limit]
- user = User.new attrs
+ attrs = attributes_for_keys [:email, :name, :password, :skype, :linkedin, :twitter, :projects_limit]
+ user = User.new attrs, as: :admin
@vsizov Collaborator
vsizov added a note

Why as :admin ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
if user.save
present user, with: Entities::User
else
View
12 spec/requests/api/projects_spec.rb
@@ -46,7 +46,7 @@
response.status.should == 201
end
- it "should repsond with 404 on failure" do
+ it "should respond with 404 on failure" do
post api("/projects", user)
response.status.should == 404
end
@@ -188,16 +188,16 @@
}.to change {project.hooks.count}.by(1)
end
end
-
+
describe "PUT /projects/:id/hooks/:hook_id" do
it "should update an existing project hook" do
put api("/projects/#{project.code}/hooks/#{hook.id}", user),
- url: 'http://example.com'
+ url: 'http://example.org'
@vsizov Collaborator
vsizov added a note

what's the point?

@NARKOZ Collaborator
NARKOZ added a note

to test the changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
response.status.should == 200
- json_response['url'].should == 'http://example.com'
+ json_response['url'].should == 'http://example.org'
end
end
-
+
describe "DELETE /projects/:id/hooks" do
it "should delete hook from project" do
@@ -239,7 +239,7 @@
end
describe "GET /projects/:id/snippets" do
- it "should return a project snippet" do
+ it "should return an array of project snippets" do
get api("/projects/#{project.code}/snippets", user)
response.status.should == 200
json_response.should be_an Array
View
8 spec/requests/api/users_spec.rb
@@ -4,7 +4,7 @@
include ApiHelpers
let(:user) { Factory :user }
- let(:admin) {Factory :admin}
+ let(:admin) { Factory :admin }
let(:key) { Factory :key, user: user }
describe "GET /users" do
@@ -42,9 +42,9 @@
end
it "should create user" do
- expect{
- post api("/users", admin), Factory.attributes(:user)
- }.to change{User.count}.by(1)
+ expect {
+ post api("/users", admin), Factory.attributes(:user, projects_limit: 3)
@vsizov Collaborator
vsizov added a note

Why projects_limit: 3 ??
I've already commented on your fork, so one more time did it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ }.to change { User.count }.by(1)
end
it "shouldn't available for non admin users" do
Something went wrong with that request. Please try again.