Got collection links working and tested

app/library/forms.py

@@ -5,7 +5,6 @@ from interface.forms import BaseForm
 
 
 class WorkCreateForm(forms.ModelForm, BaseForm):
-    #uploads = forms.FileField(label="PDFs to upload", widget=forms.ClearableFileInput(attrs={'multiple': True}), required=False)
 
     class Meta:
         model = Work

app/library/migrations/0013_auto_20230223_1322.py

@@ -0,0 +1,25 @@
+# Generated by Django 3.2.7 on 2023-02-23 02:22
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('library', '0012_auto_20230220_1013'),
+    ]
+
+    operations = [
+        migrations.AlterModelOptions(
+            name='section',
+            options={'ordering': ['doc', 'start', 'pk']},
+        ),
+        migrations.RemoveField(
+            model_name='section',
+            name='ordinal',
+        ),
+        migrations.RemoveField(
+            model_name='section',
+            name='type',
+        ),
+    ]

app/library/migrations/0014_auto_20230223_1422.py

@@ -0,0 +1,28 @@
+# Generated by Django 3.2.7 on 2023-02-23 03:22
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('library', '0013_auto_20230223_1322'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='collection',
+            name='nonce',
+            field=models.SmallIntegerField(default=1, help_text='Increment this to reset the authentication links'),
+        ),
+        migrations.AlterField(
+            model_name='work',
+            name='code',
+            field=models.CharField(blank=True, help_text='Collection specific code or number. Will be auto-generated if not supplied', max_length=100),
+        ),
+        migrations.AlterField(
+            model_name='work',
+            name='composer',
+            field=models.CharField(default='Anon', help_text='Composer or compilation editor. Use <b>Surname, Initial</b> for easy searching', max_length=255),
+        ),
+    ]

app/library/models.py

@@ -1,5 +1,6 @@
 from os import SCHED_OTHER
 from django.conf import settings
+from django.shortcuts import resolve_url
 from django.db import models
 from django.utils.text import slugify
 from django.utils.timezone import now
@@ -11,7 +12,9 @@ import re
 
 from byostorage.user import BYOStorage
 from byostorage.cached import CachedStorage
-from .imslp import Instrument
+
+from library.music_tags import MusicTag
+from interface.utils import sign_data
 
 import logging
 
@@ -41,7 +44,7 @@ class Orchestration(models.Model):
 
     def as_list(self):
         tags = [ t.strip() for t in self.instruments.split(' ') ]
-        return [ (t, Instrument.from_tag(t)) for t in tags if t ]
+        return [ (t, MusicTag.from_tag(t)) for t in tags if t ]
     
     def tag_order(self):
         tags = [ t.strip() for t in self.instruments.split(' ') if t ]
@@ -101,6 +104,8 @@ class Collection(models.Model):
             help_text="User storage for documents")
     notes = models.TextField(blank=True, 
             help_text="Publicly visible notes about collection and loans policy (markdown format)")
+    nonce = models.SmallIntegerField(default=1,
+            help_text="Increment this to reset the authentication links")
 
     def meta(self, name):
         items = WorkMeta.objects.filter(work__collection=self.pk, name=name).values_list('value', flat=True).distinct()
@@ -121,6 +126,12 @@ class Collection(models.Model):
             return True
         return user.pk in self.administrators.values_list('pk', flat=True)
 
+    def get_absolute_url(self):
+        return resolve_url('collection_work_list', self.pk)
+
+    def auth(self):
+        return sign_data(f'{self.pk}-{self.nonce}', 12)
+    
     def __str__(self):
         return self.name
 
@@ -171,14 +182,14 @@ class Work(models.Model):
     parent = models.ForeignKey('Work', null=True, blank=True, on_delete=models.SET_NULL, related_name="related_works",
             help_text="Arrangement of another work or part of an anthology")
     composer = models.CharField(max_length=255, default='Anon',
-            help_text="Surname, Initials")
+            help_text="Composer or compilation editor. Use <b>Surname, Initial</b> for easy searching")
 
     orchestration = models.ForeignKey(Orchestration, on_delete=models.SET_DEFAULT, default=1, help_text="Orchestration for the work")
     original_parts = models.JSONField(default=dict, blank=True, help_text="Original printed parts (IMSLP format)")
 
     # Collection details
     collection = models.ForeignKey(Collection, on_delete=models.CASCADE, related_name="works")
