From e6215a9a8ed20354782120a7ce6368c7e8aab9a5 Mon Sep 17 00:00:00 2001
From: Robert Schilling <rschilling@student.tugraz.at>
Date: Wed, 6 Apr 2016 20:53:17 +0200
Subject: [PATCH] Improve coding and doc style

---
 CHANGELOG                            |  2 +-
 doc/api/milestones.md                | 19 ++++++++++++++-----
 lib/api/milestones.rb                |  2 +-
 spec/requests/api/milestones_spec.rb |  8 +++++---
 4 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 531f3607297..ff86e66e16e 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -15,6 +15,7 @@ v 8.7.0 (unreleased)
   - Handle nil descriptions in Slack issue messages (Stan Hu)
   - Add default scope to projects to exclude projects pending deletion
   - Ensure empty recipients are rejected in BuildsEmailService
+  - API: Ability to filter milestones by state `active` and `closed` (Robert Schilling)
   - Implement 'Groups View' as an option for dashboard preferences !3379 (Elias W.)
   - Implement 'TODOs View' as an option for dashboard preferences !3379 (Elias W.)
   - Gracefully handle notes on deleted commits in merge requests (Stan Hu)
@@ -25,7 +26,6 @@ v 8.7.0 (unreleased)
   - Build status notifications
   - API: Ability to retrieve a specific tag (Robert Schilling)
   - API: Expose user location (Robert Schilling)
-  - API: Ability to milter milestones by state `active` and `closed` (Robert Schilling)
 
 v 8.6.5 (unreleased)
   - Check permissions when user attempts to import members from another project
diff --git a/doc/api/milestones.md b/doc/api/milestones.md
index 4d94d7f3e2c..e4202025f80 100644
--- a/doc/api/milestones.md
+++ b/doc/api/milestones.md
@@ -11,6 +11,20 @@ GET /projects/:id/milestones?state=active
 GET /projects/:id/milestones?state=closed
 ```
 
+Parameters:
+
+| Attribute | Type | Required | Description |
+| --------- | ---- | -------- | ----------- |
+| `id` | integer | yes | The ID of a project |
+| `iid` | integer | optional | Return only the milestone having the given `iid` |
+| `state` | string | optional | Return  only `active` or `closed` milestones` |
+
+```bash
+curl -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/projects/5/milestones
+```
+
+Example Response:
+
 ```json
 [
   {
@@ -27,11 +41,6 @@ GET /projects/:id/milestones?state=closed
 ]
 ```
 
-Parameters:
-
-- `id` (required) - The ID of a project
-- `iid` (optional) - Return the milestone having the given `iid`
-- `state` (optional) - Return "active" or "closed" milestones
 
 ## Get single milestone
 
diff --git a/lib/api/milestones.rb b/lib/api/milestones.rb
index 39efa1b176b..afb6ffa3609 100644
--- a/lib/api/milestones.rb
+++ b/lib/api/milestones.rb
@@ -27,7 +27,7 @@ module API
         authorize! :read_milestone, user_project
 
         milestones = user_project.milestones
-        milestones = filter_milestones_state(milestones, params[:state]) unless params[:state].nil?
+        milestones = filter_milestones_state(milestones, params[:state])
 
         present paginate(milestones), with: Entities::Milestone
       end
diff --git a/spec/requests/api/milestones_spec.rb b/spec/requests/api/milestones_spec.rb
index 281d2b25bf5..d97bf6d38ff 100644
--- a/spec/requests/api/milestones_spec.rb
+++ b/spec/requests/api/milestones_spec.rb
@@ -4,7 +4,7 @@ describe API::API, api: true  do
   include ApiHelpers
   let(:user) { create(:user) }
   let!(:project) { create(:project, namespace: user.namespace ) }
-  let!(:closed_milestone) { create(:closed_milestone, project: project, state: :closed) }
+  let!(:closed_milestone) { create(:closed_milestone, project: project) }
   let!(:milestone) { create(:milestone, project: project) }
 
   before { project.team << [user, :developer] }
@@ -22,16 +22,18 @@ describe API::API, api: true  do
       expect(response.status).to eq(401)
     end
 
-    it 'should return an array of active milestones' do
+    it 'returns an array of active milestones' do
       get api("/projects/#{project.id}/milestones?state=active", user)
+
       expect(response.status).to eq(200)
       expect(json_response).to be_an Array
       expect(json_response.length).to eq(1)
       expect(json_response.first['id']).to eq(milestone.id)
     end
 
-    it 'should return an array of closed milestones' do
+    it 'returns an array of closed milestones' do
       get api("/projects/#{project.id}/milestones?state=closed", user)
+
       expect(response.status).to eq(200)
       expect(json_response).to be_an Array
       expect(json_response.length).to eq(1)