New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
move release commits endpoint to organization path #4863
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
from __future__ import absolute_import | ||
|
||
from sentry.api.base import DocSection | ||
from sentry.api.bases.organization import OrganizationReleasesBaseEndpoint | ||
from sentry.api.exceptions import ResourceDoesNotExist | ||
from sentry.api.serializers import serialize | ||
from sentry.models import Release, ReleaseCommit | ||
|
||
|
||
class OrganizationReleaseCommitsEndpoint(OrganizationReleasesBaseEndpoint): | ||
doc_section = DocSection.RELEASES | ||
|
||
def get(self, request, organization, version): | ||
""" | ||
List an Organization Release's Commits | ||
`````````````````````````````````````` | ||
|
||
Retrieve a list of commits for a given release. | ||
|
||
:pparam string organization_slug: the slug of the organization the | ||
release belongs to. | ||
:pparam string version: the version identifier of the release. | ||
:auth: required | ||
""" | ||
try: | ||
release = Release.objects.get( | ||
organization_id=organization.id, | ||
projects=self.get_allowed_projects(request, organization), | ||
version=version, | ||
) | ||
except Release.DoesNotExist: | ||
raise ResourceDoesNotExist | ||
|
||
queryset = ReleaseCommit.objects.filter( | ||
release=release, | ||
).select_related('commit', 'commit__author') | ||
|
||
return self.paginate( | ||
request=request, | ||
queryset=queryset, | ||
order_by='order', | ||
on_results=lambda x: serialize([rc.commit for rc in x], request.user), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we're only serializing the Commit objects instead of the ReleaseCommit, why don't we just query for Commits instead? Seems a bit awkward going through ReleaseCommit just to get that. You could just do: Commit.objects.filter(releasecommit__release=release) And you'll directly have the objects you want. Then the This gives us SQL like: SELECT "sentry_commit"."id", "sentry_commit"."organization_id", "sentry_commit"."repository_id", "sentry_commit"."key", "sentry_commit"."date_added", "sentry_commit"."author_id", "sentry_commit"."message"
FROM "sentry_commit"
INNER JOIN "sentry_releasecommit" ON (
"sentry_commit"."id" = "sentry_releasecommit"."commit_id"
)
WHERE "sentry_releasecommit"."release_id" = 1
ORDER BY "sentry_releasecommit"."order" ASC Unless I'm missing something here with what you're doing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i just did it this way because that's how we were doing it in the project version of this endpoint (see below file). but i can change it in both places There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok, so that doesn't work because of https://github.com/getsentry/sentry/blob/master/src/sentry/api/paginator.py#L120 like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. welp 😢 |
||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
from __future__ import absolute_import | ||
|
||
from django.core.urlresolvers import reverse | ||
|
||
from sentry.models import Commit, Release, ReleaseCommit, Repository | ||
from sentry.testutils import APITestCase | ||
|
||
|
||
class ReleaseCommitsListTest(APITestCase): | ||
def test_simple(self): | ||
project = self.create_project( | ||
name='foo', | ||
) | ||
release = Release.objects.create( | ||
organization_id=project.organization_id, | ||
version='1', | ||
) | ||
release.add_project(project) | ||
repo = Repository.objects.create( | ||
organization_id=project.organization_id, | ||
name=project.name, | ||
) | ||
commit = Commit.objects.create( | ||
organization_id=project.organization_id, | ||
repository_id=repo.id, | ||
key='a' * 40, | ||
) | ||
commit2 = Commit.objects.create( | ||
organization_id=project.organization_id, | ||
repository_id=repo.id, | ||
key='b' * 40, | ||
) | ||
ReleaseCommit.objects.create( | ||
organization_id=project.organization_id, | ||
release=release, | ||
commit=commit, | ||
order=1, | ||
) | ||
ReleaseCommit.objects.create( | ||
organization_id=project.organization_id, | ||
release=release, | ||
commit=commit2, | ||
order=0, | ||
) | ||
url = reverse('sentry-api-0-organization-release-commits', kwargs={ | ||
'organization_slug': project.organization.slug, | ||
'version': release.version, | ||
}) | ||
|
||
self.login_as(user=self.user) | ||
|
||
response = self.client.get(url) | ||
|
||
assert response.status_code == 200, response.content | ||
assert len(response.data) == 2 | ||
assert response.data[0]['id'] == commit2.key | ||
assert response.data[1]['id'] == commit.key |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an API change.