-    code = models.CharField(max_length=100, blank=True, help_text="Collection specific code or number")
+    code = models.CharField(max_length=100, blank=True, help_text="Collection specific code or number. Will be auto-generated if not supplied")
     licence = models.PositiveSmallIntegerField(choices=LICENCE_TYPES, default=6, help_text="Copyright status")
     max_projects = models.IntegerField(default=1, help_text="How many active projects can this work be attached to")
     
@@ -213,7 +224,7 @@ class Work(models.Model):
             return []
         parts = list(self.original_parts.items())
         parts.sort(key=self.orchestration.sorter())
-        return [ (Instrument.from_tag(x[0]), x[1]) for x in parts ]
+        return [ (MusicTag.from_tag(x[0]), x[1]) for x in parts ]
 
     @property
     def tags(self):
@@ -314,7 +325,7 @@ class Document(models.Model):
     )
 
     work = models.ForeignKey('Work', on_delete=models.CASCADE, related_name="docs")
-    doctype = models.PositiveSmallIntegerField(choices=DOCTYPES, default=1)
+    doctype = models.PositiveSmallIntegerField(choices=DOCTYPES, default=DOCTYPE_PDF)
     upload = models.FileField(upload_to=doc_upload_filename, storage=library_storage)
     created = models.DateTimeField(auto_now_add=True)
     version = models.CharField(max_length=30, blank=True)
@@ -330,23 +341,7 @@ class Section(models.Model):
     """
     Section is a tagged portion of a Document
     """
-
+    
-    TYPE_INSTRUMENT = 1
-    TYPE_MOVEMENT = 2
-    TYPE_EXCERPT = 3
-
-    SECTION_TYPES = (
-        (TYPE_INSTRUMENT, "Instrument"),
-        (TYPE_MOVEMENT, "Movement"),
-        (TYPE_EXCERPT, "Excerpt"),
-    )
-
-    SECTION_CLASSES = {
-        TYPE_INSTRUMENT: 'info',
-        TYPE_MOVEMENT: 'success',
-        TYPE_EXCERPT: 'warning',
-    }
-
     PAGE_AUTO = 0
     PAGE_LEFT = 1
     PAGE_RIGHT = 2
@@ -358,33 +353,30 @@ class Section(models.Model):
     )
 
 
-    type = models.SmallIntegerField(choices=SECTION_TYPES)
     doc = models.ForeignKey(Document, on_delete=models.CASCADE, related_name="sections")
     tag = models.CharField(max_length=50, blank=True)
-    ordinal = models.IntegerField(default=0)
     start = models.SmallIntegerField(null=True, blank=True)
     end = models.SmallIntegerField(null=True, blank=True)
     page = models.SmallIntegerField(default=PAGE_AUTO, choices=PAGE_PREFERENCE) # NOT CURRENTLY USED
 
     class Meta:
-        ordering = ['type', 'ordinal', 'doc', 'start', 'pk']
+        ordering = ['doc', 'start', 'pk']
+
+    @property
+    def music_tag(self):
+        return MusicTag.from_tag(self.tag)
 
     @property
     def name(self):
-        if self.type == self.TYPE_INSTRUMENT:
+        return str(self.music_tag)
-            instr = Instrument.from_tag(self.tag)
-            if self.ordinal:
-                return f'{instr} {self.ordinal}'
-            return str(instr)
-        return f"{self.ordinal} - {self.tag}"
 
     @property
     def bulma_class(self):
-        return self.SECTION_CLASSES[self.type]
+        return "success" if self.music_tag.is_general else 'info'
 
     @property
     def filename(self):
-        return slugify(f'{self.doc.work.name} - {self.name}') + '.pdf'
+        return slugify(f'{self.doc.work.name} - {self.name}').title() + '.pdf'
 
     @property 
     def pagerange(self):

app/library/music_tags.py

@@ -1,11 +1,19 @@
 
 from collections import namedtuple
 
