Skip to content

Loading…

Expose more information about open merge requests #2901

Closed
wants to merge 2 commits into from

3 participants

@calmh

This adds a new API call per project that returns all open merge requests, including notes and commits. We use this to automatically evaluate merge requests for style etc and post a +1 or -1.

Since the existing merge requests call returns all MR:s (even those already closed or merged) and the new information I want to return might be expensive, I chose to add a new API call for only open requests instead of fleshing out the existing one.

calmh added some commits
@calmh calmh Expose detailed information on open MR:s in API
This exposes detailed open merge requests for a certain project, while
keeping the existing API intact.
ef922c3
@calmh calmh Update documentation for new API call c97c0a9
@AlexDenisov AlexDenisov commented on the diff
lib/api/merge_requests.rb
@@ -19,6 +19,23 @@ class MergeRequests < Grape::API
present paginate(user_project.merge_requests), with: Entities::MergeRequest
end
+ # List open merge requests
+ #
+ # Parameters:
+ # id (required) - The ID of a project
+ #
+ # Example:
+ # GET /projects/:id/open_merge_requests
+ #
+ # Returns a more detailed merge request object than the method above,
+ # including unmerged commits.
+ #
+ get ":id/open_merge_requests" do
+ authorize! :read_merge_request, user_project
+
+ present paginate(user_project.merge_requests.where(:closed => false, :merged => false)), with: Entities::DetailedMergeRequest

I think that better solution is to move this to user_project entity, and add new scope.
Also, you should use new ruby syntax closed: false instead of :closed => false.

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

Could you add tests, that cover this cases?

@calmh

Sure. I need to familiarize myself with the infrastructure, ignore this one until then.

@mikkyhouse

Closing this one since it need some tests. Feel free to open new one when tests will be ready

@mikkyhouse mikkyhouse closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 4, 2013
  1. @calmh

    Expose detailed information on open MR:s in API

    calmh committed
    This exposes detailed open merge requests for a certain project, while
    keeping the existing API intact.
  2. @calmh
Showing with 123 additions and 0 deletions.
  1. +101 −0 doc/api/merge_requests.md
  2. +5 −0 lib/api/entities.rb
  3. +17 −0 lib/api/merge_requests.rb
View
101 doc/api/merge_requests.md
@@ -40,6 +40,107 @@ Parameters:
]
```
+## List open merge requests
+
+Get all open (i.e. non-closed, non-merged) MR for this project. This
+returns a more filled out merge request object that includes commit and
+note information.
+
+```
+GET /projects/:id/open_merge_requests
+```
+
+Parameters:
+
++ `id` (required) - The ID of a project
+
+```json
+[
+ {
+ "id":1,
+ "target_branch":"master",
+ "source_branch":"test1",
+ "project_id":3,
+ "title":"test1",
+ "closed":true,
+ "merged":false,
+ "author":{
+ "id":1,
+ "username": "admin",
+ "email":"admin@local.host",
+ "name":"Administrator",
+ "blocked":false,
+ "created_at":"2012-04-29T08:46:00Z"
+ },
+ "assignee":{
+ "id":1,
+ "username": "admin",
+ "email":"admin@local.host",
+ "name":"Administrator",
+ "blocked":false,
+ "created_at":"2012-04-29T08:46:00Z"
+ },
+ "mr_and_commit_notes": [
+ {
+ "attachment": {
+ "url": null
+ },
+ "author_id": 5,
+ "commit_id": null,
+ "created_at": "2013-02-01T15:13:15Z",
+ "id": 159,
+ "line_code": null,
+ "note": ":-1: This needs more work.",
+ "noteable_id": 27,
+ "noteable_type": "MergeRequest",
+ "project_id": 39,
+ "updated_at": "2013-02-01T15:13:15Z"
+ },
+ {
+ "attachment": {
+ "url": null
+ },
+ "author_id": 4,
+ "commit_id": "",
+ "created_at": "2013-02-02T09:59:57Z",
+ "id": 161,
+ "line_code": null,
+ "note": ":+1: Seems legit",
+ "noteable_id": 27,
+ "noteable_type": "MergeRequest",
+ "project_id": 39,
+ "updated_at": "2013-02-02T09:59:57Z"
+ }
+ ],
+ "unmerged_commits": [
+ {
+ "commit": {
+ "id": "f8b1ebe1a21aebee3b5c19d5216e7de40b7db43f",
+ "parents": [
+ {
+ "id": "10792ad4f8f42354aed2c5a4997b503171f5d1be"
+ }
+ ],
+ "tree": "eeb3e324299e458fe1b6e6fc709900a8ed7e5d94",
+ "message": "BUG: Don't frobble the quuxes.",
+ "author": {
+ "name": "johndoe",
+ "email": "john.doe@example.com"
+ },
+ "committer": {
+ "name": "johndoe",
+ "email": "john.doe@example.com"
+ },
+ "authored_date": "2013-02-01T16:12:44+01:00",
+ "committed_date": "2013-02-01T16:12:44+01:00"
+ },
+ "head": null
+ }
+ ]
+ }
+]
+```
+
## Show MR
Show information about MR.
View
5 lib/api/entities.rb
@@ -70,6 +70,11 @@ class MergeRequest < Grape::Entity
expose :author, :assignee, using: Entities::UserBasic
end
+ class DetailedMergeRequest < MergeRequest
+ expose :unmerged_commits
+ expose :mr_and_commit_notes
+ end
+
class Note < Grape::Entity
expose :id
expose :note, as: :body
View
17 lib/api/merge_requests.rb
@@ -19,6 +19,23 @@ class MergeRequests < Grape::API
present paginate(user_project.merge_requests), with: Entities::MergeRequest
end
+ # List open merge requests
+ #
+ # Parameters:
+ # id (required) - The ID of a project
+ #
+ # Example:
+ # GET /projects/:id/open_merge_requests
+ #
+ # Returns a more detailed merge request object than the method above,
+ # including unmerged commits.
+ #
+ get ":id/open_merge_requests" do
+ authorize! :read_merge_request, user_project
+
+ present paginate(user_project.merge_requests.where(:closed => false, :merged => false)), with: Entities::DetailedMergeRequest

I think that better solution is to move this to user_project entity, and add new scope.
Also, you should use new ruby syntax closed: false instead of :closed => false.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ end
+
# Show MR
#
# Parameters:
Something went wrong with that request. Please try again.