Version page should show build status + not being able to pick a non-built doc #5169
Conversation
| @@ -41,6 +41,15 @@ <h3>{% trans "Versions" %}</h3> | |||
| {% if request.user|is_admin:project %} | |||
| <ul class="module-item-menu"> | |||
| <li><a href="{% url "project_version_detail" project.slug version.slug %}">{% trans "Edit" %}</a></li> | |||
| <li> | |||
| <span class="right quiet" style="margin:10px 5px 0px 0px;"> | |||
| {% if version.built %} | |||
There was a problem hiding this comment.
This isn't checking if the build is passing, but only that it has ever passed. We should properly check the last build for this version to get this data.
There was a problem hiding this comment.
@ericholscher
I have implemented a custom template tag for this purpose.
| @@ -41,6 +42,16 @@ <h3>{% trans "Versions" %}</h3> | |||
| {% if request.user|is_admin:project %} | |||
| <ul class="module-item-menu"> | |||
| <li><a href="{% url "project_version_detail" project.slug version.slug %}">{% trans "Edit" %}</a></li> | |||
| <li> | |||
| <span class="right quiet" style="margin:10px 5px 0px 0px;"> | |||
| is success else returns false. Returns None if no | ||
| build is found for the given version. | ||
| """ | ||
| res = version.builds.all().order_by('-date') |
There was a problem hiding this comment.
We can just fetch the success column from here, not the whole object
There was a problem hiding this comment.
Thanks.
I have pushed the changes.
|
@stsewd |
| @@ -41,6 +42,16 @@ <h3>{% trans "Versions" %}</h3> | |||
| {% if request.user|is_admin:project %} | |||
| <ul class="module-item-menu"> | |||
| <li><a href="{% url "project_version_detail" project.slug version.slug %}">{% trans "Edit" %}</a></li> | |||
| <li> | |||
| <span class="right quiet status-middle"> | |||
| {% is_latest_built_success version as is_success %} | |||
There was a problem hiding this comment.
This should use this model method: https://github.com/rtfd/readthedocs.org/blob/master/readthedocs/projects/models.py#L882 -- it's likely much more efficient.
There was a problem hiding this comment.
get_latest_build returns the latest built of the project -- irrespective of the version.
We need the builds to be of specific version here.
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
We should probably fix these. Assigned core to review. |
There was a problem hiding this comment.
I'm generally against adding more metadata into columns in our views (which are already too cramped). Our design project is already planning on addressing this, and will add more metadata to the version listing without cramping the table view. I think this needs more work, and ultimately the solution will only be temporary, as the design overhaul is going to rethink this view. I'd probably vote to pass on this PR and focus elsewhere.
If we decide to try to aim for a short term fix with this PR, the issues I have with this design are:
media/css/core.css
Outdated
| @@ -720,6 +720,8 @@ p.build-missing { font-size: .8em; color: #9d9a55; margin: 0 0 3px; } | |||
| .hide { display: none; } | |||
| .left { float: left; } | |||
| .right { float: right; } | |||
| .status-middle { margin: 10px 5px 0px 0px; } | |||
| .status-failing { margin-left: 14px } | |||
There was a problem hiding this comment.
You shouldn't use pixel margins like this, we can't guarantee the difference is 14px -- font differences, font size differences, and translated strings will all produce something other than 14px. the proper way to do this is to use a fixed block size as a parent element, which is independent of the text we're trying to separate. But there are still issues with translations in this case, so I'm generally against trying to fit more information into a coulmn format and that is why our design project is moving towards a multi-line row format for all tabular data like this.
There was a problem hiding this comment.
Also, there are already styles for adding multiple elements in a single row like this, however you'll have to search for an example, I don't have one available. There are several dashboard forms that already use a three column approach.
|
@agjohnson |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Completes feature request 1 and 3 of issue #1253
Feature Request 1
Feature Request 3