-# taken from https://imslp.org/wiki/IMSLP:Abbreviations_for_Instruments
+GENERAL = """
-# Place any extra abbreviations at the top
+mvmt    Movement
+ex      Excerpt
+sec     Section
+pce     Piece
+"""
 
-ABBREVIATIONS = """
+# taken from https://imslp.org/wiki/IMSLP:Abbreviations_for_MusicTags
+# Abbreviations at the top will take precidence in reverse lookups
+
+INSTRUMENTS = """
 score   Score
+
 cb      Double bass
 mall    Mallet percussion
 
@@ -25,7 +33,7 @@ bgtr 	Bass guitar
 bjo 	Banjo 
 bn 	    Bassoon 
 bob 	Bass oboe (Baritone oboe) 
-br 	    Brass instruments 
+br 	    Brass MusicTags 
 bryt 	Baryton 
 bstcl 	Basset clarinet 
 bsthn 	Basset horn 
@@ -52,7 +60,7 @@ dom 	Domra
 dulc 	Dulcimer 
 egtr 	Electric guitar 
 eh 	    English horn (Cor anglais) 
-elec 	Electronic Instruments 
+elec 	Electronic MusicTags 
 epf 	Electric piano 
 eq 	    Equal voices 
 erhu 	Erhu 
@@ -72,7 +80,7 @@ heck 	Heckelphone
 hn 	    Horn 
 hp 	    Harp 
 hpd 	Harpsichord 
-kbd 	Keyboard instrument 
+kbd 	Keyboard MusicTag 
 lute 	Lute 
 lyre 	Lyre 
 mand 	Mandolin 
@@ -114,7 +122,7 @@ skbt 	Sackbut
 sop 	Soprano (voice) 
 srp 	Serpent 
 stpt 	Slide trumpet 
-str 	String instruments 
+str 	String MusicTags 
 sxh 	Saxhorn 
 syn 	Synthesizer 
 tba 	Tuba 
@@ -138,48 +146,45 @@ vn 	    Violin
 vuv 	Vuvuzela 
 vv 	    Voices (multiple soloists) 
 wag 	Wagner tuba 
-ww 	    Woodwind instruments 
+ww 	    Woodwind MusicTags 
 xiao 	Xiao 
 xyl 	Xylophone 
 zith 	Zither 
 """
 
-ORCHESTRATIONS = {
+MUSIC_TAGS = []
-    'SATB': ('S', 'A', 'T', 'B'),
+GENERAL_TAGS = set()
-    'String Quartet': ('Vln1', 'Vln2', 'Vla', 'Vc'),
+for i, abbreviations in enumerate((GENERAL, INSTRUMENTS)):
-    'String Orchestra': ('Vln1', 'Vln2', 'Vla', 'Vc', 'Cb'),
+    for line in abbreviations.split('\n'):
-    'Chamber Orchestra': ('Vln1', 'Vln2', 'Vla', 'Vc', 'Cb', 
+        parts = line.strip().split(maxsplit=1)
-            'Fl1', 'Fl2', 'Cl1', 'Cl2', 'Hn1', 'Hn2', 
+        if len(parts) < 2: continue
-            'Tpt1', 'Tpt2', 'Tbn1', 'Tbn2', 'Tuba',
+        name, _, _ = parts[1].partition('(')
-            'Timp', 'Drum', 'Perc'),
+        MUSIC_TAGS.append((parts[0], name))
-    'Custom': (),
+        if i == 0:
-}
+            GENERAL_TAGS.add(parts[0])
 
 
-INSTRUMENTS = []
+MUSIC_NAME_BY_TAG = dict(MUSIC_TAGS)
-for line in ABBREVIATIONS.split('\n'):
+MUSIC_TAG_BY_NAME = dict( ( (x[1].lower(), x[0]) for x in MUSIC_TAGS ) )
-    parts = line.strip().split(maxsplit=1)
-    if len(parts) < 2: continue
-    name, _, _ = parts[1].partition('(')
-    INSTRUMENTS.append((parts[0], name))
 
-INSTRUMENT_NAMES = dict(INSTRUMENTS)
+class MusicTag(namedtuple('MusicTag', ('name', 'variant'), defaults=[None])):
-INSTRUMENT_TAGS = dict( ( (x[1].lower(), x[0]) for x in INSTRUMENTS ) )
-
-class Instrument(namedtuple('Instrument', ('name', 'variant'), defaults=[None])):
 
     @classmethod
     def from_tag(cls, tag):
         """
