diff --git a/.circleci/config.yml b/.circleci/config.yml
index c7e509566aa9c0e0e10654b6b9c6ff7a49a98a18..5d1ba29917f4ab96e42e18e0a1bb8b3e8bcf83cc 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -13,7 +13,6 @@ jobs:
PGUSER: root
DJANGO_SETTINGS_MODULE: opentech.settings.test
SEND_MESSAGES: false
- PROJECTS_ENABLED: true
- image: circleci/postgres:10.5
environment:
diff --git a/.travis.yml b/.travis.yml
index d2e18cf9fb607c1b6fb6e0318197728cc780363d..3a9983e245c3b1b21cb284eeedf40771ddd90916 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -21,7 +21,6 @@ env:
global:
- DJANGO_SETTINGS_MODULE=opentech.settings.test
- DATABASE_URL=postgres://postgres@localhost/test_db
- - PROJECTS_ENABLED=true
before_script:
# Create a database
diff --git a/opentech/apply/activity/templatetags/activity_tags.py b/opentech/apply/activity/templatetags/activity_tags.py
index 635d5754f3e5bb2561ce04ee8850ef5e9b3b8533..cef3242a63812435a66d4ae7fbfce504f9f9c957 100644
--- a/opentech/apply/activity/templatetags/activity_tags.py
+++ b/opentech/apply/activity/templatetags/activity_tags.py
@@ -12,7 +12,7 @@ register = template.Library()
@register.filter
def display_author(activity, user):
- if activity.source.user == user and isinstance(activity.related_object, Review):
+ if isinstance(activity.related_object, Review) and activity.source.user == user:
return 'Reviewer'
return activity.user
diff --git a/opentech/apply/activity/views.py b/opentech/apply/activity/views.py
index e9b664b9551d09e446cd7de5bf8dd73856fbbb07..c0edaf6b7abe782220d0d241b14e1e2eeddfb64d 100644
--- a/opentech/apply/activity/views.py
+++ b/opentech/apply/activity/views.py
@@ -15,16 +15,19 @@ class AllActivityContextMixin:
def get_context_data(self, **kwargs):
extra = {
'actions': Activity.actions.select_related(
- 'source_content_type',
'user',
+ ).prefetch_related(
+ 'source',
)[:ACTIVITY_LIMIT],
'comments': Activity.comments.select_related(
- 'source_content_type',
'user',
+ ).prefetch_related(
+ 'source',
)[:ACTIVITY_LIMIT],
'all_activity': Activity.objects.select_related(
- 'source_content_type',
'user',
+ ).prefetch_related(
+ 'source',
)[:ACTIVITY_LIMIT],
}
return super().get_context_data(**extra, **kwargs)
diff --git a/opentech/apply/determinations/tests/test_views.py b/opentech/apply/determinations/tests/test_views.py
index 9adff7a7f4139ab3f621dcd4a80078f5df312036..070836f7630d109033cdfcf6ba6d648924acbf07 100644
--- a/opentech/apply/determinations/tests/test_views.py
+++ b/opentech/apply/determinations/tests/test_views.py
@@ -2,7 +2,7 @@ import urllib
from django.contrib.messages.storage.fallback import FallbackStorage
from django.contrib.sessions.middleware import SessionMiddleware
-from django.test import RequestFactory
+from django.test import override_settings, RequestFactory
from django.urls import reverse_lazy
from opentech.apply.activity.models import Activity
@@ -88,6 +88,7 @@ class DeterminationFormTestCase(BaseViewTestCase):
response = self.get_page(submission, 'form')
self.assertNotContains(response, 'Update ')
+ @override_settings(PROJECTS_ENABLED=False)
def test_can_edit_draft_determination_if_not_lead(self):
submission = ApplicationSubmissionFactory(status='in_discussion')
determination = DeterminationFactory(submission=submission, author=self.user, accepted=True)
@@ -95,6 +96,13 @@ class DeterminationFormTestCase(BaseViewTestCase):
self.assertContains(response, 'Approved')
self.assertRedirects(response, self.absolute_url(submission.get_absolute_url()))
+ def test_can_edit_draft_determination_if_not_lead_with_projects(self):
+ submission = ApplicationSubmissionFactory(status='in_discussion')
+ determination = DeterminationFactory(submission=submission, author=self.user, accepted=True)
+ response = self.post_page(submission, {'data': 'value', 'outcome': determination.outcome}, 'form')
+ self.assertContains(response, 'Approved')
+ self.assertRedirects(response, self.absolute_url(submission.get_absolute_url()))
+
def test_sends_message_if_requires_more_info(self):
submission = ApplicationSubmissionFactory(status='in_discussion', lead=self.user)
determination = DeterminationFactory(submission=submission, author=self.user)
diff --git a/opentech/apply/determinations/views.py b/opentech/apply/determinations/views.py
index bee9a188bf0199a9e07d018d525b00d01385a9f9..5cb56920c88ccff6cad55bbf5f5c8916cd84c654 100644
--- a/opentech/apply/determinations/views.py
+++ b/opentech/apply/determinations/views.py
@@ -267,13 +267,14 @@ class DeterminationCreateOrUpdateView(CreateOrUpdateView):
if self.submission.accepted_for_funding:
project = Project.create_from_submission(self.submission)
- messenger(
- MESSAGES.CREATED_PROJECT,
- request=self.request,
- user=self.request.user,
- source=project,
- related=project.submission,
- )
+ if project:
+ messenger(
+ MESSAGES.CREATED_PROJECT,
+ request=self.request,
+ user=self.request.user,
+ source=project,
+ related=project.submission,
+ )
messenger(
MESSAGES.DETERMINATION_OUTCOME,
diff --git a/opentech/apply/funds/files.py b/opentech/apply/funds/files.py
new file mode 100644
index 0000000000000000000000000000000000000000..6f834375decea544af46540a69cd42b1370fae84
--- /dev/null
+++ b/opentech/apply/funds/files.py
@@ -0,0 +1,27 @@
+import os
+from django.urls import reverse
+
+from opentech.apply.stream_forms.files import StreamFieldFile
+
+
+def generate_submission_file_path(submission_id, field_id, file_name):
+ path = os.path.join('submission', str(submission_id), str(field_id))
+ if file_name.startswith(path):
+ return file_name
+
+ return os.path.join(path, file_name)
+
+
+class SubmissionStreamFileField(StreamFieldFile):
+ def generate_filename(self):
+ return generate_submission_file_path(self.instance.pk, self.field.id, self.name)
+
+ @property
+ def url(self):
+ return reverse(
+ 'apply:submissions:serve_private_media', kwargs={
+ 'pk': self.instance.pk,
+ 'field_id': self.field.id,
+ 'file_name': self.basename,
+ }
+ )
diff --git a/opentech/apply/funds/models/mixins.py b/opentech/apply/funds/models/mixins.py
index f4ecd6d4e823d02d600d7e828450f906ddddc0a0..aaaec9c3ad5c2056e02359527ff2e01695a259a6 100644
--- a/opentech/apply/funds/models/mixins.py
+++ b/opentech/apply/funds/models/mixins.py
@@ -1,7 +1,5 @@
-from django.conf import settings
from django.utils.text import mark_safe
from django.core.files import File
-from django.core.files.storage import get_storage_class
from opentech.apply.stream_forms.blocks import (
FileFieldBlock, FormFieldBlock, GroupToggleBlock, ImageFieldBlock, MultiFileFieldBlock
@@ -9,21 +7,13 @@ from opentech.apply.stream_forms.blocks import (
from opentech.apply.utils.blocks import SingleIncludeMixin
from opentech.apply.stream_forms.blocks import UploadableMediaBlock
-from opentech.apply.stream_forms.files import StreamFieldFile
+from opentech.apply.utils.storage import PrivateStorage
+from ..files import SubmissionStreamFileField
__all__ = ['AccessFormData']
-private_file_storage = getattr(settings, 'PRIVATE_FILE_STORAGE', None)
-submission_storage_class = get_storage_class(private_file_storage)
-
-if private_file_storage:
- submission_storage = submission_storage_class(is_submission=True)
-else:
- submission_storage = submission_storage_class()
-
-
class UnusedFieldException(Exception):
pass
@@ -34,8 +24,9 @@ class AccessFormData:
requires:
- form_data > jsonfield containing the submitted data
- form_fields > streamfield containing the original form fields
-
"""
+ stream_file_class = SubmissionStreamFileField
+ storage_class = PrivateStorage
@property
def raw_data(self):
@@ -54,46 +45,51 @@ class AccessFormData:
return data
@classmethod
- def stream_file(cls, file):
+ def stream_file(cls, instance, field, file):
if not file:
return []
- if isinstance(file, StreamFieldFile):
+ if isinstance(file, cls.stream_file_class):
return file
if isinstance(file, File):
- return StreamFieldFile(file, name=file.name, storage=submission_storage)
+ return cls.stream_file_class(instance, field, file, name=file.name, storage=cls.storage_class())
# This fixes a backwards compatibility issue with #507
# Once every application has been re-saved it should be possible to remove it
if 'path' in file:
file['filename'] = file['name']
file['name'] = file['path']
- return StreamFieldFile(None, name=file['name'], filename=file.get('filename'), storage=submission_storage)
+ return cls.stream_file_class(instance, field, None, name=file['name'], filename=file.get('filename'), storage=cls.storage_class())
@classmethod
- def process_file(cls, file):
+ def process_file(cls, instance, field, file):
if isinstance(file, list):
- return [cls.stream_file(f) for f in file]
+ return [cls.stream_file(instance, field, f) for f in file]
else:
- return cls.stream_file(file)
+ return cls.stream_file(instance, field, file)
@classmethod
def from_db(cls, db, field_names, values):
instance = super().from_db(db, field_names, values)
if 'form_data' in field_names:
# When the form_data is loaded from the DB deserialise it
- instance.form_data = cls.deserialised_data(instance.form_data, instance.form_fields)
+ instance.form_data = cls.deserialised_data(instance, instance.form_data, instance.form_fields)
return instance
@classmethod
- def deserialised_data(cls, data, form_fields):
+ def deserialised_data(cls, instance, data, form_fields):
# Converts the file dicts into actual file objects
data = data.copy()
- for field in form_fields.stream_data:
- block = form_fields.stream_block.child_blocks[field['type']]
+ # PERFORMANCE NOTE:
+ # Do not attempt to iterate over form_fields - that will fully instantiate the form_fields
+ # including any sub queries that they do
+ for i, field_data in enumerate(form_fields.stream_data):
+ block = form_fields.stream_block.child_blocks[field_data['type']]
if isinstance(block, UploadableMediaBlock):
- field_id = field.get('id')
- file = data.get(field_id, [])
- data[field_id] = cls.process_file(file)
+ field_id = field_data.get('id')
+ if field_id:
+ field = form_fields[i]
+ file = data.get(field_id, [])
+ data[field_id] = cls.process_file(instance, field, file)
return data
def get_definitive_id(self, id):
@@ -123,17 +119,25 @@ class AccessFormData:
yield field_id
@property
- def question_text_field_ids(self):
+ def file_field_ids(self):
for field_id, field in self.fields.items():
if isinstance(field.block, (FileFieldBlock, ImageFieldBlock, MultiFileFieldBlock)):
+ yield field_id
+
+ @property
+ def question_text_field_ids(self):
+ file_fields = list(self.file_field_ids)
+ for field_id, field in self.fields.items():
+ if field_id in file_fields:
pass
elif isinstance(field.block, FormFieldBlock):
yield field_id
@property
def first_group_question_text_field_ids(self):
+ file_fields = list(self.file_field_ids)
for field_id, field in self.fields.items():
- if isinstance(field.block, (FileFieldBlock, ImageFieldBlock, MultiFileFieldBlock)):
+ if field_id in file_fields:
continue
elif isinstance(field.block, GroupToggleBlock):
break
diff --git a/opentech/apply/funds/models/submissions.py b/opentech/apply/funds/models/submissions.py
index 6a5daa08fe7ecf87d436a23631afb91dba1ae2e3..008c3a5dc8bcec8f7758f29ba38bc3ba147af404 100644
--- a/opentech/apply/funds/models/submissions.py
+++ b/opentech/apply/funds/models/submissions.py
@@ -1,4 +1,3 @@
-import os
from functools import partialmethod
from django.conf import settings
@@ -201,8 +200,10 @@ class ApplicationSubmissionQueryset(JSONOrderable):
).prefetch_related(
Prefetch(
'assigned',
- queryset=AssignedReviewers.objects.reviewed().review_order().prefetch_related(
- Prefetch('opinions', queryset=ReviewOpinion.objects.select_related('author__reviewer'))
+ queryset=AssignedReviewers.objects.reviewed().review_order().select_related(
+ 'reviewer',
+ ).prefetch_related(
+ Prefetch('review__opinions', queryset=ReviewOpinion.objects.select_related('author')),
),
to_attr='has_reviewed'
),
@@ -211,7 +212,6 @@ class ApplicationSubmissionQueryset(JSONOrderable):
queryset=AssignedReviewers.objects.not_reviewed().staff(),
to_attr='hasnt_reviewed'
)
-
).select_related(
'page',
'round',
@@ -219,6 +219,7 @@ class ApplicationSubmissionQueryset(JSONOrderable):
'user',
'previous__page',
'previous__round',
+ 'previous__lead',
)
@@ -538,7 +539,7 @@ class ApplicationSubmission(
def from_draft(self):
self.is_draft = True
- self.form_data = self.deserialised_data(self.draft_revision.form_data, self.form_fields)
+ self.form_data = self.deserialised_data(self, self.draft_revision.form_data, self.form_fields)
return self
def create_revision(self, draft=False, force=False, by=None, **kwargs):
@@ -587,13 +588,12 @@ class ApplicationSubmission(
def process_file_data(self, data):
for field in self.form_fields:
if isinstance(field.block, UploadableMediaBlock):
- file = self.process_file(data.get(field.id, []))
- folder = os.path.join('submission', str(self.id), field.id)
+ file = self.process_file(self, field, data.get(field.id, []))
try:
- file.save(folder)
+ file.save()
except AttributeError:
for f in file:
- f.save(folder)
+ f.save()
self.form_data[field.id] = file
def save(self, *args, update_fields=list(), **kwargs):
diff --git a/opentech/apply/funds/templates/funds/applicationsubmission_admin_detail.html b/opentech/apply/funds/templates/funds/applicationsubmission_admin_detail.html
index 2216c7cfc8e3a3a7ce11c3a373583fb3ce9ec754..fbb20c4b1e135d2cb2e90e50756d418ecaf79bbf 100644
--- a/opentech/apply/funds/templates/funds/applicationsubmission_admin_detail.html
+++ b/opentech/apply/funds/templates/funds/applicationsubmission_admin_detail.html
@@ -17,7 +17,7 @@
{% include "funds/includes/update_lead_form.html" %}
{% include "funds/includes/update_reviewer_form.html" %}
{% include "funds/includes/update_partner_form.html" %}
- {% if settings.PROJECTS_ENABLED %}
+ {% if PROJECTS_ENABLED %}
{% include "funds/includes/create_project_form.html" %}
{% endif %}
{% include "funds/includes/update_meta_categories_form.html" %}
diff --git a/opentech/apply/funds/templates/funds/applicationsubmission_detail.html b/opentech/apply/funds/templates/funds/applicationsubmission_detail.html
index 077f4ae6edf893c78a9d506c59426a52dcfaaef3..7ac1ea17de1c00aaa804f633313a88bc0391d615 100644
--- a/opentech/apply/funds/templates/funds/applicationsubmission_detail.html
+++ b/opentech/apply/funds/templates/funds/applicationsubmission_detail.html
@@ -99,7 +99,7 @@
{% endblock %}
{% endif %}
- {% if object.project and settings.PROJECTS_ENABLED %}
+ {% if object.project and PROJECTS_ENABLED %}
{% include 'funds/includes/project_block.html' %}
{% endif %}
diff --git a/opentech/apply/funds/templates/funds/submissions.html b/opentech/apply/funds/templates/funds/submissions.html
index ca70880476eaac1808566d08c7b7a7c21e54ef21..353450fa78d785a9339a12023d7eb3f69c5f3736 100644
--- a/opentech/apply/funds/templates/funds/submissions.html
+++ b/opentech/apply/funds/templates/funds/submissions.html
@@ -14,11 +14,6 @@
</div>
<div class="wrapper wrapper--large wrapper--inner-space-medium">
-
- {% if closed_rounds or open_rounds %}
- {% include "funds/includes/round-block.html" with closed_rounds=closed_rounds open_rounds=open_rounds title=rounds_title %}
- {% endif %}
-
{% block table %}
{{ block.super }}
{% endblock %}
diff --git a/opentech/apply/funds/tests/test_views.py b/opentech/apply/funds/tests/test_views.py
index 55b5d4119bbf414e13fe80d8921681f84fc2e49f..2d7918c3e7b4bc4bad44d1d73c709c210195e22d 100644
--- a/opentech/apply/funds/tests/test_views.py
+++ b/opentech/apply/funds/tests/test_views.py
@@ -1,9 +1,11 @@
from datetime import timedelta
import json
+from django.contrib.auth.models import AnonymousUser
from django.core.exceptions import PermissionDenied
from django.http import Http404
from django.test import RequestFactory, TestCase
+from django.urls import reverse
from django.utils import timezone
from django.utils.text import slugify
@@ -694,3 +696,56 @@ class TestSubmissionDetailSimplifiedView(TestCase):
ProjectFactory(submission=submission)
response = SubmissionDetailSimplifiedView.as_view()(request, pk=submission.pk)
self.assertEqual(response.status_code, 200)
+
+
+class BaseSubmissionFileViewTestCase(BaseViewTestCase):
+ url_name = 'funds:submissions:{}'
+ base_view_name = 'serve_private_media'
+
+ def get_kwargs(self, instance):
+ document_fields = list(instance.file_field_ids)
+ field_id = document_fields[0]
+ document = instance.data(field_id)
+ return {
+ 'pk': instance.pk,
+ 'field_id': field_id,
+ 'file_name': document.basename,
+ }
+
+
+class TestStaffSubmissionFileView(BaseSubmissionFileViewTestCase):
+ user_factory = StaffFactory
+
+ def test_staff_can_access(self):
+ submission = ApplicationSubmissionFactory()
+ response = self.get_page(submission)
+ self.assertEqual(response.status_code, 200)
+ self.assertEqual(response.redirect_chain, [])
+
+
+class TestUserSubmissionFileView(BaseSubmissionFileViewTestCase):
+ user_factory = ApplicantFactory
+
+ def test_owner_can_access(self):
+ submission = ApplicationSubmissionFactory(user=self.user)
+ response = self.get_page(submission)
+ self.assertEqual(response.status_code, 200)
+ self.assertEqual(response.redirect_chain, [])
+
+ def test_user_can_not_access(self):
+ submission = ApplicationSubmissionFactory()
+ response = self.get_page(submission)
+ self.assertEqual(response.status_code, 403)
+ self.assertEqual(response.redirect_chain, [])
+
+
+class TestAnonSubmissionFileView(BaseSubmissionFileViewTestCase):
+ user_factory = AnonymousUser
+
+ def test_anonymous_can_not_access(self):
+ submission = ApplicationSubmissionFactory()
+ response = self.get_page(submission)
+ self.assertEqual(response.status_code, 200)
+ self.assertEqual(len(response.redirect_chain), 2)
+ for path, _ in response.redirect_chain:
+ self.assertIn(reverse('users_public:login'), path)
diff --git a/opentech/apply/funds/urls.py b/opentech/apply/funds/urls.py
index 3cb33249fa79b4baa48d9a365a3a1a9a6c4241a7..706fddede0349dcf01515894b85a659f4e23e46e 100644
--- a/opentech/apply/funds/urls.py
+++ b/opentech/apply/funds/urls.py
@@ -1,6 +1,6 @@
from django.urls import include, path
-from opentech.apply.projects.urls import urlpatterns as projects_urls
+from opentech.apply.projects import urls as projects_urls
from .views import (
RevisionCompareView,
@@ -40,16 +40,16 @@ app_name = 'funds'
submission_urls = ([
path('', SubmissionOverviewView.as_view(), name="overview"),
path('all/', SubmissionListView.as_view(), name="list"),
- path(
- 'documents/submission/<int:submission_id>/<uuid:field_id>/<str:file_name>/',
- SubmissionPrivateMediaView.as_view(), name='serve_private_media'
- ),
path('<int:pk>/', include([
path('', SubmissionDetailView.as_view(), name="detail"),
path('edit/', SubmissionEditView.as_view(), name="edit"),
path('sealed/', SubmissionSealedView.as_view(), name="sealed"),
path('simplified/', SubmissionDetailSimplifiedView.as_view(), name="simplified"),
path('delete/', SubmissionDeleteView.as_view(), name="delete"),
+ path(
+ 'documents/<uuid:field_id>/<str:file_name>',
+ SubmissionPrivateMediaView.as_view(), name='serve_private_media'
+ ),
])),
path('<int:submission_pk>/', include([
path('', include('opentech.apply.review.urls', namespace="reviews")),
@@ -85,6 +85,6 @@ rounds_urls = ([
urlpatterns = [
path('submissions/', include(submission_urls)),
path('rounds/', include(rounds_urls)),
- path('projects/', include((projects_urls, 'projects'))),
+ path('projects/', include(projects_urls)),
path('api/', include(api_urls)),
]
diff --git a/opentech/apply/funds/views.py b/opentech/apply/funds/views.py
index 2553e7a60a655c622b91d98f98253a49ac13ed18..43922b73365c26c2324bb654821934e8999c9786 100644
--- a/opentech/apply/funds/views.py
+++ b/opentech/apply/funds/views.py
@@ -1,22 +1,17 @@
-import mimetypes
from copy import copy
-from wsgiref.util import FileWrapper
-from django.conf import settings
from django.contrib.auth.decorators import login_required, permission_required
from django.contrib.auth.mixins import UserPassesTestMixin
-from django.contrib.auth.views import redirect_to_login
from django.contrib import messages
from django.core.exceptions import PermissionDenied
-from django.core.files.storage import get_storage_class
from django.db.models import Count, F, Q
-from django.http import HttpResponseRedirect, Http404, StreamingHttpResponse
+from django.http import HttpResponseRedirect, Http404
from django.shortcuts import get_object_or_404
from django.urls import reverse_lazy
from django.utils.decorators import method_decorator
from django.utils.text import mark_safe
from django.utils.translation import ugettext_lazy as _
-from django.views.generic import CreateView, DetailView, FormView, ListView, UpdateView, DeleteView, View
+from django.views.generic import CreateView, DetailView, FormView, ListView, UpdateView, DeleteView
from django_filters.views import FilterView
from django_tables2.views import SingleTableMixin
@@ -35,9 +30,11 @@ from opentech.apply.projects.forms import CreateProjectForm
from opentech.apply.projects.models import Project
from opentech.apply.review.views import ReviewContextMixin
from opentech.apply.users.decorators import staff_required
+from opentech.apply.utils.storage import PrivateMediaView
from opentech.apply.utils.views import DelegateableListView, DelegateableView, ViewDispatcher
from .differ import compare
+from .files import generate_submission_file_path
from .forms import (
BatchUpdateSubmissionLeadForm,
BatchUpdateReviewersForm,
@@ -56,6 +53,7 @@ from .models import (
RoundBase,
LabBase
)
+from .permissions import is_user_has_access_to_view_submission
from .tables import (
AdminSubmissionsTable,
ReviewerSubmissionsTable,
@@ -66,9 +64,6 @@ from .tables import (
SummarySubmissionsTable,
)
from .workflow import INITIAL_STATE, STAGE_CHANGE_ACTIONS, PHASES_MAPPING, review_statuses
-from .permissions import is_user_has_access_to_view_submission
-
-submission_storage = get_storage_class(getattr(settings, 'PRIVATE_FILE_STORAGE', None))()
class BaseAdminSubmissionsTable(SingleTableMixin, FilterView):
@@ -917,48 +912,23 @@ class SubmissionDeleteView(DeleteView):
return response
-class SubmissionPrivateMediaView(UserPassesTestMixin, View):
+@method_decorator(login_required, name='dispatch')
+class SubmissionPrivateMediaView(UserPassesTestMixin, PrivateMediaView):
+ raise_exception = True
+
+ def dispatch(self, *args, **kwargs):
+ submission_pk = self.kwargs['pk']
+ self.submission = get_object_or_404(ApplicationSubmission, pk=submission_pk)
+ return super().dispatch(*args, **kwargs)
- def get(self, *args, **kwargs):
- submission_id = kwargs['submission_id']
+ def get_media(self, *args, **kwargs):
field_id = kwargs['field_id']
file_name = kwargs['file_name']
- file_name_with_path = f'submission/{submission_id}/{field_id}/{file_name}'
-
- submission_file = submission_storage.open(file_name_with_path)
- wrapper = FileWrapper(submission_file)
- encoding_map = {
- 'bzip2': 'application/x-bzip',
- 'gzip': 'application/gzip',
- 'xz': 'application/x-xz',
- }
- content_type, encoding = mimetypes.guess_type(file_name)
- # Encoding isn't set to prevent browsers from automatically uncompressing files.
- content_type = encoding_map.get(encoding, content_type)
- content_type = content_type or 'application/octet-stream'
- # From Django 2.1, we can use FileResponse instead of StreamingHttpResponse
- response = StreamingHttpResponse(wrapper, content_type=content_type)
-
- response['Content-Disposition'] = f'filename={file_name}'
- response['Content-Length'] = submission_file.size
-
- return response
+ path_to_file = generate_submission_file_path(self.submission.pk, field_id, file_name)
+ return self.storage.open(path_to_file)
def test_func(self):
- submission_id = self.kwargs['submission_id']
- submission = get_object_or_404(ApplicationSubmission, id=submission_id)
-
- return is_user_has_access_to_view_submission(self.request.user, submission)
-
- def handle_no_permission(self):
- # This method can be removed after upgrading Django to 2.1
- # https://github.com/django/django/commit/9b1125bfc7e2dc747128e6e7e8a2259ff1a7d39f
- # In older versions, authenticated users who lacked permissions were
- # redirected to the login page (which resulted in a loop) instead of
- # receiving an HTTP 403 Forbidden response.
- if self.raise_exception or self.request.user.is_authenticated:
- raise PermissionDenied(self.get_permission_denied_message())
- return redirect_to_login(self.request.get_full_path(), self.get_login_url(), self.get_redirect_field_name())
+ return is_user_has_access_to_view_submission(self.request.user, self.submission)
@method_decorator(staff_required, name='dispatch')
diff --git a/opentech/apply/projects/context_processors.py b/opentech/apply/projects/context_processors.py
new file mode 100644
index 0000000000000000000000000000000000000000..b04840afcfa1bc16f139ec39dcac52e4da541387
--- /dev/null
+++ b/opentech/apply/projects/context_processors.py
@@ -0,0 +1,5 @@
+from django.conf import settings
+
+
+def projects_enabled(request):
+ return {'PROJECTS_ENABLED': settings.PROJECTS_ENABLED}
diff --git a/opentech/apply/projects/migrations/0012_adjust_storage_class.py b/opentech/apply/projects/migrations/0012_adjust_storage_class.py
new file mode 100644
index 0000000000000000000000000000000000000000..7834f8d2758b480be70a754dab1375eea9b50bb1
--- /dev/null
+++ b/opentech/apply/projects/migrations/0012_adjust_storage_class.py
@@ -0,0 +1,20 @@
+# Generated by Django 2.0.13 on 2019-08-10 09:54
+
+import django.core.files.storage
+from django.db import migrations, models
+import opentech.apply.projects.models
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ('application_projects', '0011_add_packet_file'),
+ ]
+
+ operations = [
+ migrations.AlterField(
+ model_name='packetfile',
+ name='document',
+ field=models.FileField(storage=django.core.files.storage.FileSystemStorage(), upload_to=opentech.apply.projects.models.document_path),
+ ),
+ ]
diff --git a/opentech/apply/projects/models.py b/opentech/apply/projects/models.py
index e7c1dd8a939002928796dbf1989d93c5d303c709..8e014e08696ee5987d67fdcee95175a5fff3533b 100644
--- a/opentech/apply/projects/models.py
+++ b/opentech/apply/projects/models.py
@@ -12,6 +12,9 @@ from django.db import models
from django.urls import reverse
from django.utils.translation import ugettext as _
+from opentech.apply.utils.storage import PrivateStorage
+
+
logger = logging.getLogger(__name__)
@@ -55,7 +58,7 @@ class PacketFile(models.Model):
project = models.ForeignKey("Project", on_delete=models.CASCADE, related_name="packet_files")
title = models.TextField()
- document = models.FileField(upload_to=document_path)
+ document = models.FileField(upload_to=document_path, storage=PrivateStorage())
def __str__(self):
return f'Project file: {self.title}'
@@ -183,7 +186,9 @@ class Project(models.Model):
return self.lead and not self.is_locked
def get_absolute_url(self):
- return reverse('apply:projects:detail', args=[self.id])
+ if settings.PROJECTS_ENABLED:
+ return reverse('apply:projects:detail', args=[self.id])
+ return '#'
@property
def can_make_approval(self):
diff --git a/opentech/apply/projects/templates/application_projects/includes/supporting_documents.html b/opentech/apply/projects/templates/application_projects/includes/supporting_documents.html
index f0da56dcfbf0ada7d37b5813c628903080fdb6f4..8e3e1acc21c727fc912a6082bbc4aa8143343eba 100644
--- a/opentech/apply/projects/templates/application_projects/includes/supporting_documents.html
+++ b/opentech/apply/projects/templates/application_projects/includes/supporting_documents.html
@@ -75,7 +75,7 @@
<p class="docs-block__document-info">{{ document.category.name }}</p>
</div>
<div class="docs-block__document-inner">
- <a class="docs-block__document-link" href="{{ document.document.url }}">Download</a>
+ <a class="docs-block__document-link" href="{% url 'apply:projects:document' pk=object.pk file_pk=document.pk %}">Download</a>
<form method="POST" id="{{ remove_document_form.name }}">
{% csrf_token %}
{{ document.get_remove_form }}
diff --git a/opentech/apply/projects/tests/test_views.py b/opentech/apply/projects/tests/test_views.py
index ac577d5e03b1b70fb7c03a42ec1c851f646fe7f1..7f54fcc2678b2a272a423261e51ca336986007cb 100644
--- a/opentech/apply/projects/tests/test_views.py
+++ b/opentech/apply/projects/tests/test_views.py
@@ -1,6 +1,8 @@
from io import BytesIO
+from django.contrib.auth.models import AnonymousUser
from django.test import TestCase
+from django.urls import reverse
from opentech.apply.funds.tests.factories import LabSubmissionFactory
from opentech.apply.users.tests.factories import (
@@ -550,3 +552,52 @@ class TestApproveContractView(BaseViewTestCase):
messages = list(response.context['messages'])
self.assertEqual(len(messages), 1)
+
+
+class BasePacketFileViewTestCase(BaseViewTestCase):
+ url_name = 'funds:projects:{}'
+ base_view_name = 'document'
+
+ def get_kwargs(self, instance):
+ return {
+ 'pk': instance.project.pk,
+ 'file_pk': instance.id,
+ }
+
+
+class TestStaffPacketView(BasePacketFileViewTestCase):
+ user_factory = StaffFactory
+
+ def test_staff_can_access(self):
+ document = PacketFileFactory()
+ response = self.get_page(document)
+ self.assertEqual(response.status_code, 200)
+ self.assertEqual(response.redirect_chain, [])
+
+
+class TestUserPacketView(BasePacketFileViewTestCase):
+ user_factory = ApplicantFactory
+
+ def test_owner_can_access(self):
+ document = PacketFileFactory(project__user=self.user)
+ response = self.get_page(document)
+ self.assertEqual(response.status_code, 200)
+ self.assertEqual(response.redirect_chain, [])
+
+ def test_user_can_not_access(self):
+ document = PacketFileFactory()
+ response = self.get_page(document)
+ self.assertEqual(response.status_code, 403)
+ self.assertEqual(response.redirect_chain, [])
+
+
+class TestAnonPacketView(BasePacketFileViewTestCase):
+ user_factory = AnonymousUser
+
+ def test_anonymous_can_not_access(self):
+ document = PacketFileFactory()
+ response = self.get_page(document)
+ self.assertEqual(response.status_code, 200)
+ self.assertEqual(len(response.redirect_chain), 2)
+ for path, _ in response.redirect_chain:
+ self.assertIn(reverse('users_public:login'), path)
diff --git a/opentech/apply/projects/urls.py b/opentech/apply/projects/urls.py
index cad1a6080d33c000514c1013008f3c19550a9381..1b2f625fda11c5b784ba16ab0267023f4b0373c4 100644
--- a/opentech/apply/projects/urls.py
+++ b/opentech/apply/projects/urls.py
@@ -1,7 +1,15 @@
from django.conf import settings
from django.urls import include, path
-from .views import ApproveContractView, ProjectDetailView, ProjectEditView
+from .views import (
+ AppriveContractView,
+ ProjectDetailView,
+ ProjectEditView,
+ ProjectPrivateMediaView
+)
+
+
+app_name = 'projects'
urlpatterns = []
@@ -15,5 +23,6 @@ if settings.PROJECTS_ENABLED:
ApproveContractView.as_view(),
name="approve-contract",
),
+ path('documents/<int:file_pk>/', ProjectPrivateMediaView.as_view(), name="document"),
])),
]
diff --git a/opentech/apply/projects/views.py b/opentech/apply/projects/views.py
index 007ba010bda5322ff74733452c378451ce4716c2..b7d5f5fde838c8c13f84fca4af41ec7b274318cd 100644
--- a/opentech/apply/projects/views.py
+++ b/opentech/apply/projects/views.py
@@ -2,8 +2,10 @@ from copy import copy
from django.contrib import messages
from django.contrib.auth.decorators import login_required
+from django.contrib.auth.mixins import UserPassesTestMixin
from django.core.exceptions import PermissionDenied
from django.db import transaction
+from django.http import Http404
from django.shortcuts import get_object_or_404, redirect
from django.utils.decorators import method_decorator
from django.utils.translation import ugettext_lazy as _
@@ -12,10 +14,11 @@ from django.views.generic import CreateView, DetailView, FormView, UpdateView
from opentech.apply.activity.messaging import MESSAGES, messenger
from opentech.apply.activity.views import ActivityContextMixin, CommentFormView
from opentech.apply.users.decorators import staff_required
+from opentech.apply.utils.storage import PrivateMediaView
from opentech.apply.utils.views import (
DelegateableView,
DelegatedViewMixin,
- ViewDispatcher
+ ViewDispatcher,
)
from .forms import (
@@ -314,6 +317,31 @@ class ApplicantProjectDetailView(ActivityContextMixin, DelegateableView, Contrac
return super().dispatch(request, *args, **kwargs)
+@method_decorator(login_required, name='dispatch')
+class ProjectPrivateMediaView(UserPassesTestMixin, PrivateMediaView):
+ raise_exception = True
+
+ def dispatch(self, *args, **kwargs):
+ project_pk = self.kwargs['pk']
+ self.project = get_object_or_404(Project, pk=project_pk)
+ return super().dispatch(*args, **kwargs)
+
+ def get_media(self, *args, **kwargs):
+ document = PacketFile.objects.get(pk=kwargs['file_pk'])
+ if document.project != self.project:
+ raise Http404
+ return document.document
+
+ def test_func(self):
+ if self.request.user.is_apply_staff:
+ return True
+
+ if self.request.user == self.project.user:
+ return True
+
+ return False
+
+
class ProjectDetailView(ViewDispatcher):
admin_view = AdminProjectDetailView
applicant_view = ApplicantProjectDetailView
diff --git a/opentech/apply/stream_forms/files.py b/opentech/apply/stream_forms/files.py
index 8fdf2febd2906c7baf65f19efe2f9dcb45db596e..214b12c2623b31eb94be9113520503b8809559fb 100644
--- a/opentech/apply/stream_forms/files.py
+++ b/opentech/apply/stream_forms/files.py
@@ -16,15 +16,28 @@ class StreamFieldDataEncoder(DjangoJSONEncoder):
class StreamFieldFile(File):
- def __init__(self, *args, filename=None, storage=default_storage, **kwargs):
+ """
+ Attempts to mimic the behaviour of the bound fields in django models
+
+ see django.db.models.fields.files for the inspiration
+ """
+ def __init__(self, instance, field, *args, filename=None, storage=default_storage, **kwargs):
super().__init__(*args, **kwargs)
+ # Field is the wagtail field that the file was uploaded to
+ self.field = field
+ # Instance is the parent model object that created this file object
+ self.instance = instance
self.storage = storage
- self.filename = filename or os.path.basename(self.name)
+ self.filename = filename or self.basename
self._committed = False
def __str__(self):
return self.filename
+ @property
+ def basename(self):
+ return os.path.basename(self.name)
+
def __eq__(self, other):
if isinstance(other, File):
return self.filename == other.filename and self.size == other.size
@@ -79,10 +92,11 @@ class StreamFieldFile(File):
self.file.open(mode)
return self
- def save(self, folder):
- name = self.name
- if not name.startswith(folder):
- name = os.path.join(folder, name)
+ def generate_filename(self):
+ return self.name
+
+ def save(self):
+ name = self.generate_filename()
name = self.storage.generate_filename(name)
if not self._committed:
self.name = self.storage.save(name, self.file)
diff --git a/opentech/apply/stream_forms/tests.py b/opentech/apply/stream_forms/tests.py
index a8d37d02de09a7a503431f897d4c7299db54e86a..2a861023ffd8f1b6bb9d7173b2b00c0ed85a0a48 100644
--- a/opentech/apply/stream_forms/tests.py
+++ b/opentech/apply/stream_forms/tests.py
@@ -12,7 +12,7 @@ fake = Faker()
def make_files(number):
file_names = [f'{fake.word()}_{i}.pdf' for i in range(number)]
files = [
- StreamFieldFile(SimpleUploadedFile(name, b'Some File Content'), filename=name)
+ StreamFieldFile('fake', 'uuid', SimpleUploadedFile(name, b'Some File Content'), filename=name)
for name in file_names
]
return files
diff --git a/opentech/apply/utils/image.py b/opentech/apply/utils/image.py
index 5086fb3f658fbab73318be5b8804b40dacd9e3df..93cfdc5fb13089cedc9ea822601c2a3dcf0cef31 100644
--- a/opentech/apply/utils/image.py
+++ b/opentech/apply/utils/image.py
@@ -1,12 +1,22 @@
+from django.core.cache import cache
from django.urls import reverse
from django.utils.html import format_html
+def image_url_cache_key(image_id, spec):
+ return f'image_url_cache_{image_id}_{spec}'
+
+
def generate_image_url(image, filter_spec):
+ cache_key = image_url_cache_key(image.id, filter_spec)
+ url = cache.get(cache_key)
+ if url:
+ return url
from wagtail.images.views.serve import generate_signature
signature = generate_signature(image.id, filter_spec)
url = reverse('wagtailimages_serve', args=(signature, image.id, filter_spec))
url += image.file.name[len('original_images/'):]
+ cache.set(cache_key, url)
return url
diff --git a/opentech/apply/utils/storage.py b/opentech/apply/utils/storage.py
new file mode 100644
index 0000000000000000000000000000000000000000..5e257d3598f360b0607cbcb718e8df4eed976cd3
--- /dev/null
+++ b/opentech/apply/utils/storage.py
@@ -0,0 +1,41 @@
+import mimetypes
+import os
+from wsgiref.util import FileWrapper
+
+from django.conf import settings
+from django.core.files.storage import get_storage_class
+from django.http import StreamingHttpResponse
+from django.views.generic import View
+
+
+private_file_storage = getattr(settings, 'PRIVATE_FILE_STORAGE', None)
+PrivateStorage = get_storage_class(private_file_storage)
+
+
+class PrivateMediaView(View):
+ storage = PrivateStorage()
+
+ def get_media(self, *args, **kwargs):
+ # Convert the URL request to a path which the storage can use to find the file
+ raise NotImplementedError()
+
+ def get(self, *args, **kwargs):
+ file_to_serve = self.get_media(*args, **kwargs)
+ wrapper = FileWrapper(file_to_serve)
+ encoding_map = {
+ 'bzip2': 'application/x-bzip',
+ 'gzip': 'application/gzip',
+ 'xz': 'application/x-xz',
+ }
+ file_name = os.path.basename(file_to_serve.name)
+ content_type, encoding = mimetypes.guess_type(file_name)
+ # Encoding isn't set to prevent browsers from automatically uncompressing files.
+ content_type = encoding_map.get(encoding, content_type)
+ content_type = content_type or 'application/octet-stream'
+ # From Django 2.1, we can use FileResponse instead of StreamingHttpResponse
+ response = StreamingHttpResponse(wrapper, content_type=content_type)
+
+ response['Content-Disposition'] = f'filename={file_name}'
+ response['Content-Length'] = file_to_serve.size
+
+ return response
diff --git a/opentech/apply/utils/testing/tests.py b/opentech/apply/utils/testing/tests.py
index bba277b3d117141767a458d942b252b9a2cd63b6..2940aceb26b441b25726c68255964da9d9f1db54 100644
--- a/opentech/apply/utils/testing/tests.py
+++ b/opentech/apply/utils/testing/tests.py
@@ -29,7 +29,8 @@ class BaseViewTestCase(TestCase):
super().setUpTestData()
def setUp(self):
- self.client.force_login(self.user)
+ if not self.user.is_anonymous:
+ self.client.force_login(self.user)
self.factory = RequestFactory()
def get_kwargs(self, instance):
diff --git a/opentech/settings/base.py b/opentech/settings/base.py
index 6fa7c5852c2f38ecd3906c7fc7d1e5c2bd2a0775..dc741ff71b615039920b2de262272f8361154451 100644
--- a/opentech/settings/base.py
+++ b/opentech/settings/base.py
@@ -182,6 +182,7 @@ TEMPLATES = [
'opentech.public.utils.context_processors.global_vars',
'social_django.context_processors.backends',
'social_django.context_processors.login_redirect',
+ 'opentech.apply.projects.context_processors.projects_enabled',
],
},
},
diff --git a/opentech/settings/dev.py b/opentech/settings/dev.py
index b6fd212bc1fa13d4ba601106914e106ea8906468..01e2301e5edaf0e08dfaff107651eaab54c39f06 100644
--- a/opentech/settings/dev.py
+++ b/opentech/settings/dev.py
@@ -34,6 +34,8 @@ try:
except ImportError:
pass
+PROJECTS_ENABLED = True
+
# We add these here so they can react on settings made in local.py.
# E-mail to local files.
diff --git a/opentech/settings/test.py b/opentech/settings/test.py
index 11a71f78e5248d8568b15eaf0f8d0ca00ff70acd..3d6102ba8bff38edc37cef28a368432054de92c1 100644
--- a/opentech/settings/test.py
+++ b/opentech/settings/test.py
@@ -8,6 +8,8 @@ from .base import * # noqa
SECRET_KEY = 'NOT A SECRET'
+PROJECTS_ENABLED = True
+
# Need this to ensure white noise doesn't kill the speed of testing
# http://whitenoise.evans.io/en/latest/django.html#whitenoise-makes-my-tests-run-slow
WHITENOISE_AUTOREFRESH = True
diff --git a/opentech/storage_backends.py b/opentech/storage_backends.py
index e7f08555901397cda8232239a2bed3d01b289432..4dba8b95e81bd05ce6dc79e6a4932e2e91d68428 100644
--- a/opentech/storage_backends.py
+++ b/opentech/storage_backends.py
@@ -1,7 +1,6 @@
from urllib import parse
from django.conf import settings
-from django.urls import reverse
from django.utils.encoding import filepath_to_uri
from storages.backends.s3boto3 import S3Boto3Storage
@@ -29,21 +28,8 @@ class PrivateMediaStorage(S3Boto3Storage):
file_overwrite = False
querystring_auth = True
url_protocol = 'https:'
- is_submission = False
def url(self, name, parameters=None, expire=None):
- if self.is_submission:
- try:
- name_parts = name.split('/')
- return reverse(
- 'apply:submissions:serve_private_media', kwargs={
- 'submission_id': name_parts[1], 'field_id': name_parts[2],
- 'file_name': name_parts[3]
- }
- )
- except IndexError:
- pass
-
url = super().url(name, parameters, expire)
if hasattr(settings, 'AWS_PRIVATE_CUSTOM_DOMAIN'):