Skip to content
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

feat(copy project): Add copy project functionality and refactor project hierarchy validation #1133

Open
wants to merge 6 commits into
base: 2.3.0
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 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
24 changes: 24 additions & 0 deletions rdmo/projects/admin.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from django import forms
from django.contrib import admin
from django.db.models import Prefetch
from django.urls import reverse
Expand All @@ -15,12 +16,35 @@
Snapshot,
Value,
)
from .validators import ProjectParentValidator


class ProjectAdminForm(forms.ModelForm):

class Meta:
model = Project
fields = [
'parent',
'site',
'title',
'description',
'catalog',
'views'
]


def clean(self):
super().clean()
ProjectParentValidator(self.instance)(self.cleaned_data)


@admin.register(Project)
class ProjectAdmin(admin.ModelAdmin):
form = ProjectAdminForm

search_fields = ('title', 'user__username')
list_display = ('title', 'owners', 'updated', 'created')
readonly_fields = ('progress_count', 'progress_total')

def get_queryset(self, request):
return Project.objects.prefetch_related(
Expand Down
28 changes: 17 additions & 11 deletions rdmo/projects/assets/js/projects/components/main/Projects.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,21 +140,27 @@ const Projects = ({ config, configActions, currentUserObject, projectsActions, p

return (
<div className="icon-container">
{(isProjectManager || isProjectOwner || isManager) &&
<Link
href={`${rowUrl}/update/`}
className="element-link fa fa-pencil"
title={row.title}
onClick={() => window.location.href = `${rowUrl}/update/${params}`}
href={`${rowUrl}/copy/`}
className="fa fa-copy"
title={gettext('Copy project')}
onClick={() => window.location.href = `${rowUrl}/copy/${params}`}
/>
{(isProjectManager || isProjectOwner || isManager) &&
<Link
href={`${rowUrl}/update/`}
className="fa fa-pencil"
title={row.title}
onClick={() => window.location.href = `${rowUrl}/update/${params}`}
/>
}
{(isProjectOwner || isManager) &&
<Link
href={`${rowUrl}/delete/`}
className="element-link fa fa-trash"
title={row.title}
onClick={() => window.location.href = `${rowUrl}/delete/${params}`}
/>
<Link
href={`${rowUrl}/delete/`}
className="fa fa-trash"
title={row.title}
onClick={() => window.location.href = `${rowUrl}/delete/${params}`}
/>
}
</div>
)
Expand Down
1 change: 1 addition & 0 deletions rdmo/projects/assets/scss/projects.scss
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ a.disabled {
display: flex;
gap: 5px;
margin-bottom: 10px;
justify-content: flex-end;
}

.projects {
Expand Down
12 changes: 12 additions & 0 deletions rdmo/projects/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

from .constants import ROLE_CHOICES
from .models import Integration, IntegrationOption, Invite, Membership, Project, Snapshot
from .validators import ProjectParentValidator


class CatalogChoiceField(forms.ModelChoiceField):
Expand Down Expand Up @@ -53,6 +54,8 @@ class ProjectForm(forms.ModelForm):
use_required_attribute = False

def __init__(self, *args, **kwargs):
self.copy = kwargs.pop('copy', False)

catalogs = kwargs.pop('catalogs')
projects = kwargs.pop('projects')
super().__init__(*args, **kwargs)
Expand All @@ -66,6 +69,11 @@ def __init__(self, *args, **kwargs):
if settings.NESTED_PROJECTS:
self.fields['parent'].queryset = projects

def clean(self):
if not self.copy:
ProjectParentValidator(self.instance)(self.cleaned_data)
super().clean()

class Meta:
model = Project

Expand Down Expand Up @@ -160,6 +168,10 @@ def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self.fields['parent'].queryset = projects

def clean(self):
ProjectParentValidator(self.instance)(self.cleaned_data)
super().clean()

class Meta:
model = Project
fields = ('parent', )
Expand Down
10 changes: 5 additions & 5 deletions rdmo/projects/models/project.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
from django.conf import settings
from django.contrib.sites.models import Site
from django.core.exceptions import ValidationError
from django.db import models
from django.db.models.signals import pre_delete
from django.dispatch import receiver
Expand Down Expand Up @@ -88,11 +87,12 @@ def __str__(self):
def get_absolute_url(self):
return reverse('project', kwargs={'pk': self.pk})

def clean(self):
def save(self, *args, **kwargs):
# ensure that the project hierarchy is not disturbed
if self.id and self.parent in self.get_descendants(include_self=True):
raise ValidationError({
'parent': [_('A project may not be moved to be a child of itself or one of its descendants.')]
})
raise RuntimeError('A project may not be moved to be a child of itself or one of its descendants.')

super().save(*args, **kwargs)

@property
def catalog_uri(self):
Expand Down
13 changes: 12 additions & 1 deletion rdmo/projects/serializers/v1/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from rdmo.services.validators import ProviderValidator

from ...models import Integration, IntegrationOption, Invite, Issue, IssueResource, Membership, Project, Snapshot, Value
from ...validators import ValueConflictValidator, ValueQuotaValidator, ValueTypeValidator
from ...validators import ProjectParentValidator, ValueConflictValidator, ValueQuotaValidator, ValueTypeValidator


class UserSerializer(serializers.ModelSerializer):
Expand Down Expand Up @@ -78,6 +78,17 @@ class Meta:
read_only_fields = (
'snapshots',
)
validators = [
ProjectParentValidator()
]


class ProjectCopySerializer(ProjectSerializer):

class Meta:
model = Project
fields = ProjectSerializer.Meta.fields
read_only_fields = ProjectSerializer.Meta.read_only_fields


class ProjectMembershipSerializer(serializers.ModelSerializer):
Expand Down
5 changes: 3 additions & 2 deletions rdmo/projects/templates/projects/project_detail_sidebar.html
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ <h2>{% trans 'Options' %}</h2>
</ul>
{% endif %}

{% if can_change_project or can_delete_project %}
<ul class="list-unstyled">
{% if can_change_project %}
<li>
Expand All @@ -63,13 +62,15 @@ <h2>{% trans 'Options' %}</h2>
</li>
{% endif %}
{% endif %}
<li>
<a href="{% url 'project_copy' project.pk %}">{% trans 'Copy project' %}</a>
</li>
{% if can_delete_project %}
<li>
<a href="{% url 'project_delete' project.pk %}">{% trans 'Delete project' %}</a>
</li>
{% endif %}
</ul>
{% endif %}

{% has_perm 'projects.add_membership_object' request.user project as can_add_membership %}
{% if can_add_membership %}
Expand Down
90 changes: 89 additions & 1 deletion rdmo/projects/tests/test_utils.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import pytest

from django.contrib.auth.models import User
from django.contrib.sites.models import Site
from django.http import QueryDict

from ..filters import ProjectFilter
from ..utils import set_context_querystring_with_filter_and_page
from ..models import Project
from ..utils import copy_project, set_context_querystring_with_filter_and_page

GET_queries = [
'page=2&title=project',
Expand Down Expand Up @@ -32,3 +35,88 @@ def test_set_context_querystring_with_filter_and_page(GET_query):
assert context.get('querystring', 'not-in-context') == ''
else:
assert context.get('querystring', 'not-in-context') == 'not-in-context'


def test_copy_project(db, files):
project = Project.objects.get(id=1)
site = Site.objects.get(id=2)
user = User.objects.get(id=1)
project_copy = copy_project(project, site, [user])

# re fetch the original project
project = Project.objects.get(id=1)

# check that site, owners, tasks, and views are correct
assert project_copy.site == site
assert list(project_copy.owners) == [user]
assert list(project_copy.user.values('id')) == [{'id': user.id}]
assert list(project_copy.tasks.values('id')) == list(project.tasks.values('id'))
assert list(project_copy.views.values('id')) == list(project.views.values('id'))

# check that no ids are the same
assert project_copy.id != project.id
assert not set(project_copy.snapshots.values_list('id')).intersection(set(project.snapshots.values_list('id')))
assert not set(project_copy.values.values_list('id')).intersection(set(project.values.values_list('id')))

# check the snapshots
snapshot_fields = (
'title',
'description'
)
for snapshot_copy, snapshot in zip(
project_copy.snapshots.values(*snapshot_fields),
project.snapshots.values(*snapshot_fields)
):
assert snapshot_copy == snapshot

# check the values
value_fields = (
'attribute',
'set_prefix',
'set_collection',
'set_index',
'collection_index',
'text',
'option',
'value_type',
'unit',
'external_id'
)
ordering = (
'attribute',
'set_prefix',
'set_index',
'collection_index'
)
for value_copy, value in zip(
project_copy.values.filter(snapshot=None).order_by(*ordering),
project.values.filter(snapshot=None).order_by(*ordering)
):
for field in value_fields:
assert getattr(value_copy, field) == getattr(value, field), field

if value_copy.file:
assert value_copy.file.path == value_copy.file.path.replace(
f'/projects/{project.id}/values/{value.id}/',
f'/projects/{project_copy.id}/values/{value_copy.id}/'
)
assert value_copy.file.size == value_copy.file.size
else:
assert not value.file

for snapshot_copy, snapshot in zip(project_copy.snapshots.all(), project.snapshots.all()):
for value_copy, value in zip(
project_copy.values.filter(snapshot=snapshot_copy).order_by(*ordering),
project.values.filter(snapshot=snapshot).order_by(*ordering)
):
for field in value_fields:
assert getattr(value_copy, field) == getattr(value, field)

if value_copy.file:
assert value_copy.file.path == value_copy.file.path.replace(
f'/projects/{project.id}/snapshot/{snapshot.id}/values/{value.id}/',
f'/projects/{project_copy.id}/snapshot/{snapshot.id}/values/{value_copy.id}/'
)
assert value_copy.file.open('rb').read() == value_copy.file.open('rb').read()
else:
assert not value.file
34 changes: 22 additions & 12 deletions rdmo/projects/tests/test_view_project.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,9 @@
export_formats = ('rtf', 'odt', 'docx', 'html', 'markdown', 'tex', 'pdf')

site_id = 1
parent_project_id = 1
project_id = 1
parent_id = 3
parent_ancestors = [2, 3]
catalog_id = 1


Expand Down Expand Up @@ -266,7 +268,7 @@ def test_project_create_parent_post(db, client, username, password):
'title': 'A new project',
'description': 'Some description',
'catalog': catalog_id,
'parent': parent_project_id
'parent': project_id
}
response = client.post(url, data)

Expand Down Expand Up @@ -335,17 +337,21 @@ def test_project_update_post_parent(db, client, username, password, project_id):
'title': project.title,
'description': project.description,
'catalog': project.catalog.pk,
'parent': parent_project_id
'parent': parent_id
}
response = client.post(url, data)

if project_id in change_project_permission_map.get(username, []):
if project_id == parent_project_id:
if parent_id in view_project_permission_map.get(username, []):
if project_id in parent_ancestors:
assert response.status_code == 200
assert Project.objects.get(pk=project_id).parent == project.parent
else:
assert response.status_code == 302
assert Project.objects.get(pk=project_id).parent_id == parent_id
else:
assert response.status_code == 200
assert Project.objects.get(pk=project_id).parent == project.parent
else:
assert response.status_code == 302
assert Project.objects.get(pk=project_id).parent_id == parent_project_id
else:
if password:
assert response.status_code == 403
Expand Down Expand Up @@ -545,17 +551,21 @@ def test_project_update_parent_post(db, client, username, password, project_id):

url = reverse('project_update_parent', args=[project_id])
data = {
'parent': parent_project_id
'parent': parent_id
}
response = client.post(url, data)

if project_id in change_project_permission_map.get(username, []):
if project_id == parent_project_id:
if parent_id in view_project_permission_map.get(username, []):
if project_id in parent_ancestors:
assert response.status_code == 200
assert Project.objects.get(pk=project_id).parent == project.parent
else:
assert response.status_code == 302
assert Project.objects.get(pk=project_id).parent_id == parent_id
else:
assert response.status_code == 200
assert Project.objects.get(pk=project_id).parent == project.parent
else:
assert response.status_code == 302
assert Project.objects.get(pk=project_id).parent_id == parent_project_id
else:
if password:
assert response.status_code == 403
Expand Down
Loading
Loading