-        >>> Instrument.from_tag('vn-1')
+        >>> MusicTag.from_tag('vn-1')
-        Instrument(name='Violin', variant='1')
+        MusicTag(name='Violin', variant='1')
-        >>> Instrument.from_tag('db')
+        >>> MusicTag.from_tag('db')
-        Instrument(name='Double Bass', variant=None)
+        MusicTag(name='Double Bass', variant=None)
-        >>> Instrument.from_tag('Jaws Harp')
+        >>> MusicTag.from_tag('Jaws Harp')
-        Instrument(name='Jaws Harp', variant=None)
+        MusicTag(name='Jaws Harp', variant=None)
+        >>> MusicTag.from_tag('mvmt-2')
+        MusicTag(name='Movement', variant='2')
+        >>> MusicTag.from_tag('pce-A2')
+        MusicTag(name='Piece', variant='A2')
         """
         abbr, _, variant = tag.partition('-')
-        name = INSTRUMENT_NAMES.get(abbr.lower(), abbr)
+        name = MUSIC_NAME_BY_TAG.get(abbr.lower(), abbr)
 
         if variant:
             return cls(name, variant)
@@ -188,25 +193,35 @@ class Instrument(namedtuple('Instrument', ('name', 'variant'), defaults=[None]))
     @property
     def tag(self):
         l = self.name.lower()
-        return INSTRUMENT_TAGS.get(l, l)
+        return MUSIC_TAG_BY_NAME.get(l, l)
+
+    @property
+    def is_general(self):
+        """
+        >>> MusicTag('Piece', 'A3').is_general
+        True
+        >>> MusicTag('Violin', 2).is_general
+        False
+        """
+        return self.tag in GENERAL_TAGS 
 
     def abbreviate(self):
         """
-        >>> Instrument('Violin', 1).abbreviate()
+        >>> MusicTag('Violin', 1).abbreviate()
         'vn-1'
-        >>> Instrument('Double Bass').abbreviate()
+        >>> MusicTag('Double Bass').abbreviate()
         'db'
         """
-        tag = INSTRUMENT_TAGS.get(self.name.lower())
+        tag = MUSIC_TAG_BY_NAME.get(self.name.lower())
         if self.variant:
             tag = f"{tag}-{self.variant}"
         return tag
 
     def __str__(self):
         """
-        >>> str(Instrument('Violin', 1))
+        >>> str(MusicTag('Violin', 1))
         'Violin 1'
