Skip to content

UI improvements to the admin interface for setting project upload limits #2473

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 2 commits into from
Oct 6, 2017
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
21 changes: 15 additions & 6 deletions tests/unit/admin/views/test_projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -354,21 +354,21 @@ def test_sets_limitwith_integer(self, db_request):
flash=pretend.call_recorder(lambda *a, **kw: None),
)
db_request.matchdict["project_name"] = project.normalized_name
db_request.POST["upload_limit"] = "12345"
db_request.POST["upload_limit"] = "90"

views.set_upload_limit(project, db_request)

assert db_request.session.flash.calls == [
pretend.call(
"Successfully set the upload limit on 'foo' to 12345",
"Successfully set the upload limit on 'foo'.",
queue="success"),
]

assert project.upload_limit == 12345
assert project.upload_limit == 90 * views.ONE_MB

def test_sets_limit_with_none(self, db_request):
project = ProjectFactory.create(name="foo")
project.upload_limit = 12345
project.upload_limit = 90 * views.ONE_MB

db_request.route_path = pretend.call_recorder(
lambda *a, **kw: "/admin/projects/")
Expand All @@ -381,17 +381,26 @@ def test_sets_limit_with_none(self, db_request):

assert db_request.session.flash.calls == [
pretend.call(
"Successfully set the upload limit on 'foo' to None",
"Successfully set the upload limit on 'foo'.",
queue="success"),
]

assert project.upload_limit is None

def test_sets_limit_with_bad_value(self, db_request):
def test_sets_limit_with_non_integer(self, db_request):
project = ProjectFactory.create(name="foo")

db_request.matchdict["project_name"] = project.normalized_name
db_request.POST["upload_limit"] = "meep"

with pytest.raises(HTTPBadRequest):
views.set_upload_limit(project, db_request)

def test_sets_limit_with_less_than_minimum(self, db_request):
project = ProjectFactory.create(name="foo")

db_request.matchdict["project_name"] = project.normalized_name
db_request.POST["upload_limit"] = "20"

with pytest.raises(HTTPBadRequest):
views.set_upload_limit(project, db_request)
17 changes: 14 additions & 3 deletions warehouse/admin/templates/admin/projects/detail.html
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,13 @@ <h3 class="project-name text-center">{{ project.name }}</h3>
</tr>
<tr>
<td>Upload limit</td>
<td>{{ project.upload_limit }}</td>
<td>
{% if project.upload_limit %}
{{ project.upload_limit|filesizeformat(binary=True) }}
{% else %}
Default ({{ MAX_FILESIZE|filesizeformat(binary=True) }})
{% endif %}
</td>
</tr>
</table>
</div>
Expand Down Expand Up @@ -159,8 +165,13 @@ <h3 class="box-title">Set upload limit</h3>
<input name="csrf_token" type="hidden" value="{{ request.session.get_csrf_token() }}">
<div class="box-body">
<div class="form-group col-sm-12">
<label for="uploadLimit">Upload limit (in bytes)</label>
<input type="text" name="upload_limit" class="form-control" id="uploadLimit" rows="3" value="{{ project.upload_limit | default('', True)}}">
<label for="uploadLimit">Upload limit (in Megabytes)</label>
{% if project.upload_limit %}
{% set upload_limit_value = project.upload_limit / ONE_MB %}
Copy link
Member

Choose a reason for hiding this comment

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

Unlike the display issue, this needs to use the same logic as the python code to go from bytes to MB, so this makes sense to keep this regardless of what happens above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Noted.

{% else %}
{% set upload_limit_value = '' %}
{% endif %}
<input type="number" name="upload_limit" class="form-control" id="uploadLimit" min="{{ MAX_FILESIZE / ONE_MB }}" value="{{ upload_limit_value }}">
</div>
</div>

Expand Down
30 changes: 24 additions & 6 deletions warehouse/admin/views/projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@
from warehouse.accounts.models import User
from warehouse.packaging.models import Project, Release, Role, JournalEntry
from warehouse.utils.paginate import paginate_url_factory
from warehouse.forklift.legacy import MAX_FILESIZE

ONE_MB = 1024 * 1024 # bytes


@view_config(
Expand Down Expand Up @@ -101,7 +104,13 @@ def project_detail(project, request):
)
]

return {"project": project, "maintainers": maintainers, "journal": journal}
return {
"project": project,
"maintainers": maintainers,
"journal": journal,
"ONE_MB": ONE_MB,
"MAX_FILESIZE": MAX_FILESIZE
}


@view_config(
Expand Down Expand Up @@ -218,18 +227,27 @@ def set_upload_limit(project, request):
# If the upload limit is an empty string or othrwise falsy, just set the
# limit to None, indicating the default limit.
if not upload_limit:
project.upload_limit = None
upload_limit = None
else:
try:
project.upload_limit = int(upload_limit)
upload_limit = int(upload_limit)
except ValueError:
raise HTTPBadRequest(
f"Invalid value for upload_limit: {upload_limit}, "
f"Invalid value for upload limit: {upload_limit}, "
f"must be integer or empty string.")

# The form is in MB, but the database field is in bytes.
upload_limit *= ONE_MB

if upload_limit < MAX_FILESIZE:
raise HTTPBadRequest(
f"Upload limit can not be less than the default limit of "
f"{MAX_FILESIZE / ONE_MB}MB.")

project.upload_limit = upload_limit

request.session.flash(
f"Successfully set the upload limit on {project.name!r} to "
f"{project.upload_limit!r}",
f"Successfully set the upload limit on {project.name!r}.",
queue="success",
)

Expand Down