Skip to content

Update build list and detail page UX #5916

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

Merged
merged 6 commits into from
Jul 15, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions readthedocs/api/v2/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ class BuildSerializer(serializers.ModelSerializer):
version_slug = serializers.ReadOnlyField(source='version.slug')
docs_url = serializers.ReadOnlyField(source='version.get_absolute_url')
state_display = serializers.ReadOnlyField(source='get_state_display')
commit_url = serializers.ReadOnlyField(source='get_commit_url')
# Jsonfield needs an explicit serializer
# https://github.com/dmkoch/django-jsonfield/issues/188#issuecomment-300439829
config = serializers.JSONField(required=False)
Expand Down
3 changes: 3 additions & 0 deletions readthedocs/builds/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,3 +87,6 @@
'description': 'The build succeeded!',
},
}

GITHUB_EXTERNAL_VERSION_NAME = 'Pull Request'
GENERIC_EXTERNAL_VERSION_NAME = 'External Version'
122 changes: 102 additions & 20 deletions readthedocs/builds/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,13 @@
from readthedocs.config import LATEST_CONFIGURATION_VERSION
from readthedocs.core.utils import broadcast
from readthedocs.projects.constants import (
BITBUCKET_COMMIT_URL,
BITBUCKET_URL,
GITHUB_COMMIT_URL,
GITHUB_URL,
GITHUB_PULL_REQUEST_URL,
GITHUB_PULL_REQUEST_COMMIT_URL,
GITLAB_COMMIT_URL,
GITLAB_URL,
PRIVACY_CHOICES,
PRIVATE,
Expand All @@ -36,6 +41,8 @@
BUILD_STATE_FINISHED,
BUILD_STATE_TRIGGERED,
BUILD_TYPES,
GENERIC_EXTERNAL_VERSION_NAME,
GITHUB_EXTERNAL_VERSION_NAME,
INTERNAL,
LATEST,
NON_REPOSITORY_VERSIONS,
Expand Down Expand Up @@ -63,6 +70,7 @@
get_gitlab_username_repo,
)
from readthedocs.builds.version_slug import VersionSlugField
from readthedocs.oauth.models import RemoteRepository


log = logging.getLogger(__name__)
Expand Down Expand Up @@ -158,9 +166,20 @@ def vcs_url(self):
"""
Generate VCS (github, gitlab, bitbucket) URL for this version.

Branch/Tag Example: https://github.com/rtfd/readthedocs.org/tree/3.4.2/.
Pull/merge Request Example: https://github.com/rtfd/readthedocs.org/pull/9999/.
Example: https://github.com/rtfd/readthedocs.org/tree/3.4.2/.
External Version Example: https://github.com/rtfd/readthedocs.org/pull/99/.
"""
if self.type == EXTERNAL:
if 'github' in self.project.repo:
user, repo = get_github_username_repo(self.project.repo)
return GITHUB_PULL_REQUEST_URL.format(
user=user,
repo=repo,
number=self.verbose_name,
)
# TODO: Add VCS URL for other Git Providers
return ''

url = ''
if self.slug == STABLE:
slug_url = self.ref
Expand All @@ -169,25 +188,12 @@ def vcs_url(self):
else:
slug_url = self.slug

if self.type == EXTERNAL:
if 'github' in self.project.repo:
slug_url = self.verbose_name
url = f'/pull/{slug_url}/'
if ('github' in self.project.repo) or ('gitlab' in self.project.repo):
url = f'/tree/{slug_url}/'

if 'gitlab' in self.project.repo:
slug_url = self.identifier
url = f'/merge_requests/{slug_url}/'

if 'bitbucket' in self.project.repo:
slug_url = self.identifier
url = f'/pull-requests/{slug_url}'
else:
if ('github' in self.project.repo) or ('gitlab' in self.project.repo):
url = f'/tree/{slug_url}/'

if 'bitbucket' in self.project.repo:
slug_url = self.identifier
url = f'/src/{slug_url}'
if 'bitbucket' in self.project.repo:
slug_url = self.identifier
url = f'/src/{slug_url}'

# TODO: improve this replacing
return self.project.repo.replace('git://', 'https://').replace('.git', '') + url
Expand Down Expand Up @@ -745,6 +751,60 @@ def get_full_url(self):
)
return full_url

def get_commit_url(self):
"""Return the commit URL."""
repo_url = self.project.repo
if self.is_external:
if 'github' in repo_url:
user, repo = get_github_username_repo(repo_url)
if not user and not repo:
return ''

repo = repo.rstrip('/')
return GITHUB_PULL_REQUEST_COMMIT_URL.format(
user=user,
repo=repo,
number=self.version.verbose_name,
commit=self.commit
)
# TODO: Add External Version Commit URL for other Git Providers
else:
if 'github' in repo_url:
user, repo = get_github_username_repo(repo_url)
if not user and not repo:
return ''

repo = repo.rstrip('/')
return GITHUB_COMMIT_URL.format(
user=user,
repo=repo,
commit=self.commit
)
if 'gitlab' in repo_url:
user, repo = get_gitlab_username_repo(repo_url)
if not user and not repo:
return ''

