Skip to content
Permalink
Browse files

Merge branch 'develop'

  • Loading branch information...
pbanaszkiewicz committed Jun 19, 2019
2 parents 11d7fcf + 3da1319 commit 3e4daf2b98ee5c67f076337016f92bb1047c822a
@@ -0,0 +1,22 @@
---
name: issue_template
about: Informs contributors of how issues will be reviewed
title: ''
labels: ''
assignees: ''

---

Thank you for submitting this issue to AMY. If this represents an urgent problem such as AMY not loading at all, please contact team@carpentries.org so we can respond promptly.

Non-urgent issues will be reviewed weekly by @maneesha. She will follow up here with you and/or other team members if more information is needed. Issues will be prioritized, considering whether they identify broken features, identify potential enhancements to existing features, or request implemention of entirely new features.


After prioritizing, one of the following will happen:

* High priority issues will be included in an upcoming work cycle and labeled for an existing milestone.
* Medium to low priority issues will be considered for a future work cycle and labeled "later." These will be re-reviewed periodically to consider them for an upcoming work cycle.
* Issues that are stale, redundant, or are otherwise assessed not to positively impact user workflows will be closed and labeled "wont-fix"


Comments in the issue itself will provide further context. Please contact team@carpentries.org with any questions. Thank you again for your contribution and helping us improve AMY.
@@ -29,3 +29,6 @@ mediafiles/*

# Database backup files
DB_backups/*

# log file
amy.log
@@ -1,3 +1,6 @@
from datetime import date, datetime, timedelta

from django.utils import timezone
from django.urls import reverse

from workshops.models import Person
@@ -72,10 +75,189 @@ def test_switched_names_persons(self):

def test_duplicate_persons(self):
rv = self.client.get(self.url)
switched = rv.context['duplicate_persons']
self.assertIn(self.ron, switched)
self.assertIn(self.ron2, switched)
duplicated = rv.context['duplicate_persons']
self.assertIn(self.ron, duplicated)
self.assertIn(self.ron2, duplicated)
self.assertNotIn(self.harry, duplicated)
self.assertNotIn(self.potter, duplicated)


class TestFindingReviewedDuplicates(TestBase):
def setUp(self):
self._setUpUsersAndLogin()

self.harry = Person.objects.create(
personal='Harry', family='Potter', username='potter_harry',
email='hp@hogwart.edu')
self.potter = Person.objects.create(
personal='Potter', family='Harry', username='harry_potter',
email='hp+1@hogwart.edu')
self.ron = Person.objects.create(
personal='Ron', family='Weasley', username='weasley_ron',
email='rw@hogwart.edu')
self.ron2 = Person.objects.create(
personal='Ron', family='Weasley', username='weasley_ron_2',
email='rw+1@hogwart.edu')

self.url = reverse('duplicate_persons')
self.review_url = reverse('review_duplicate_persons')

def test_finding_unreviewed_duplicates(self):
rv = self.client.get(self.url)
switched = rv.context['switched_persons']
duplicates = rv.context['duplicate_persons']

self.assertEqual(self.harry.duplication_reviewed_on, None)
self.assertEqual(self.potter.duplication_reviewed_on, None)
self.assertEqual(self.ron.duplication_reviewed_on, None)
self.assertEqual(self.ron2.duplication_reviewed_on, None)

self.assertIn(self.harry, switched)
self.assertIn(self.potter, switched)
self.assertNotIn(self.ron, switched)
self.assertNotIn(self.ron2, switched)

self.assertIn(self.ron, duplicates)
self.assertIn(self.ron2, duplicates)
self.assertNotIn(self.harry, duplicates)
self.assertNotIn(self.potter, duplicates)

def test_not_finding_reviewed_duplicates(self):
"""Ensure records with `last_changed_at` timestamp close to their
`duplication_reviewed_on` timestamp don't show up in the results."""

# modify duplication_reviewed_on to point to the
# same timestamp (or very close) that last_updated_at will
# after save() so that these records don't show up in results
review_date = timezone.now()

self.harry.duplication_reviewed_on = review_date
self.harry.save()
self.potter.duplication_reviewed_on = review_date
self.potter.save()
self.ron.duplication_reviewed_on = review_date
self.ron.save()
self.ron2.duplication_reviewed_on = review_date
self.ron2.save()

rv = self.client.get(self.url)
switched = rv.context['switched_persons']
duplicates = rv.context['duplicate_persons']

self.assertNotIn(self.harry, switched)
self.assertNotIn(self.potter, switched)
self.assertNotIn(self.ron, switched)
self.assertNotIn(self.ron2, switched)

self.assertNotIn(self.ron, duplicates)
self.assertNotIn(self.ron2, duplicates)
self.assertNotIn(self.harry, duplicates)
self.assertNotIn(self.potter, duplicates)

def test_finding_duplicates_changed_soon_after_reviewed(self):
# make sure after changing the timestamp difference between
# `last_updated_at` and `duplication_reviewed_on` to couple minutes,
# the records show up
review_date = timezone.now() - timedelta(minutes=2)

self.harry.duplication_reviewed_on = review_date
self.harry.save()
self.potter.duplication_reviewed_on = review_date
self.potter.save()
self.ron.duplication_reviewed_on = review_date
self.ron.save()
self.ron2.duplication_reviewed_on = review_date
self.ron2.save()

rv = self.client.get(self.url)
switched = rv.context['switched_persons']
duplicates = rv.context['duplicate_persons']

self.assertIn(self.harry, switched)
self.assertIn(self.potter, switched)
self.assertNotIn(self.ron, switched)
self.assertNotIn(self.ron2, switched)

self.assertIn(self.ron, duplicates)
self.assertIn(self.ron2, duplicates)
self.assertNotIn(self.harry, duplicates)
self.assertNotIn(self.potter, duplicates)

def test_finding_reviewed_changed_duplicates(self):
# modify last_updated_at and duplication_reviewed_on
# so that these records don't show up in results
change_timestamp = timezone.now()
review_date = change_timestamp - timedelta(days=1)

self.harry.duplication_reviewed_on = review_date
self.harry.last_updated_at = change_timestamp
self.harry.save()
self.potter.duplication_reviewed_on = review_date
self.potter.last_updated_at = change_timestamp
self.potter.save()
self.ron.duplication_reviewed_on = review_date
self.ron.last_updated_at = change_timestamp
self.ron.save()
self.ron2.duplication_reviewed_on = review_date
self.ron2.last_updated_at = change_timestamp
self.ron2.save()

rv = self.client.get(self.url)
switched = rv.context['switched_persons']
duplicates = rv.context['duplicate_persons']

self.assertIn(self.harry, switched)
self.assertIn(self.potter, switched)
self.assertNotIn(self.ron, switched)
self.assertNotIn(self.ron2, switched)

self.assertIn(self.ron, duplicates)
self.assertIn(self.ron2, duplicates)
self.assertNotIn(self.harry, duplicates)
self.assertNotIn(self.potter, duplicates)

def test_not_finding_partially_reviewed_duplicates(self):
"""Ensure that if some records from the duplicated/switched
names pair don't show up in the results, the other records won't
either."""

# modify duplication_reviewed_on to point to the
# same date that last_updated_at will after save()
# so that these records don't show up in results
review_date = timezone.now()

self.harry.duplication_reviewed_on = review_date
self.harry.save()
#self.potter.duplication_reviewed_on = review_date
#self.potter.save()
self.ron.duplication_reviewed_on = review_date
self.ron.save()
#self.ron2.duplication_reviewed_on = review_date
#self.ron2.save()

rv = self.client.get(self.url)
switched = rv.context['switched_persons']
duplicates = rv.context['duplicate_persons']

self.assertNotIn(self.harry, switched)
self.assertNotIn(self.potter, switched)
self.assertNotIn(self.ron, switched)
self.assertNotIn(self.ron2, switched)

self.assertNotIn(self.ron, duplicates)
self.assertNotIn(self.ron2, duplicates)
self.assertNotIn(self.harry, duplicates)
self.assertNotIn(self.potter, duplicates)

def test_reviewing_persons(self):
self.assertFalse(self.harry.duplication_reviewed_on)
self.assertFalse(self.ron.duplication_reviewed_on)

rv = self.client.post(
self.review_url, {'person_id': [self.harry.pk, self.ron.pk]}
)

# there might be more to come
self.harry.refresh_from_db()
self.ron.refresh_from_db()
self.assertTrue(self.harry.duplication_reviewed_on)
self.assertTrue(self.ron.duplication_reviewed_on)
@@ -13,5 +13,6 @@
path('workshop_issues/', views.workshop_issues, name='workshop_issues'),
path('instructor_issues/', views.instructor_issues, name='instructor_issues'),
path('duplicate_persons/', views.duplicate_persons, name='duplicate_persons'),
path('duplicate_persons/review/', views.review_duplicate_persons, name='review_duplicate_persons'),
path('duplicate_training_requests/', views.duplicate_training_requests, name='duplicate_training_requests'),
]
@@ -1,5 +1,6 @@
import datetime

from django.contrib import messages
from django.db.models import (
Case,
When,
@@ -10,7 +11,8 @@
F,
Prefetch,
)
from django.shortcuts import render
from django.shortcuts import render, redirect
from django.utils import timezone
from django.urls import reverse

from api.filters import (
@@ -324,9 +326,15 @@ def duplicate_persons(request):
Criteria for persons:
* switched personal/family names
* same name on different people."""
names_normal = set(Person.objects.all().values_list('personal', 'family'))
names_switched = set(Person.objects.all().values_list('family',
'personal'))

names_normal = set(
Person.objects.duplication_review_expired()
.values_list('personal', 'family')
)
names_switched = set(
Person.objects.duplication_review_expired()
.values_list('family', 'personal')
)
names = names_normal & names_switched # intersection

switched_criteria = Q(id=0)
@@ -335,10 +343,12 @@ def duplicate_persons(request):
# get people who appear in `names`
switched_criteria |= (Q(personal=personal) & Q(family=family))

switched_persons = Person.objects.filter(switched_criteria) \
switched_persons = Person.objects.duplication_review_expired()\
.filter(switched_criteria) \
.order_by('email')

duplicate_names = Person.objects.values('personal', 'family') \
duplicate_names = Person.objects.duplication_review_expired() \
.values('personal', 'family') \
.order_by() \
.annotate(count_id=Count('id')) \
.filter(count_id__gt=1)
@@ -348,7 +358,9 @@ def duplicate_persons(request):
# get people who appear in `names`
duplicate_criteria |= (Q(personal=name['personal']) &
Q(family=name['family']))
duplicate_persons = Person.objects.filter(duplicate_criteria) \

duplicate_persons = Person.objects.duplication_review_expired() \
.filter(duplicate_criteria) \
.order_by('family', 'personal', 'email')

context = {
@@ -360,6 +372,24 @@ def duplicate_persons(request):
return render(request, 'reports/duplicate_persons.html', context)


@admin_required
def review_duplicate_persons(request):
if request.method == 'POST' and 'person_id' in request.POST:
ids = request.POST.getlist('person_id')
now = timezone.now()
number = Person.objects.filter(id__in=ids) \
.update(duplication_reviewed_on=now)
messages.success(
request,
'Successfully marked {} persons as reviewed.'.format(number)
)

else:
messages.warning(request, 'Wrong request or request data missing.')

return redirect(reverse('duplicate_persons'))


@admin_required
def duplicate_training_requests(request):
"""Find possible duplicates amongst training requests.
@@ -40,6 +40,9 @@ table .additional-links {
table .additional-links-wider {
width: 100px;
}
table .table-row-distinctive {
border-top: 4px solid #dee2e6 !important;
}

.form-group small {
font-size: 0.8rem;

0 comments on commit 3e4daf2

Please sign in to comment.
You can’t perform that action at this time.