-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Added a link to open new issue with prefilled details #3683
Changes from 9 commits
5f3ca53
6ad59e6
7ccd8b6
4b3fde7
59b533f
60672aa
02979dc
d075d54
2f337dd
3ebcf90
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 |
---|---|---|
|
@@ -10,6 +10,7 @@ | |
) | ||
|
||
import logging | ||
import textwrap | ||
|
||
from builtins import object | ||
from django.contrib import messages | ||
|
@@ -23,7 +24,10 @@ | |
from django.urls import reverse | ||
from django.utils.decorators import method_decorator | ||
from django.views.generic import DetailView, ListView | ||
from requests.utils import quote | ||
from urllib.parse import urlparse | ||
|
||
from readthedocs.doc_builder.exceptions import BuildEnvironmentError | ||
from readthedocs.builds.models import Build, Version | ||
from readthedocs.core.permissions import AdminPermission | ||
from readthedocs.core.utils import trigger_build | ||
|
@@ -106,6 +110,49 @@ class BuildDetail(BuildBase, DetailView): | |
def get_context_data(self, **kwargs): | ||
context = super(BuildDetail, self).get_context_data(**kwargs) | ||
context['project'] = self.project | ||
|
||
build = self.get_object() | ||
if build.error != BuildEnvironmentError.GENERIC_WITH_BUILD_ID.format(build_id=build.pk): | ||
# Do not suggest to open an issue if the error is not generic | ||
return context | ||
|
||
scheme = ( | ||
'https://github.com/rtfd/readthedocs.org/issues/new' | ||
'?title={title}{build_id}' | ||
'&body={body}' | ||
) | ||
|
||
# TODO: we could use ``.github/ISSUE_TEMPLATE.md`` here, but we would | ||
# need to add some variables to it which could impact in the UX when | ||
# filling an issue from the web | ||
body = """ | ||
## Details: | ||
|
||
* Project URL: https://readthedocs.org/projects/{project_slug}/ | ||
* Build URL(if applicable): https://readthedocs.org{build_path} | ||
* Read the Docs username(if applicable): {username} | ||
|
||
## Expected Result | ||
|
||
*A description of what you wanted to happen* | ||
|
||
## Actual Result | ||
|
||
*A description of what actually happened*""".format( | ||
project_slug=self.project, | ||
build_path=self.request.path, | ||
username=self.request.user, | ||
) | ||
|
||
scheme_dict = { | ||
'title': quote('Build error with build id #'), | ||
'build_id': context['build'].id, | ||
'body': quote(textwrap.dedent(body)), | ||
} | ||
|
||
issue_url = scheme.format(**scheme_dict) | ||
issue_url = urlparse(issue_url).geturl() | ||
context['issue_url'] = issue_url | ||
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. This is tough to follow due to excessive variable usage. Perhaps this could be simplified into a single multi line string with interpolation through |
||
return context | ||
|
||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -177,6 +177,15 @@ <h3>{% trans "Error" %}</h3> | |
data-bind="text: error"> | ||
{{ build.error }} | ||
</p> | ||
<p> | ||
{% block github_issue_link %} | ||
{% if issue_url %} | ||
{% blocktrans trimmed with url=issue_url %} | ||
<a href="{{ url }}">Report any build issues here</a>. | ||
{% endblocktrans %} | ||
{% endif %} | ||
{% endblock %} | ||
</p> | ||
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. This looks good so far! However, we don't want to unconditionally show this paragraph block on a build error, we only want to show this if there was an unknown error with the build -- otherwise we get swamped with support requests we can't handle. We don't have a great way of determining this from our build database yet. We set a generic build failure here: I don't have any immediate suggestions on how to determine here if the build was a generic failure though. Let me think about this more, or perhaps someone else has a suggestion for the interim. I started to outline what I'm describing in #3399, but this is a large amount of work we haven't decided on yet. 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 would like to take up #3399 once we decide the method to determine generic build failure. We could add a field to build models which stores the failure type and then use that to show the paragraph block. 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 push some changes that checks for the generic error and if it's not, the link is not shown. |
||
</div> | ||
|
||
<div id="build-commands" | ||
|
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.
Can't the
urlparse
to add the path args instead of doing.format
?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.
urlparse
takes arguments in a different form and also it was encoding various other characters which resulted in a wrong url.