repo = repo.rstrip('/')
return GITLAB_COMMIT_URL.format(
user=user,
repo=repo,
commit=self.commit
)
if 'bitbucket' in repo_url:
user, repo = get_bitbucket_username_repo(repo_url)
if not user and not repo:
return ''

repo = repo.rstrip('/')
return BITBUCKET_COMMIT_URL.format(
user=user,
repo=repo,
commit=self.commit
)

return ''
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do wonder if we should return None here, instead of ''. I don't think it matters too much though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for URL's I think its better to return empty string. as it will show href='' not href=None in the template.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not output the link if we don't have an href attribute.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merging this, but we should fix it ^^

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ericholscher Updated please see this 763b857


@property
def finished(self):
"""Return if build has a finished state."""
Expand All @@ -756,6 +816,28 @@ def is_stale(self):
mins_ago = timezone.now() - datetime.timedelta(minutes=5)
return self.state == BUILD_STATE_TRIGGERED and self.date < mins_ago

@property
def is_external(self):
return self.version.type == EXTERNAL

@property
def external_version_name(self):
if self.is_external:
try:
if self.project.remote_repository.account.provider == 'github':
return GITHUB_EXTERNAL_VERSION_NAME
# TODO: Add External Version Name for other Git Providers
except RemoteRepository.DoesNotExist:
log.info('Remote repository does not exist for %s', self.project)
return GENERIC_EXTERNAL_VERSION_NAME
except Exception:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should log this exception, otherwise it will hide real issues. What exception are we really trying to catch?

log.exception(
'Unhandled exception raised for %s while getting external_version_name',
self.project
)
return GENERIC_EXTERNAL_VERSION_NAME
return None

def using_latest_config(self):
return int(self.config.get('version', '1')) == LATEST_CONFIGURATION_VERSION

Expand Down
2 changes: 2 additions & 0 deletions readthedocs/builds/static-src/builds/js/detail.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ function BuildDetailView(instance) {
});
self.commit = ko.observable(instance.commit);
self.docs_url = ko.observable(instance.docs_url);
self.commit_url = ko.observable(instance.commit_url);

/* Others */
self.legacy_output = ko.observable(false);
Expand All @@ -72,6 +73,7 @@ function BuildDetailView(instance) {
self.length(data.length);
self.commit(data.commit);
self.docs_url(data.docs_url);
self.commit_url(data.commit_url);
var n;
for (n in data.commands) {
var command = data.commands[n];
Expand Down
2 changes: 1 addition & 1 deletion readthedocs/builds/static/builds/js/detail.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 0 additions & 15 deletions readthedocs/builds/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,21 +84,6 @@ def post(self, request, project_slug):

class BuildList(BuildBase, BuildTriggerMixin, ListView):

def get_queryset(self):
# this is used to include only internal version
# builds in the build list page
self.project_slug = self.kwargs.get('project_slug', None)
self.project = get_object_or_404(
Project.objects.protected(self.request.user),
slug=self.project_slug,
)
queryset = Build.objects.public(
user=self.request.user,
project=self.project,
).select_related('project', 'version')

return queryset

def get_context_data(self, **kwargs):
context = super().get_context_data(**kwargs)

Expand Down
20 changes: 20 additions & 0 deletions readthedocs/projects/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -325,13 +325,33 @@
'https://github.com/{user}/{repo}/'
'{action}/{version}{docroot}{path}{source_suffix}'
)
GITHUB_COMMIT_URL = (
'https://github.com/{user}/{repo}/'
'commit/{commit}'
)
GITHUB_PULL_REQUEST_URL = (
'https://github.com/{user}/{repo}/'
'pull/{number}'
)
GITHUB_PULL_REQUEST_COMMIT_URL = (
'https://github.com/{user}/{repo}/'
'pull/{number}/commits/{commit}'
)
BITBUCKET_URL = (
'https://bitbucket.org/{user}/{repo}/'
'src/{version}{docroot}{path}{source_suffix}'
)
BITBUCKET_COMMIT_URL = (
'https://bitbucket.org/{user}/{repo}/'
'commits/{commit}'
)
GITLAB_URL = (
'https://gitlab.com/{user}/{repo}/'
'{action}/{version}{docroot}{path}{source_suffix}'
)
GITLAB_COMMIT_URL = (
'https://gitlab.com/{user}/{repo}/'
'commit/{commit}'
)

GITHUB_GIT_PATTERN = 'pull/{id}/head:external-{id}'
4 changes: 2 additions & 2 deletions readthedocs/projects/views/public.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ def get_context_data(self, **kwargs):
context = super().get_context_data(**kwargs)

project = self.get_object()
context['versions'] = Version.objects.public(
context['versions'] = Version.internal.public(
user=self.request.user,
project=project,
)
Expand Down Expand Up @@ -274,7 +274,7 @@ def project_versions(request, project_slug):
slug=project_slug,
)

versions = Version.objects.public(
versions = Version.internal.public(
user=request.user,
project=project,
only_active=False,
Expand Down
Loading