-        >>> str(Instrument('Double Bass'))
+        >>> str(MusicTag('Double Bass'))
         'Double Bass'
         """
         if self.variant:

app/library/tests.py

@@ -1,12 +1,56 @@
-from django.test import TestCase
+from interface.tests import AccessTestCase 
 
 from django.contrib.auth.models import User
 from interface.models import Ensemble, Project
 from . import models
 
-class IntegrationTestCase(TestCase):
+class IntegrationTestCase(AccessTestCase):
 
-    def setUp(self):
+    USERS = (
+        {'username': 'admin', 'password': 'secret', 'is_superuser': True, 'is_staff': True},
+        {'username': 'homer', 'password': 'maggie'},
+    )
+
+    ENSEMBLES = (
+        {'name': 'The Be Sharps', 'slug': 'be-sharps', 'admins': ['homer']},
+        {'name': 'Lisa & the Bleeding Gums', 'slug': 'bleeding-gums'},
+        {'name': 'Party Posse'},
+    )
+
+    PROJECTS = (
+        ('Baker St', 'bleeding-gums', -12),
+        ('Navy Recruitment Day', 'party-posse', 6),
+        ('Barbershop Contest', 'be-sharps', 28),
+        ('Open Mic Night', 'bleeding-gums', 1)
+    )
+
+    COLLECTIONS = (
+        {'name': 'Springfield Elementary Library', 'prefix': 'sel'},
+        {'name': 'Neds Library', 'prefix': 'ned', 'admins': ['homer']},
+    )
+
+    WORKS = (
+        {'name': 'Baby on Board', 'collection': 'ned'},
+    )
+
+    @classmethod
+    def setUpTestData(cls):
+        super().setUpTestData()
+        
+        cls.collections = {}
+        for details in cls.COLLECTIONS:
+            admins = details.pop('admins', [])
+            obj = models.Collection.objects.create(**details)
+            for admin in admins:
+                obj.administrators.add(cls.users[admin])
+            cls.collections[details['prefix']] = obj
+
+        cls.works = {}
+        for details in cls.WORKS:
+            collection = details.pop('collection')
+            cls.works[details['name']] = models.Work.objects.create(collection=cls.collections[collection], **details)
+
+    def oldSetUp(self):
         self.homer = User.objects.create(username='homer')
         self.ned = User.objects.create(username="ned")
         self.lisa = User.objects.create(username="lisa")
@@ -21,30 +65,72 @@ class IntegrationTestCase(TestCase):
     def test_integration(self):
         pass
 
+    def test_superuser_access(self):
+        self.login('admin', 'secret')
+        self.assertAccess({
+            '/collections': True,
+            '/collections/1': True,
+            '/collections/2/works/1': True,
+        })
+
+    def test_administrator_access(self):
+        self.login('homer', 'maggie')
+        self.assertAccess({
+            '/collections': True,
+            '/collections/1': False,
+            '/collections/2': True,
+            '/collections/2/works/1': True,
+        })
+
+    def test_link_access(self):
+        self.assertAccess({
+            '/collections': True,
+            '/collections/1': False,
+            '/collections/2': False,
+            '/collections/2/works/1': False,
+        })
+
+        self.authorize(models.Collection, pk=2)
+        self.assertAccess({
+            '/collections': True,
+            '/collections/1': False,
+            '/collections/2': True,
+            '/collections/2/works/1': True,
+        })
+
+    def test_anon_access(self):
+        self.assertAccess({
+            '/collections': True,
+            '/collections/1': False,
+            '/collections/2': False,
+            '/collections/2/works/1': False,
+        })
+        
+
     def test_movement_from_large_work(self):
         '''
         Will be common to store a work which has several movements, but the project is only going to play one.
         This also should give us the ability to store an anthology as one Work have Project reference 'no:23'
         '''
 
-        work = self.sel.works.create(name="Some Quartet", composer="Beethoven")
+        work = self.collections['sel'].works.create(name="Some Quartet", composer="Beethoven")
-        for g in ('vl1', 'vl2', 'vla', 'vc'):
+        for g in ('vl-1', 'vl-2', 'vla', 'vc'):
             doc = work.docs.create(upload=f'sel/beethoven/some_quartet/some_quartet_{g}.pdf')
-            doc.sections.create(tag='mvmt:1', start=1, end=3)
+            doc.sections.create(tag='mvmt-1', start=1, end=3)
-            doc.sections.create(tag='mvmt:2', start=4, end=8)
+            doc.sections.create(tag='mvmt-2', start=4, end=8)
-            doc.sections.create(tag='mvmt:3', start=9, end=12)
+            doc.sections.create(tag='mvmt-3', start=9, end=12)
-            doc.sections.create(tag=f'inst:{g}')
+            doc.sections.create(tag=g)
 
         # no tags - get nothing (should it be everything?)
         self.assertEqual(work.extract(), [])
 
         # single tag - should get just that range
-        self.assertEqual(work.extract('inst:vl1'), [('sel/beethoven/some_quartet/some_quartet_vl1.pdf', None, None)])
+        self.assertEqual(work.extract('vl-1'), [('sel/beethoven/some_quartet/some_quartet_vl-1.pdf', None, None)])
 
         # single tag - returns all documents with that range
-        result = work.extract('mvmt:2')
+        result = work.extract('mvmt-2')
         self.assertEqual(len(result), 4)
 
         # multiple tags - returns the overlapping portion of all documents that have all tags
-        self.assertEqual(work.extract('inst:vl1', 'mvmt:2'), [('sel/beethoven/some_quartet/some_quartet_vl1.pdf', 4, 8)])
+        self.assertEqual(work.extract('vl-1', 'mvmt-2'), [('sel/beethoven/some_quartet/some_quartet_vl-1.pdf', 4, 8)])
-        self.assertEqual(work.extract('inst:vl1', 'inst:vl2'), [])
+        self.assertEqual(work.extract('vl-1', 'vl-2'), [])

app/library/views/__init__.py

@@ -9,6 +9,8 @@ from django.db import transaction
 from django.utils.timezone import now
 from django.urls import reverse
 from django.template.loader import render_to_string
+from django.core.exceptions import SuspiciousOperation
+from django.http import Http404, HttpResponseRedirect
 
 import json
 import os.path
@@ -17,14 +19,10 @@ import re
 from interface.views import EnsembleMixin, ProjectMixin, AuthorizedResourceMixin
 from interface.models import Project
 from library.models import Collection, Work, Document, Section
-from library.imslp import INSTRUMENT_TAGS, INSTRUMENTS
+from library.music_tags import MUSIC_TAGS
 from library import forms, models
 from library.pdf_utils import extract_pages, extract_and_concat
 
-"""
-
-"""
-
 
 class ProjectItemListView(ProjectMixin, ListView):
     template_name = "library/item_list.html"
@@ -72,7 +70,7 @@ class ProjectItemListView(ProjectMixin, ListView):
 
     def get_context_data(self, **kwargs):
         data = super(ProjectItemListView, self).get_context_data(**kwargs)
-        data['instruments'] = INSTRUMENTS
+        data['instruments'] = MUSIC_TAGS
         data['instrument'] = self.request.session.get('instrument', 'Score')
         data['part'] = self.request.session.get('part', '0')
         data['running_time'] = self.get_queryset().aggregate(Sum('work__running_time'))['work__running_time__sum']
@@ -129,7 +127,10 @@ class CollectionMixin(AuthorizedResourceMixin):
         if self.collection.has_administrator(self.request.user):
             self.request.is_admin = True
             return True
-        
+
+        if self.is_authorized_key('collection', collection_id, self.collection.nonce):
+            return True
+
         return False
 
     def get_context_data(self, **kwargs):
@@ -159,6 +160,14 @@ class CollectionListView(ListView):
 class WorkListView(CollectionMixin, ListView):
     paginate_by = 20
 
+    def request_denied(self):
+        if 'auth' in self.request.GET:
+            if self.request.GET['auth'] != self.collection.auth():
+                raise SuspiciousOperation("Bad collection link")
+            self.add_authorized_key('collection', self.collection.pk, self.collection.nonce)
+            return HttpResponseRedirect(self.request.path)
+        return super().request_denied()
+
     def get_works(self):
         collections = CollectionMixin.get_queryset(self)
         return  Work.objects.filter(collection__in=collections).select_related('collection')
@@ -230,7 +239,7 @@ class WorkDetailView(CollectionMixin, DetailView):
     model = models.Work 
 
 class WorkUpdateView(CollectionMixin, UpdateView):
-    #fields = ['name', 'composer', 'edition', 'code', 'orchestration', 'licence', 'max_projects', 'running_time', 'notes']
+    model = models.Work
     form_class = forms.WorkCreateForm
     template_name = 'interface/default_form.html'
 
@@ -348,12 +357,10 @@ class DocumentMixin(CollectionMixin):
     model = models.Document
 
     def get_queryset(self):
-        return models.Document.objects.filter(work__collection=self.collection)
+        qs = models.Document.objects.select_related('work')
-
+        if self.request.is_admin:
-#    def get_queryset(self):
+            return qs
-#        if self.request.is_admin:
+        return qs.filter(work__collection=self.collection)
-#            return Document.objects.select_related('work')
-#        return Document.objects.filter(work__ensemble=self.request.ensemble_id).select_related('work')
 
 class DocumentDetailView(DocumentMixin, DetailView):
     pass
@@ -395,7 +402,7 @@ class DocumentAnnotateView(DocumentMixin, DetailView):
         for part in data['document'].sections.all():
             pages.append((part.tag, part.start, part.end))
 
-        data['json_data'] = {'pageTags': pages, 'instruments': dict(INSTRUMENTS)}
+        data['json_data'] = {'pageTags': pages, 'instruments': dict(MUSIC_TAGS)}
         return data
 
 class DocumentDeleteView(DocumentMixin, DeleteView):