diff --git a/backend/toolshed/api/group.py b/backend/toolshed/api/group.py index 74e8e47..e682934 100644 --- a/backend/toolshed/api/group.py +++ b/backend/toolshed/api/group.py @@ -40,18 +40,11 @@ class GroupDetail(APIView, ViewSetMixin): authentication_classes = [SignatureAuthentication] permission_classes = [IsAuthenticated] - def get(self, request, pk, format=None): # /api/groups// - group = get_object_or_404(Group, pk=pk) - if not group.is_member(request.user): - return Response(status=status.HTTP_404_NOT_FOUND) - return Response(GroupSerializer(group).data) - - -class GroupDetailByHandle(APIView, ViewSetMixin): - authentication_classes = [SignatureAuthentication] - permission_classes = [IsAuthenticated] - - def get(self, request, name, domain, format=None): # /api/groups/handle/// + def get(self, request, handle, format=None): # /api/groups// + try: + name, domain = split_userhandle_or_throw(handle) + except ValueError: + return Response(status=status.HTTP_400_BAD_REQUEST, data={'status': 'invalid group handle'}) group = get_object_or_404(Group, name=name, domain=domain) if not group.is_member(request.user): return Response(status=status.HTTP_404_NOT_FOUND) @@ -61,8 +54,12 @@ class GroupDetailByHandle(APIView, ViewSetMixin): @api_view(['DELETE']) @authentication_classes([SignatureAuthentication]) @permission_classes([IsAuthenticated]) -def removeGroupMember(request, pk, identity_id, format=None): # /api/groups//members// - group = get_object_or_404(Group, pk=pk) +def removeGroupMember(request, handle, identity_id, format=None): # /api/groups//members// + try: + name, domain = split_userhandle_or_throw(handle) + except ValueError: + return Response(status=status.HTTP_400_BAD_REQUEST, data={'status': 'invalid group handle'}) + group = get_object_or_404(Group, name=name, domain=domain) if not group.is_member(request.user): return Response(status=status.HTTP_404_NOT_FOUND) member = get_object_or_404(group.members, pk=identity_id) @@ -76,8 +73,12 @@ def removeGroupMember(request, pk, identity_id, format=None): # /api/groups//invites/ - group = get_object_or_404(Group, pk=pk) +def createGroupInvite(request, handle, format=None): # /api/groups//invites/ + try: + name, domain = split_userhandle_or_throw(handle) + except ValueError: + return Response(status=status.HTTP_400_BAD_REQUEST, data={'status': 'invalid group handle'}) + group = get_object_or_404(Group, name=name, domain=domain) if not group.is_member(request.user): return Response(status=status.HTTP_404_NOT_FOUND) invitee = request.data.get('invitee') @@ -197,10 +198,9 @@ def acceptGroupInvite(request, format=None): # /api/group_invites/accept/ - lan urlpatterns = [ path('groups/', Groups.as_view(), name='groups'), - path('groups//', GroupDetail.as_view(), name='group_detail'), - path('groups/handle///', GroupDetailByHandle.as_view(), name='group_detail_by_handle'), - path('groups//members//', removeGroupMember, name='remove_group_member'), - path('groups//invites/', createGroupInvite, name='create_group_invite'), + path('groups//', GroupDetail.as_view(), name='group_detail'), + path('groups//members//', removeGroupMember, name='remove_group_member'), + path('groups//invites/', createGroupInvite, name='create_group_invite'), path('groupinvites/', GroupInvitesIncoming.as_view(), name='group_invites_incoming'), path('groupinvites//', declineGroupInvite, name='decline_group_invite'), path('groupinvites//accept/', recordGroupMembership, name='record_group_membership'), diff --git a/backend/toolshed/api/inventory.py b/backend/toolshed/api/inventory.py index 833c00e..3f32258 100644 --- a/backend/toolshed/api/inventory.py +++ b/backend/toolshed/api/inventory.py @@ -15,6 +15,21 @@ from toolshed.serializers import InventoryItemSerializer, StorageLocationSeriali router = routers.SimpleRouter() +def resolve_group_by_handle(handle): + """handle is "name@domain" (no leading '#') - the same format/parser group.py's GroupDetail + uses, so a group reference parses identically everywhere it appears (URL path, ?group= query + param, or an owner_group payload field), rather than some spots taking a handle and others a + bare pk.""" + try: + name, domain = split_userhandle_or_throw(handle) + except ValueError: + return None + try: + return Group.objects.get(name=name, domain=domain) + except Group.DoesNotExist: + return None + + def inventory_items(identity): try: user = identity.user.get() @@ -54,13 +69,10 @@ class InventoryItemViewSet(viewsets.ModelViewSet): if identity.user.exists(): return InventoryItem.objects.filter(owner=identity.user.get()) | group_items return group_items - group_id = self.request.query_params.get('group') - if group_id: - try: - group = Group.objects.get(pk=group_id) - except Group.DoesNotExist: - return InventoryItem.objects.none() - if not group.is_member(identity): + group_handle = self.request.query_params.get('group') + if group_handle: + group = resolve_group_by_handle(group_handle) + if not group or not group.is_member(identity): return InventoryItem.objects.none() return InventoryItem.objects.filter(owner_group=group) if identity.user.exists(): @@ -68,12 +80,11 @@ class InventoryItemViewSet(viewsets.ModelViewSet): return InventoryItem.objects.none() def perform_create(self, serializer): - group_id = self.request.data.get('owner_group') + group_handle = self.request.data.get('owner_group') with transaction.atomic(): - if group_id: - try: - group = Group.objects.get(pk=group_id) - except Group.DoesNotExist: + if group_handle: + group = resolve_group_by_handle(group_handle) + if not group: raise NotFound('No such group') if not group.is_member(self.request.user): raise PermissionDenied('Not a member of this group') @@ -155,21 +166,53 @@ class StorageLocationViewSet(viewsets.ModelViewSet): lookup_url_kwarg = 'pk' def get_queryset(self): - if type(self.request.user) == KnownIdentity and self.request.user.user.exists(): - return StorageLocation.objects.filter(owner=self.request.user.user.get()) + # Mirrors InventoryItemViewSet.get_queryset() - see its own comments for why the + # list/detail scopes differ and why group membership alone (no linked ToolshedUser + # required) is enough for the group branch. + if type(self.request.user) != KnownIdentity: + return StorageLocation.objects.none() + identity = self.request.user + group_locations = StorageLocation.objects.filter(owner_group__in=identity.member_of_groups.all()) + if self.action != 'list': + if identity.user.exists(): + return StorageLocation.objects.filter(owner=identity.user.get()) | group_locations + return group_locations + group_handle = self.request.query_params.get('group') + if group_handle: + group = resolve_group_by_handle(group_handle) + if not group or not group.is_member(identity): + return StorageLocation.objects.none() + return StorageLocation.objects.filter(owner_group=group) + if identity.user.exists(): + return StorageLocation.objects.filter(owner=identity.user.get()) return StorageLocation.objects.none() def perform_create(self, serializer): + group_handle = self.request.data.get('owner_group') with transaction.atomic(): - serializer.save(owner=self.request.user.user.get()) + if group_handle: + group = resolve_group_by_handle(group_handle) + if not group: + raise NotFound('No such group') + if not group.is_member(self.request.user): + raise PermissionDenied('Not a member of this group') + serializer.save(owner=None, owner_group=group).clean() + else: + serializer.save(owner=self.request.user.user.get()).clean() + + @staticmethod + def _is_authorized(request, instance): + if instance.owner_id: + return request.user.user.filter(pk=instance.owner_id).exists() + return instance.owner_group.is_member(request.user) def perform_update(self, serializer): with transaction.atomic(): - if serializer.instance.owner == self.request.user.user.get(): - serializer.save() + if self._is_authorized(self.request, serializer.instance): + serializer.save().clean() def perform_destroy(self, instance): - if instance.owner == self.request.user.user.get(): + if self._is_authorized(self.request, instance): instance.delete() diff --git a/backend/toolshed/migrations/0023_remove_storagelocation_storagelocation_unique_owner_scoped_id_and_more.py b/backend/toolshed/migrations/0023_remove_storagelocation_storagelocation_unique_owner_scoped_id_and_more.py new file mode 100644 index 0000000..6ef08a2 --- /dev/null +++ b/backend/toolshed/migrations/0023_remove_storagelocation_storagelocation_unique_owner_scoped_id_and_more.py @@ -0,0 +1,63 @@ +# Generated by Django 4.2.2 on 2026-08-26 18:55 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('authentication', '0006_groupmembership'), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('toolshed', '0022_storagelocation_id_not_null_and_unique'), + ] + + operations = [ + migrations.RemoveConstraint( + model_name='storagelocation', + name='storagelocation_unique_owner_scoped_id', + ), + migrations.AddField( + model_name='ownerstoragelocationsequence', + name='owner_group', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='+', to='authentication.group'), + ), + migrations.AddField( + model_name='storagelocation', + name='owner_group', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='storage_locations', to='authentication.group'), + ), + migrations.AlterField( + model_name='owneritemsequence', + name='id', + field=models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID'), + ), + migrations.AlterField( + model_name='ownerstoragelocationsequence', + name='id', + field=models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID'), + ), + migrations.AlterField( + model_name='ownerstoragelocationsequence', + name='owner', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='+', to=settings.AUTH_USER_MODEL), + ), + migrations.AlterField( + model_name='storagelocation', + name='owner', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='storage_locations', to=settings.AUTH_USER_MODEL), + ), + migrations.AddConstraint( + model_name='ownerstoragelocationsequence', + constraint=models.UniqueConstraint(condition=models.Q(('owner__isnull', False)), fields=('owner',), name='ownerstoragelocationsequence_unique_owner'), + ), + migrations.AddConstraint( + model_name='ownerstoragelocationsequence', + constraint=models.UniqueConstraint(condition=models.Q(('owner_group__isnull', False)), fields=('owner_group',), name='ownerstoragelocationsequence_unique_owner_group'), + ), + migrations.AddConstraint( + model_name='storagelocation', + constraint=models.UniqueConstraint(fields=('owner', 'owner_group', 'id'), name='storagelocation_unique_owner_scoped_id'), + ), + ] diff --git a/backend/toolshed/models.py b/backend/toolshed/models.py index 6136a86..9989cba 100644 --- a/backend/toolshed/models.py +++ b/backend/toolshed/models.py @@ -170,15 +170,25 @@ class ItemTag(models.Model): class OwnerStorageLocationSequence(models.Model): - """Tracks the last StorageLocation id handed out per owner for sequential, gapless allocation - (see StorageLocation.create_for_owner).""" - owner = models.ForeignKey(ToolshedUser, on_delete=models.CASCADE, related_name='+', unique=True) + """Tracks the last StorageLocation id handed out per owner/owner_group scope for sequential, + gapless allocation (see StorageLocation.create_for_owner); exactly one of owner/owner_group is + set, mirroring OwnerItemSequence/InventoryItem's own split.""" + owner = models.ForeignKey(ToolshedUser, on_delete=models.CASCADE, null=True, blank=True, related_name='+') + owner_group = models.ForeignKey(Group, on_delete=models.CASCADE, null=True, blank=True, related_name='+') last_id = models.PositiveIntegerField(default=0) + class Meta: + constraints = [ + models.UniqueConstraint(fields=['owner'], condition=models.Q(owner__isnull=False), + name='ownerstoragelocationsequence_unique_owner'), + models.UniqueConstraint(fields=['owner_group'], condition=models.Q(owner_group__isnull=False), + name='ownerstoragelocationsequence_unique_owner_group'), + ] + @classmethod - def allocate(cls, *, owner): + def allocate(cls, *, owner=None, owner_group=None): with transaction.atomic(): - seq, _ = cls.objects.select_for_update().get_or_create(owner=owner) + seq, _ = cls.objects.select_for_update().get_or_create(owner=owner, owner_group=owner_group) seq.last_id += 1 seq.save(update_fields=['last_id']) return seq.last_id @@ -186,7 +196,7 @@ class OwnerStorageLocationSequence(models.Model): class StorageLocation(models.Model): internal_id = models.AutoField(primary_key=True) - # Externally visible id, sequential/gapless within the owner's own locations (see + # Externally visible id, sequential/gapless within the owner/owner_group's own locations (see # OwnerStorageLocationSequence), never internal_id; always allocate via create_for_owner, not # .objects.create(). id = models.PositiveIntegerField(editable=False) @@ -195,24 +205,32 @@ class StorageLocation(models.Model): category = models.ForeignKey(Category, on_delete=models.CASCADE, null=True, blank=True, related_name='storage_locations') parent = models.ForeignKey('self', on_delete=models.CASCADE, null=True, blank=True, related_name='children') - owner = models.ForeignKey(ToolshedUser, on_delete=models.CASCADE, related_name='storage_locations') + owner = models.ForeignKey(ToolshedUser, on_delete=models.CASCADE, null=True, blank=True, + related_name='storage_locations') + owner_group = models.ForeignKey(Group, on_delete=models.CASCADE, null=True, blank=True, + related_name='storage_locations') class Meta: constraints = [ - models.UniqueConstraint(fields=['owner', 'id'], name='storagelocation_unique_owner_scoped_id'), + models.UniqueConstraint(fields=['owner', 'owner_group', 'id'], + name='storagelocation_unique_owner_scoped_id'), ] def __str__(self): parent = str(self.parent) + "/" if self.parent else "" return parent + self.name + def clean(self): + if (self.owner is None) == (self.owner_group is None): + raise ValidationError("Exactly one of owner or owner_group must be set") + @classmethod - def create_for_owner(cls, *, owner, **kwargs): + def create_for_owner(cls, *, owner=None, owner_group=None, **kwargs): """The only supported way to create a StorageLocation: atomically allocates the next id - for this owner's scope.""" + for this owner/owner_group's scope.""" with transaction.atomic(): - next_id = OwnerStorageLocationSequence.allocate(owner=owner) - return cls.objects.create(owner=owner, id=next_id, **kwargs) + next_id = OwnerStorageLocationSequence.allocate(owner=owner, owner_group=owner_group) + return cls.objects.create(owner=owner, owner_group=owner_group, id=next_id, **kwargs) class WorkflowInstance(models.Model): diff --git a/backend/toolshed/serializers.py b/backend/toolshed/serializers.py index 0bfe6b0..846117b 100644 --- a/backend/toolshed/serializers.py +++ b/backend/toolshed/serializers.py @@ -1,4 +1,5 @@ from django.core.exceptions import ObjectDoesNotExist +from django.db.models import Q from rest_framework import serializers from authentication.models import KnownIdentity, ToolshedUser, FriendRequestIncoming, Group, GroupInviteIncoming, \ GroupMembership @@ -177,9 +178,13 @@ class OwnerScopedPrimaryKeyRelatedField(serializers.PrimaryKeyRelatedField): def get_queryset(self): queryset = super().get_queryset() request = self.context.get('request') - if request is not None and type(request.user) == KnownIdentity and request.user.user.exists(): - return queryset.filter(owner=request.user.user.get()) - return queryset.none() + if request is None or type(request.user) != KnownIdentity: + return queryset.none() + identity = request.user + owner_filter = Q(owner_group__in=identity.member_of_groups.all()) + if identity.user.exists(): + owner_filter |= Q(owner=identity.user.get()) + return queryset.filter(owner_filter) def to_internal_value(self, data): queryset = self.get_queryset() @@ -198,6 +203,7 @@ class OwnerScopedPrimaryKeyRelatedField(serializers.PrimaryKeyRelatedField): class StorageLocationSerializer(serializers.ModelSerializer): owner = OwnerSerializer(read_only=True) + owner_group = GroupOwnerSerializer(read_only=True) category = serializers.CharField(required=False, allow_null=True, allow_blank=True) parent = OwnerScopedPrimaryKeyRelatedField(queryset=StorageLocation.objects.all(), required=False, allow_null=True) @@ -205,7 +211,7 @@ class StorageLocationSerializer(serializers.ModelSerializer): class Meta: model = StorageLocation - fields = ['id', 'name', 'description', 'path', 'category', 'owner', 'parent'] + fields = ['id', 'name', 'description', 'path', 'category', 'owner', 'owner_group', 'parent'] read_only_fields = ['id', 'path'] @staticmethod diff --git a/backend/toolshed/tests/test_group.py b/backend/toolshed/tests/test_group.py index bf9ac7a..f3f1405 100644 --- a/backend/toolshed/tests/test_group.py +++ b/backend/toolshed/tests/test_group.py @@ -53,56 +53,47 @@ class GroupApiTestCase(UserTestMixin, GroupTestMixin, ToolshedTestCase): def test_group_detail_member(self): self.prepare_groups() - reply = client.get('/api/groups/{}/'.format(self.f['group1'].id), self.f['local_user1']) + group = self.f['group1'] + reply = client.get('/api/groups/{}@{}/'.format(group.name, group.domain), self.f['local_user1']) self.assertEqual(reply.status_code, 200) - self.assertEqual(reply.json()['handle'], str(self.f['group1'])) + self.assertEqual(reply.json()['handle'], str(group)) self.assertEqual(len(reply.json()['members']), 1) def test_group_detail_non_member(self): self.prepare_groups() - reply = client.get('/api/groups/{}/'.format(self.f['group1'].id), self.f['local_user2']) + group = self.f['group1'] + reply = client.get('/api/groups/{}@{}/'.format(group.name, group.domain), self.f['local_user2']) self.assertEqual(reply.status_code, 404) - def test_group_detail_by_handle_member(self): - self.prepare_groups() - group = self.f['group1'] - reply = client.get('/api/groups/handle/{}/{}/'.format(group.name, group.domain), self.f['local_user1']) - self.assertEqual(reply.status_code, 200) - self.assertEqual(reply.json()['id'], group.id) - self.assertEqual(reply.json()['handle'], str(group)) - - def test_group_detail_by_handle_non_member(self): - self.prepare_groups() - group = self.f['group1'] - reply = client.get('/api/groups/handle/{}/{}/'.format(group.name, group.domain), self.f['local_user2']) - self.assertEqual(reply.status_code, 404) - - def test_group_detail_by_handle_no_such_group(self): - reply = client.get('/api/groups/handle/nonexistent/example.com/', self.f['local_user1']) + def test_group_detail_no_such_group(self): + reply = client.get('/api/groups/nonexistent@example.com/', self.f['local_user1']) self.assertEqual(reply.status_code, 404) def test_remove_member(self): self.prepare_groups() - self.f['group1'].members.add(self.f['local_user2'].public_identity) - reply = client.delete('/api/groups/{}/members/{}/'.format( - self.f['group1'].id, self.f['local_user2'].public_identity.id), self.f['local_user1']) + group = self.f['group1'] + group.members.add(self.f['local_user2'].public_identity) + reply = client.delete('/api/groups/{}@{}/members/{}/'.format( + group.name, group.domain, self.f['local_user2'].public_identity.id), self.f['local_user1']) self.assertEqual(reply.status_code, 204) - self.assertEqual(self.f['group1'].members.count(), 1) + self.assertEqual(group.members.count(), 1) def test_remove_last_member_blocked(self): self.prepare_groups() - reply = client.delete('/api/groups/{}/members/{}/'.format( - self.f['group1'].id, self.f['local_user1'].public_identity.id), self.f['local_user1']) + group = self.f['group1'] + reply = client.delete('/api/groups/{}@{}/members/{}/'.format( + group.name, group.domain, self.f['local_user1'].public_identity.id), self.f['local_user1']) self.assertEqual(reply.status_code, 400) - self.assertEqual(self.f['group1'].members.count(), 1) + self.assertEqual(group.members.count(), 1) def test_remove_member_non_member_denied(self): self.prepare_groups() - self.f['group1'].members.add(self.f['local_user2'].public_identity) - reply = client.delete('/api/groups/{}/members/{}/'.format( - self.f['group1'].id, self.f['local_user1'].public_identity.id), self.f['ext_user1']) + group = self.f['group1'] + group.members.add(self.f['local_user2'].public_identity) + reply = client.delete('/api/groups/{}@{}/members/{}/'.format( + group.name, group.domain, self.f['local_user1'].public_identity.id), self.f['ext_user1']) self.assertEqual(reply.status_code, 404) - self.assertEqual(self.f['group1'].members.count(), 2) + self.assertEqual(group.members.count(), 2) class GroupInviteApiTestCase(UserTestMixin, GroupTestMixin, ToolshedTestCase): @@ -116,7 +107,8 @@ class GroupInviteApiTestCase(UserTestMixin, GroupTestMixin, ToolshedTestCase): inviter = self.f['local_user1'] invitee = self.f['local_user2'] - reply1 = client.post('/api/groups/{}/invites/'.format(group.id), inviter, {'invitee': str(invitee)}) + reply1 = client.post('/api/groups/{}@{}/invites/'.format(group.name, group.domain), inviter, + {'invitee': str(invitee)}) self.assertEqual(reply1.status_code, 201) secret = reply1.json()['secret'] self.assertEqual(GroupInvite.objects.count(), 1) @@ -152,7 +144,7 @@ class GroupInviteApiTestCase(UserTestMixin, GroupTestMixin, ToolshedTestCase): def test_invite_non_member_denied(self): group = self.f['group1'] - reply = client.post('/api/groups/{}/invites/'.format(group.id), self.f['local_user2'], + reply = client.post('/api/groups/{}@{}/invites/'.format(group.name, group.domain), self.f['local_user2'], {'invitee': str(self.f['ext_user1'])}) self.assertEqual(reply.status_code, 404) self.assertEqual(GroupInvite.objects.count(), 0) @@ -160,7 +152,7 @@ class GroupInviteApiTestCase(UserTestMixin, GroupTestMixin, ToolshedTestCase): def test_invite_already_member(self): group = self.f['group1'] group.members.add(self.f['local_user2'].public_identity) - reply = client.post('/api/groups/{}/invites/'.format(group.id), self.f['local_user1'], + reply = client.post('/api/groups/{}@{}/invites/'.format(group.name, group.domain), self.f['local_user1'], {'invitee': str(self.f['local_user2'])}) self.assertEqual(reply.status_code, 208) @@ -177,7 +169,7 @@ class GroupInviteApiTestCase(UserTestMixin, GroupTestMixin, ToolshedTestCase): group = self.f['group1'] inviter = self.f['local_user1'] invitee = self.f['local_user2'] - client.post('/api/groups/{}/invites/'.format(group.id), inviter, {'invitee': str(invitee)}) + client.post('/api/groups/{}@{}/invites/'.format(group.name, group.domain), inviter, {'invitee': str(invitee)}) reply = client.post('/api/group_invites/accept/', invitee, { 'group': str(group), 'invitee': str(invitee), 'invitee_key': invitee.public_key(), 'secret': 'not-the-right-secret' @@ -189,7 +181,8 @@ class GroupInviteApiTestCase(UserTestMixin, GroupTestMixin, ToolshedTestCase): group = self.f['group1'] inviter = self.f['local_user1'] invitee = DummyExternalUser('newmember', 'remote.example', known=False) - reply1 = client.post('/api/groups/{}/invites/'.format(group.id), inviter, {'invitee': str(invitee)}) + reply1 = client.post('/api/groups/{}@{}/invites/'.format(group.name, group.domain), inviter, + {'invitee': str(invitee)}) secret = reply1.json()['secret'] reply = client.post('/api/group_invites/accept/', invitee, { @@ -206,7 +199,8 @@ class GroupInviteApiTestCase(UserTestMixin, GroupTestMixin, ToolshedTestCase): group = self.f['group1'] inviter = self.f['local_user1'] invitee = self.f['local_user2'] - reply1 = client.post('/api/groups/{}/invites/'.format(group.id), inviter, {'invitee': str(invitee)}) + reply1 = client.post('/api/groups/{}@{}/invites/'.format(group.name, group.domain), inviter, + {'invitee': str(invitee)}) secret = reply1.json()['secret'] bad_signature_client = SignatureAuthClient(bad_signature=True) reply = bad_signature_client.post('/api/group_invites/accept/', invitee, { diff --git a/backend/toolshed/tests/test_inventory.py b/backend/toolshed/tests/test_inventory.py index eba0c12..8a1b1a5 100644 --- a/backend/toolshed/tests/test_inventory.py +++ b/backend/toolshed/tests/test_inventory.py @@ -353,7 +353,7 @@ class GroupOwnedInventoryApiTestCase(UserTestMixin, GroupTestMixin, CategoryTest def create_group_item(self, name='drill'): return client.post('/api/inventory_items/', self.f['local_user1'], { 'name': name, 'owned_quantity': 1, 'availability_policy': 'private', - 'owner_group': self.f['group1'].id, + 'owner_group': str(self.f['group1'])[1:], }) def test_create_group_owned_item(self): @@ -368,7 +368,7 @@ class GroupOwnedInventoryApiTestCase(UserTestMixin, GroupTestMixin, CategoryTest def test_create_group_owned_item_non_member_denied(self): reply = client.post('/api/inventory_items/', self.f['ext_user1'], { 'name': 'drill', 'owned_quantity': 1, 'availability_policy': 'private', - 'owner_group': self.f['group1'].id, + 'owner_group': str(self.f['group1'])[1:], }) self.assertEqual(reply.status_code, 403) self.assertEqual(InventoryItem.objects.count(), 0) @@ -413,14 +413,14 @@ class GroupOwnedInventoryApiTestCase(UserTestMixin, GroupTestMixin, CategoryTest def test_group_items_listed_by_group_query_param(self): self.create_group_item() - reply = client.get('/api/inventory_items/?group={}'.format(self.f['group1'].id), self.f['local_user2']) + reply = client.get('/api/inventory_items/?group={}'.format(str(self.f['group1'])[1:]), self.f['local_user2']) self.assertEqual(reply.status_code, 200) self.assertEqual(len(reply.json()), 1) self.assertEqual(reply.json()[0]['name'], 'drill') def test_group_items_not_listed_for_non_member_query_param(self): self.create_group_item() - reply = client.get('/api/inventory_items/?group={}'.format(self.f['group1'].id), self.f['ext_user1']) + reply = client.get('/api/inventory_items/?group={}'.format(str(self.f['group1'])[1:]), self.f['ext_user1']) self.assertEqual(reply.status_code, 200) self.assertEqual(len(reply.json()), 0) @@ -435,7 +435,7 @@ class GroupOwnedInventoryApiTestCase(UserTestMixin, GroupTestMixin, CategoryTest 'owned_quantity': 3, 'tags': ['tag1', 'tag2'], 'properties': [{'name': 'prop1', 'value': 'value1'}, {'name': 'prop2', 'value': 'value2'}], - 'owner_group': self.f['group1'].id, + 'owner_group': str(self.f['group1'])[1:], }) self.assertEqual(reply.status_code, 201) item = InventoryItem.objects.get(name='drill') @@ -451,20 +451,21 @@ class GroupOwnedInventoryApiTestCase(UserTestMixin, GroupTestMixin, CategoryTest def test_create_group_owned_item_empty_fails(self): # Parity with InventoryApiTestCase.test_post_new_item_empty: clean()'s name-or-files validation still applies. reply = client.post('/api/inventory_items/', self.f['local_user1'], { - 'availability_policy': 'private', 'owned_quantity': 1, 'owner_group': self.f['group1'].id, + 'availability_policy': 'private', 'owned_quantity': 1, 'owner_group': str(self.f['group1'])[1:], }) self.assertEqual(reply.status_code, 400) self.assertEqual(InventoryItem.objects.count(), 0) def test_create_group_owned_item_nonexistent_group(self): reply = client.post('/api/inventory_items/', self.f['local_user1'], { - 'name': 'drill', 'owned_quantity': 1, 'availability_policy': 'private', 'owner_group': 999999, + 'name': 'drill', 'owned_quantity': 1, 'availability_policy': 'private', + 'owner_group': 'nonexistent@example.com', }) self.assertEqual(reply.status_code, 404) self.assertEqual(InventoryItem.objects.count(), 0) def test_group_items_listed_for_nonexistent_group_query_param(self): - reply = client.get('/api/inventory_items/?group=999999', self.f['local_user1']) + reply = client.get('/api/inventory_items/?group=nonexistent@example.com', self.f['local_user1']) self.assertEqual(reply.status_code, 200) self.assertEqual(len(reply.json()), 0) @@ -495,7 +496,7 @@ class GroupOwnedInventoryApiTestCase(UserTestMixin, GroupTestMixin, CategoryTest reply = client.post('/api/inventory_items/', self.f['local_user1'], { 'name': 'drill', 'owned_quantity': 1, 'availability_policy': 'private', 'category': 'cat1', 'tags': ['tag1'], - 'owner_group': self.f['group1'].id, + 'owner_group': str(self.f['group1'])[1:], }) item_id = reply.json()['id'] reply = client.patch('/api/inventory_items/{}/'.format(item_id), self.f['local_user2'], { @@ -509,7 +510,7 @@ class GroupOwnedInventoryApiTestCase(UserTestMixin, GroupTestMixin, CategoryTest def test_group_item_storage_location(self): reply = client.post('/api/inventory_items/', self.f['local_user1'], { 'name': 'drill', 'owned_quantity': 1, 'availability_policy': 'private', - 'storage_location': self.f['loc1'].id, 'owner_group': self.f['group1'].id, + 'storage_location': self.f['loc1'].id, 'owner_group': str(self.f['group1'])[1:], }) self.assertEqual(reply.status_code, 201) item = InventoryItem.objects.get(name='drill') @@ -519,7 +520,7 @@ class GroupOwnedInventoryApiTestCase(UserTestMixin, GroupTestMixin, CategoryTest # Parity with TestInventoryItemWithFileApiTestCase.test_post_item_with_file_id. reply = client.post('/api/inventory_items/', self.f['local_user1'], { 'name': 'drill', 'owned_quantity': 1, 'availability_policy': 'private', - 'files': [self.f['test_file1'].id], 'owner_group': self.f['group1'].id, + 'files': [self.f['test_file1'].id], 'owner_group': str(self.f['group1'])[1:], }) self.assertEqual(reply.status_code, 201) item = InventoryItem.objects.get(name='drill') @@ -529,7 +530,7 @@ class GroupOwnedInventoryApiTestCase(UserTestMixin, GroupTestMixin, CategoryTest reply = client.post('/api/inventory_items/', self.f['local_user1'], { 'name': 'drill', 'owned_quantity': 1, 'availability_policy': 'private', 'files': [{'data': self.f['encoded_content3'], 'mime_type': 'text/plain'}], - 'owner_group': self.f['group1'].id, + 'owner_group': str(self.f['group1'])[1:], }) self.assertEqual(reply.status_code, 201) item = InventoryItem.objects.get(name='drill') diff --git a/backend/toolshed/tests/test_locations.py b/backend/toolshed/tests/test_locations.py index 438775d..6850db4 100644 --- a/backend/toolshed/tests/test_locations.py +++ b/backend/toolshed/tests/test_locations.py @@ -1,4 +1,4 @@ -from authentication.tests import SignatureAuthClient, UserTestMixin, ToolshedTestCase +from authentication.tests import SignatureAuthClient, UserTestMixin, GroupTestMixin, ToolshedTestCase from files.tests import FilesTestMixin from toolshed.models import InventoryItem, Category, StorageLocation from toolshed.tests import InventoryTestMixin, LocationTestMixin @@ -129,3 +129,110 @@ class LocationApiTestCase(UserTestMixin, InventoryTestMixin, LocationTestMixin, self.assertEqual(reply.status_code, 404) self.assertEqual(StorageLocation.objects.filter(id=self.f['loc1'].id).count(), 1) + +class GroupOwnedLocationApiTestCase(UserTestMixin, GroupTestMixin, ToolshedTestCase): + def setUp(self): + super().setUp() + self.prepare_users() + self.prepare_groups() + self.f['group1'].members.add(self.f['local_user2'].public_identity) + + def create_group_location(self, name='shelf'): + return client.post('/api/storage_locations/', self.f['local_user1'], { + 'name': name, 'owner_group': str(self.f['group1'])[1:], + }) + + def test_create_group_owned_location(self): + reply = self.create_group_location() + self.assertEqual(reply.status_code, 201) + location = StorageLocation.objects.get(name='shelf') + self.assertIsNone(location.owner) + self.assertEqual(location.owner_group, self.f['group1']) + self.assertEqual(reply.json()['owner_group'], str(self.f['group1'])) + self.assertIsNone(reply.json()['owner']) + + def test_create_group_owned_location_non_member_denied(self): + reply = client.post('/api/storage_locations/', self.f['ext_user1'], { + 'name': 'shelf', 'owner_group': str(self.f['group1'])[1:], + }) + self.assertEqual(reply.status_code, 403) + self.assertEqual(StorageLocation.objects.count(), 0) + + def test_other_member_can_edit(self): + location_id = self.create_group_location().json()['id'] + reply = client.patch('/api/storage_locations/{}/'.format(location_id), self.f['local_user2'], { + 'name': 'shelf-renamed' + }) + self.assertEqual(reply.status_code, 200) + self.assertEqual(StorageLocation.objects.get(id=location_id).name, 'shelf-renamed') + + def test_other_member_can_delete(self): + location_id = self.create_group_location().json()['id'] + reply = client.delete('/api/storage_locations/{}/'.format(location_id), self.f['local_user2']) + self.assertEqual(reply.status_code, 204) + self.assertEqual(StorageLocation.objects.filter(id=location_id).count(), 0) + + def test_remote_member_without_local_account_can_edit(self): + # A remote member (KnownIdentity, no ToolshedUser row) must still act on group-owned + # locations - not unauthorized just because .user.exists() is False. + self.f['group1'].members.add(self.f['ext_user1'].public_identity) + location_id = self.create_group_location().json()['id'] + reply = client.get('/api/storage_locations/{}/'.format(location_id), self.f['ext_user1']) + self.assertEqual(reply.status_code, 200) + reply = client.patch('/api/storage_locations/{}/'.format(location_id), self.f['ext_user1'], { + 'name': 'shelf-renamed-by-remote-member' + }) + self.assertEqual(reply.status_code, 200) + self.assertEqual(StorageLocation.objects.get(id=location_id).name, 'shelf-renamed-by-remote-member') + + def test_non_member_cannot_see_or_edit(self): + location_id = self.create_group_location().json()['id'] + reply = client.get('/api/storage_locations/{}/'.format(location_id), self.f['ext_user1']) + self.assertEqual(reply.status_code, 404) + + def test_group_locations_excluded_from_personal_list(self): + self.create_group_location() + reply = client.get('/api/storage_locations/', self.f['local_user1']) + self.assertEqual(reply.status_code, 200) + self.assertEqual(len(reply.json()), 0) + + def test_group_locations_listed_by_group_query_param(self): + self.create_group_location() + reply = client.get('/api/storage_locations/?group={}'.format(str(self.f['group1'])[1:]), self.f['local_user2']) + self.assertEqual(reply.status_code, 200) + self.assertEqual(len(reply.json()), 1) + self.assertEqual(reply.json()[0]['name'], 'shelf') + + def test_group_locations_not_listed_for_non_member_query_param(self): + self.create_group_location() + reply = client.get('/api/storage_locations/?group={}'.format(str(self.f['group1'])[1:]), self.f['ext_user1']) + self.assertEqual(reply.status_code, 200) + self.assertEqual(len(reply.json()), 0) + + def test_group_location_can_be_parent_for_group_member(self): + parent_id = self.create_group_location('shelf').json()['id'] + reply = client.post('/api/storage_locations/', self.f['local_user2'], { + 'name': 'bin', 'owner_group': str(self.f['group1'])[1:], 'parent': parent_id, + }) + self.assertEqual(reply.status_code, 201) + self.assertEqual(reply.json()['path'], 'shelf/bin') + + def test_own_personal_location_can_be_parent_of_group_location(self): + # OwnerScopedPrimaryKeyRelatedField (see serializers.py) scopes `parent` to every + # location the requester can act on - own personal ones plus any group's - with no + # further check that it matches the new location's own owner_group. Same behavior + # InventoryItem.storage_location already had before group ownership existed here; not + # something this feature narrows. + personal = StorageLocation.create_for_owner(name='mine', owner=self.f['local_user1']) + reply = client.post('/api/storage_locations/', self.f['local_user1'], { + 'name': 'bin', 'owner_group': str(self.f['group1'])[1:], 'parent': personal.id, + }) + self.assertEqual(reply.status_code, 201) + + def test_group_location_not_valid_parent_for_non_member(self): + parent_id = self.create_group_location('shelf').json()['id'] + reply = client.post('/api/storage_locations/', self.f['ext_user1'], { + 'name': 'bin', 'parent': parent_id, + }) + self.assertEqual(reply.status_code, 400) + diff --git a/frontend/src/router.js b/frontend/src/router.js index b37ece4..fd8d9d1 100644 --- a/frontend/src/router.js +++ b/frontend/src/router.js @@ -51,6 +51,11 @@ function itemDetailRoute(handle, item_local_id) { return handle ? `/inventory/${encodeHandleForUrl(handle)}/${item_local_id}` : null; } +// storage_location/group_storage_location share this /storage-locations/:handle/:id shape, same idea as itemDetailRoute above. +function storageLocationDetailRoute(handle, storage_location_id) { + return handle ? `/storage-locations/${encodeHandleForUrl(handle)}/${storage_location_id}` : null; +} + export function ownerOverviewRoute(item) { return item.owner_group ? `/groups/${encodeHandleForUrl(item.owner_group)}` : '/inventory'; } @@ -64,12 +69,15 @@ const EXPANDED_ROUTE_BUILDERS = { const handle = store.getters.groupHandleById[group_id]; return handle ? `/groups/${encodeHandleForUrl(handle)}` : null; }, - storage_location: ({storage_location_id}) => `/storage-locations/${storage_location_id}`, + storage_location: ({owner_identity_id, storage_location_id}) => + storageLocationDetailRoute(store.getters.identityHandleById[owner_identity_id], storage_location_id), + group_storage_location: ({owner_group_id, storage_location_id}) => + storageLocationDetailRoute(store.getters.groupHandleById[owner_group_id], storage_location_id), workflow: ({workflow_id}) => `/workflows/${workflow_id}`, }; // Kinds whose route needs identityHandleById/groupHandleById (from state.idmap); ShortId.vue only waits on an idmap fetch for these. -export const NEEDS_IDMAP = new Set(['item', 'group_item', 'group']); +export const NEEDS_IDMAP = new Set(['item', 'group_item', 'group', 'storage_location', 'group_storage_location']); export function expandedRoute({kind, ...fields}) { const buildRoute = EXPANDED_ROUTE_BUILDERS[kind]; @@ -188,12 +196,12 @@ const routes = [{path: '/', component: Dashboard, meta: {requiresAuth: true}}, { component: StorageLocation, meta: {requiresAuth: true} }, { - path: '/storage-locations/:id', + path: '/storage-locations/:handle/:id', component: StorageLocationDetail, meta: {requiresAuth: true}, props: true }, { - path: '/storage-locations/:id/edit', + path: '/storage-locations/:handle/:id/edit', component: StorageLocationEdit, meta: {requiresAuth: true}, props: true diff --git a/frontend/src/short-id.js b/frontend/src/short-id.js index f26e4f7..1f79672 100644 --- a/frontend/src/short-id.js +++ b/frontend/src/short-id.js @@ -40,6 +40,12 @@ const SCHEMAS = { fields: ['category_id'], interpret: ([category_id]) => ({kind: 'category', category_id}) }, + '4-2': { + name: 'group_storage_location', + fields: ['owner_group_id', 'storage_location_id'], + interpret: ([owner_group_id, storage_location_id]) => + ({kind: 'group_storage_location', owner_group_id, storage_location_id}) + }, } const BASE64URL_ALPHABET = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_' diff --git a/frontend/src/store.js b/frontend/src/store.js index 162b980..b89377d 100644 --- a/frontend/src/store.js +++ b/frontend/src/store.js @@ -83,6 +83,7 @@ export default createStore({ idmap: {identities: [], groups: []}, idmapLoaded: false, item_map: {}, + location_map: {}, home_servers: null, all_friends_servers: null, messages: [], @@ -105,6 +106,9 @@ export default createStore({ setInventoryItems(state, {url, items}) { state.item_map[url] = items; }, + setLocationsForKey(state, {url, locations}) { + state.location_map[url] = locations; + }, setFriends(state, friends) { state.friends = friends; }, @@ -397,16 +401,17 @@ export default createStore({ return items }, async createInventoryItem({state, dispatch, getters}, item) { - const servers = await dispatch('getHomeServers') - const data = {availability_policy: 'private', ...item} + const servers = item.owner_group + ? await dispatch('getFriendServers', {username: 'x@' + splitGroupHandle(item.owner_group).domain}) + : await dispatch('getHomeServers') + const data = { + availability_policy: 'private', ...item, + owner_group: item.owner_group ? item.owner_group.slice(1) : null + } const reply = await servers.post(getters.signAuth, '/api/inventory_items/', data) state.last_load.files = 0 return reply }, - // A group-owned item lives on its owning group's own backend, not necessarily the - // caller's home one (see the group actions below) - item.owner_group ("#name@domain") - // already carries that domain, so update/delete resolve it the same way rather than - // needing the caller to track it separately. async updateInventoryItem({state, dispatch, getters}, item) { const servers = item.owner_group ? await dispatch('getFriendServers', {username: 'x@' + splitGroupHandle(item.owner_group).domain}) @@ -465,16 +470,10 @@ export default createStore({ return null; } }, - // A group handle resolves via fetchGroup (itself domain-routed, see below), so a group - // hosted elsewhere works exactly like one hosted on the caller's own backend - no idmap - // detour needed here. ServerSet.get() never checks response.ok, so a 404 (nonexistent or - // non-member group) surfaces as a JSON-parse rejection rather than resolving falsy - same - // try/catch shape as fetchForeignItem. async fetchItemByHandle({dispatch, getters}, {handle, id}) { if (handle.startsWith('#')) { try { - const group = await dispatch('fetchGroup', {handle}) - const items = await dispatch('fetchGroupInventoryItems', {groupId: group.id, domain: group.domain}) + const items = await dispatch('fetchGroupInventoryItems', {groupHandle: handle}) return items.find(item => item.id === parseInt(id)) || null } catch (error) { console.error(`Failed to fetch group item ${id} for ${handle}:`, error); @@ -530,14 +529,6 @@ export default createStore({ const servers = await dispatch('getHomeServers') return await servers.delete(getters.signAuth, '/api/friends/' + id + '/') }, - // Listing/creating a group is inherently home-scoped - it's this backend's own view of - // "groups I host you in" - but every action below that acts on an *existing* group - // (found via its handle, e.g. from a GroupMembership pointer to a group hosted - // elsewhere - see GroupMembership/docs/design-in-progress/groups-mvp.md) resolves that - // group's own domain and talks to it directly, the same way requestFriend/ - // fetchForeignItem resolve a friend's domain, rather than assuming home. A group hosted - // on the caller's own domain goes through exactly the same code path - there's nothing - // to special-case. async fetchGroups({commit, dispatch, getters}) { const servers = await dispatch('getHomeServers') const data = await servers.get(getters.signAuth, '/api/groups/') @@ -552,17 +543,19 @@ export default createStore({ }, // handle is "#name@domain". async fetchGroup({dispatch, getters}, {handle}) { - const {name, domain} = splitGroupHandle(handle) + const {domain} = splitGroupHandle(handle) const servers = await dispatch('getFriendServers', {username: 'x@' + domain}) - return await servers.get(getters.signAuth, `/api/groups/handle/${name}/${domain}/`) + return await servers.get(getters.signAuth, `/api/groups/${handle.slice(1)}/`) }, async createGroup({dispatch, getters}, {name}) { const servers = await dispatch('getHomeServers') return await servers.post(getters.signAuth, '/api/groups/', {name}) }, - async removeGroupMember({dispatch, getters}, {groupId, domain, identityId}) { + async removeGroupMember({dispatch, getters}, {groupHandle, identityId}) { + const {domain} = splitGroupHandle(groupHandle) const servers = await dispatch('getFriendServers', {username: 'x@' + domain}) - return await servers.delete(getters.signAuth, '/api/groups/' + groupId + '/members/' + identityId + '/') + return await servers.delete( + getters.signAuth, '/api/groups/' + groupHandle.slice(1) + '/members/' + identityId + '/') }, async fetchGroupInvites({commit, dispatch, getters}) { const servers = await dispatch('getHomeServers') @@ -576,10 +569,11 @@ export default createStore({ commit('setGroupMemberships', data) return data }, - async inviteToGroup({state, dispatch, getters}, {groupId, domain, groupHandle, invitee}) { + async inviteToGroup({state, dispatch, getters}, {groupHandle, invitee}) { + const {domain} = splitGroupHandle(groupHandle) const group_servers = await dispatch('getFriendServers', {username: 'x@' + domain}) const group_reply = await group_servers.post( - getters.signAuth, '/api/groups/' + groupId + '/invites/', {invitee}) + getters.signAuth, '/api/groups/' + groupHandle.slice(1) + '/invites/', {invitee}) if (!group_reply.secret) { return false } @@ -610,13 +604,14 @@ export default createStore({ const servers = await dispatch('getHomeServers') return await servers.delete(getters.signAuth, '/api/groupinvites/' + invite.id + '/') }, - async fetchGroupInventoryItems({commit, dispatch, getters}, {groupId, domain}) { + async fetchGroupInventoryItems({commit, dispatch, getters}, {groupHandle}) { + const {domain} = splitGroupHandle(groupHandle) const servers = await dispatch('getFriendServers', {username: 'x@' + domain}) - const items = await servers.get(getters.signAuth, '/api/inventory_items/?group=' + groupId) - // Namespaced by domain, not just groupId: that pk is only unique within its own + const items = await servers.get(getters.signAuth, '/api/inventory_items/?group=' + groupHandle.slice(1)) + // Keyed by the full handle, not a bare id: a group pk is only unique within its own // backend's database, so two different domains could otherwise collide on the same - // item_map key. - commit('setInventoryItems', {url: '/group/' + domain + '/' + groupId, items}) + // item_map key - the handle already carries the domain, so no separate namespacing is needed. + commit('setInventoryItems', {url: '/group/' + groupHandle, items}) return items }, async fetchFiles({state, commit, dispatch, getters}) { @@ -722,23 +717,50 @@ export default createStore({ commit('setStorageLocations', data) state.last_load.storage_locations = Date.now() return data + },async fetchGroupStorageLocations({commit, dispatch, getters}, {groupHandle}) { + const {domain} = splitGroupHandle(groupHandle) + const servers = await dispatch('getFriendServers', {username: 'x@' + domain}) + const locations = await servers.get(getters.signAuth, '/api/storage_locations/?group=' + groupHandle.slice(1)) + commit('setLocationsForKey', {url: '/group/' + groupHandle, locations}) + return locations + }, + async fetchStorageLocationByHandle({dispatch, getters}, {handle, id}) { + if (handle.startsWith('#')) { + try { + const locations = await dispatch('fetchGroupStorageLocations', {groupHandle: handle}) + return locations.find(location => location.id === parseInt(id)) || null + } catch (error) { + console.error(`Failed to fetch group storage location ${id} for ${handle}:`, error); + return null; + } + } + const locations = await dispatch('fetchStorageLocations') + return locations.find(location => location.id === parseInt(id)) || null }, async deleteStorageLocation({state, dispatch, getters}, location) { - const servers = await dispatch('getHomeServers') + const servers = location.owner_group + ? await dispatch('getFriendServers', {username: 'x@' + splitGroupHandle(location.owner_group).domain}) + : await dispatch('getHomeServers') const ret = await servers.delete(getters.signAuth, '/api/storage_locations/' + location.id + '/') dispatch('fetchStorageLocations') return ret }, + // location.owner_group is the group's own "#name@domain" handle - see + // createInventoryItem's own comment. async createStorageLocation({state, dispatch, getters}, location) { - const servers = await dispatch('getHomeServers') - const data = {...location} + const servers = location.owner_group + ? await dispatch('getFriendServers', {username: 'x@' + splitGroupHandle(location.owner_group).domain}) + : await dispatch('getHomeServers') + const data = {...location, owner_group: location.owner_group ? location.owner_group.slice(1) : null} if (data.parent === '') data.parent = null const reply = await servers.post(getters.signAuth, '/api/storage_locations/', data) state.last_load.storage_locations = 0 return reply }, async updateStorageLocation({state, dispatch, getters}, location) { - const servers = await dispatch('getHomeServers') + const servers = location.owner_group + ? await dispatch('getFriendServers', {username: 'x@' + splitGroupHandle(location.owner_group).domain}) + : await dispatch('getHomeServers') const data = {...location} if (data.parent === '') data.parent = null const reply = await servers.patch(getters.signAuth, '/api/storage_locations/' + location.id + '/', data) @@ -848,15 +870,14 @@ export default createStore({ inventory_items(state) { return state.item_map['/'] || [] }, - groupInventoryItems: (state) => (groupId, domain) => state.item_map['/group/' + domain + '/' + groupId] || [], + groupInventoryItems: (state) => (groupHandle) => state.item_map['/group/' + groupHandle] || [], + groupStorageLocations: (state) => (groupHandle) => state.location_map['/group/' + groupHandle] || [], identityIdByHandle(state) { return Object.fromEntries(state.idmap.identities.map(i => [i.username, i.id])) }, groupIdByHandle(state) { return Object.fromEntries(state.idmap.groups.map(g => [g.handle, g.id])) }, - // Reverse of the two getters above: turns a short-id's raw owner_identity_id/owner_group_id - // back into a handle (see router.js EXPANDED_ROUTE_BUILDERS), no backend lookup needed. identityHandleById(state) { return Object.fromEntries(state.idmap.identities.map(i => [i.id, i.username])) }, diff --git a/frontend/src/tests/short-id.js b/frontend/src/tests/short-id.js index df53525..8e92fb5 100644 --- a/frontend/src/tests/short-id.js +++ b/frontend/src/tests/short-id.js @@ -21,6 +21,7 @@ test('every kind round-trips through serialize/encode/decode/deserialize', () => {kind: 'group_item', owner_group_id: 5, item_local_id: 42}, {kind: 'group', group_id: 11}, {kind: 'storage_location', owner_identity_id: 3, storage_location_id: 1000}, + {kind: 'group_storage_location', owner_group_id: 5, storage_location_id: 1000}, {kind: 'file', file_id: 123}, {kind: 'workflow', owner_identity_id: 2, workflow_id: 9}, ] diff --git a/frontend/src/views/GroupDetail.vue b/frontend/src/views/GroupDetail.vue index 21eaaa9..b079b59 100644 --- a/frontend/src/views/GroupDetail.vue +++ b/frontend/src/views/GroupDetail.vue @@ -127,7 +127,55 @@
- Add + +
+ +
+
+
+
Group storage locations
+
+ + + + + + + + + + + + + + + + + +
NamePathCategoryActions
+ {{ location.name }} + + {{ location.path }} + + {{ location.category }} + - + + + + + + + + + + +
+
+
+ + Add
@@ -139,7 +187,7 @@ diff --git a/frontend/src/views/InventoryNew.vue b/frontend/src/views/InventoryNew.vue index 3c1065e..7042431 100644 --- a/frontend/src/views/InventoryNew.vue +++ b/frontend/src/views/InventoryNew.vue @@ -28,7 +28,7 @@ @@ -84,7 +84,7 @@ import BaseLayout from "@/components/BaseLayout.vue"; import TagField from "@/components/TagField.vue"; import PropertyField from "@/components/PropertyField.vue"; import CombinedFileField from "@/components/CombinedFileField.vue"; -import {ownerOverviewRoute} from "@/router"; +import {decodeHandleFromUrl, ownerOverviewRoute} from "@/router"; export default { name: "InventoryNew", @@ -107,23 +107,34 @@ export default { properties: [], files: [], storage_location: null, + // The group's own "#name@domain" handle once picked, or null for a personal + // item - createInventoryItem resolves the target domain from it directly. owner_group: null } } }, methods: { ownerOverviewRoute, - ...mapActions(['createInventoryItem', 'fetchInfo', 'fetchStorageLocations', 'fetchGroups']) + ...mapActions(['createInventoryItem', 'fetchInfo', 'fetchStorageLocations', 'fetchGroups', + 'fetchGroupMemberships']) }, computed: { - ...mapState(["availability_policies", "storage_locations", "groups"]), + ...mapState(["availability_policies", "storage_locations", "groups", "groupMemberships"]), + // Groups hosted here plus groups only known via a GroupMembership pointer (see + // Groups.vue's allGroups for the same merge/dedupe). + ownerGroups() { + const hostedHandles = new Set(this.groups.map(group => group.handle)) + const foreign = this.groupMemberships.filter(m => !hostedHandles.has(m.handle)) + return [...this.groups, ...foreign].sort((a, b) => a.handle.localeCompare(b.handle)) + } }, async mounted() { await this.fetchInfo(); await this.fetchStorageLocations(); await this.fetchGroups(); + await this.fetchGroupMemberships(); if (this.$route.query.group) { - this.item.owner_group = parseInt(this.$route.query.group, 10) + this.item.owner_group = decodeHandleFromUrl(this.$route.query.group) } } } diff --git a/frontend/src/views/Print.vue b/frontend/src/views/Print.vue index bca75a7..4b1ffab 100644 --- a/frontend/src/views/Print.vue +++ b/frontend/src/views/Print.vue @@ -464,10 +464,18 @@ export default { } if (f.userHandle.startsWith("#")) { const owner_group_id = this.groupIdByHandle[f.userHandle]; - if (owner_group_id === undefined || !f.itemId) { + if (owner_group_id === undefined) { return null; } - return shortenedRoute({kind: "group_item", owner_group_id, item_local_id: f.itemId}).slice(1); + if (f.itemId) { + return shortenedRoute({kind: "group_item", owner_group_id, item_local_id: f.itemId}).slice(1); + } + if (f.locationId) { + return shortenedRoute({ + kind: "group_storage_location", owner_group_id, storage_location_id: f.locationId + }).slice(1); + } + return null; } const owner_identity_id = this.identityIdByHandle[f.userHandle]; if (owner_identity_id === undefined) { diff --git a/frontend/src/views/Scan.vue b/frontend/src/views/Scan.vue index 918bf92..719f058 100644 --- a/frontend/src/views/Scan.vue +++ b/frontend/src/views/Scan.vue @@ -154,7 +154,7 @@ export default { }; }, methods: { - ...mapActions(["fetchItemByHandle", "fetchStorageLocations", "fetchIdMap"]), + ...mapActions(["fetchItemByHandle", "fetchStorageLocationByHandle", "fetchIdMap"]), resolveDescription(entry) { const {link, text} = entry; @@ -219,12 +219,19 @@ export default { } return handle === undefined ? null : handle; } - if (decoded.kind === "storage_location") { - if (!this.$store.state.storage_locations.length) { - await this.fetchStorageLocations(); + if (decoded.kind === "storage_location" || decoded.kind === "group_storage_location") { + const byId = decoded.kind === "storage_location" + ? this.$store.getters.identityHandleById + : this.$store.getters.groupHandleById; + const ownerId = decoded.kind === "storage_location" ? decoded.owner_identity_id : decoded.owner_group_id; + let handle = byId[ownerId]; + if (handle === undefined) { + await this.fetchIdMap(); + handle = (decoded.kind === "storage_location" + ? this.$store.getters.identityHandleById + : this.$store.getters.groupHandleById)[ownerId]; } - const location = this.$store.state.storage_locations.find(l => l.id === decoded.storage_location_id); - return location ? `[#${location.id}] ${location.name}` : null; + return handle === undefined ? null : this.describeLocation(handle, decoded.storage_location_id); } return null; // workflow, category, file: no per-item title lookup wired up yet }, @@ -234,6 +241,11 @@ export default { return item ? `[#${item.id}] ${item.name}` : null; }, + async describeLocation(handle, id) { + const location = await this.fetchStorageLocationByHandle({handle, id}); + return location ? `[#${location.id}] ${location.name}` : null; + }, + // Handler for CameraScanner's "scan" event - logs every code from the batch it decoded. onCameraScan(codes) { codes.forEach(this.logDecode); diff --git a/frontend/src/views/ShortId.vue b/frontend/src/views/ShortId.vue index 03fff50..d71ba81 100644 --- a/frontend/src/views/ShortId.vue +++ b/frontend/src/views/ShortId.vue @@ -73,6 +73,7 @@ const EXAMPLES = [ {kind: 'group_item', owner_group_id: 5, item_local_id: 42}, {kind: 'file', file_id: 123}, {kind: 'storage_location', owner_identity_id: 3, storage_location_id: 1000}, + {kind: 'group_storage_location', owner_group_id: 5, storage_location_id: 1000}, {kind: 'category', category_id: 5}, {kind: 'workflow', owner_identity_id: 2, workflow_id: 9}, ]; diff --git a/frontend/src/views/StorageLocation.vue b/frontend/src/views/StorageLocation.vue index 3d23bb7..579788d 100644 --- a/frontend/src/views/StorageLocation.vue +++ b/frontend/src/views/StorageLocation.vue @@ -27,7 +27,7 @@ - {{ location.name }} + {{ location.name }} {{ location.path }} @@ -37,10 +37,10 @@ - - + - + @@ -60,7 +60,7 @@
- + {{ location.name }}
@@ -75,7 +75,7 @@ - Edit
-
{{ location.owner?.username || '-' }}
+
{{ location.owner || location.owner_group || '-' }}
-
- +
@@ -47,7 +52,8 @@ diff --git a/frontend/src/views/StorageLocationEdit.vue b/frontend/src/views/StorageLocationEdit.vue index b792950..6d330c3 100644 --- a/frontend/src/views/StorageLocationEdit.vue +++ b/frontend/src/views/StorageLocationEdit.vue @@ -55,6 +55,7 @@ import * as BIcons from "bootstrap-icons-vue"; import {mapActions, mapState} from "vuex"; import BaseLayout from "@/components/BaseLayout.vue"; +import {decodeHandleFromUrl, ownerOverviewRoute} from "@/router"; export default { name: "StorageLocationEdit", @@ -63,32 +64,56 @@ export default { ...BIcons }, props: { + handle: { + type: String, + required: true + }, id: { type: String, required: true } }, - computed: { - ...mapState(["storage_locations", "categories"]), - location() { - return { + data() { + return { + location: { name: "", description: "", category: null, parent: null, - ...this.storage_locations.find(loc => loc.id === parseInt(this.id)) - } + }, + // The location's own owner's full location list, for the parent dropdown - own + // personal locations, or the owning group's own (see loadLocation). + siblingLocations: [] + } + }, + computed: { + ...mapState(["categories"]), + decodedHandle() { + return decodeHandleFromUrl(this.handle) }, availableParents() { // Filter out self and children to prevent circular references - return this.storage_locations.filter(loc => + return this.siblingLocations.filter(loc => loc.id !== parseInt(this.id) && !this.isChildOf(loc, parseInt(this.id)) ) } }, methods: { - ...mapActions(["fetchStorageLocations", "updateStorageLocation", "fetchInfo"]), + ownerOverviewRoute, + ...mapActions(["fetchStorageLocationByHandle", "updateStorageLocation", "fetchInfo", + "fetchStorageLocations", "fetchGroupStorageLocations"]), + async loadLocation() { + const loaded = await this.fetchStorageLocationByHandle({handle: this.decodedHandle, id: this.id}) + if (loaded) { + this.location = {...this.location, ...loaded} + } + if (this.decodedHandle.startsWith('#')) { + this.siblingLocations = await this.fetchGroupStorageLocations({groupHandle: this.decodedHandle}) + } else { + this.siblingLocations = await this.fetchStorageLocations() + } + }, submitForm() { // Convert empty strings to null for ForeignKey fields const locationData = { @@ -96,21 +121,25 @@ export default { category: this.location.category === "" ? null : this.location.category, parent: this.location.parent === "" ? null : this.location.parent }; - this.updateStorageLocation(locationData); + this.updateStorageLocation(locationData).then(updated => this.$router.push(ownerOverviewRoute(updated))); }, isChildOf(location, parentId) { // Simple check to prevent circular references let current = location; while (current && current.parent) { if (current.parent === parentId) return true; - current = this.storage_locations.find(loc => loc.id === current.parent); + current = this.siblingLocations.find(loc => loc.id === current.parent); } return false; } }, + watch: { + handle: 'loadLocation', + id: 'loadLocation' + }, async mounted() { await this.fetchInfo(); - await this.fetchStorageLocations(); + await this.loadLocation(); } } diff --git a/frontend/src/views/StorageLocationNew.vue b/frontend/src/views/StorageLocationNew.vue index 0bbc130..074afdc 100644 --- a/frontend/src/views/StorageLocationNew.vue +++ b/frontend/src/views/StorageLocationNew.vue @@ -26,12 +26,21 @@ +
+ + +
@@ -53,6 +62,7 @@ import * as BIcons from "bootstrap-icons-vue"; import {mapActions, mapState} from "vuex"; import BaseLayout from "@/components/BaseLayout.vue"; +import {decodeHandleFromUrl, ownerOverviewRoute} from "@/router"; export default { name: "StorageLocationNew", @@ -66,12 +76,27 @@ export default { name: "", description: "", category: null, - parent: null - } + parent: null, + // The group's own "#name@domain" handle once picked, or null for a personal + // location - createStorageLocation resolves the target domain from it directly. + owner_group: null + }, + // The selected owner's own location list, for the parent dropdown - own personal + // locations, or the chosen group's own (see loadParentOptionsForOwner). + groupLocations: [] } }, methods: { - ...mapActions(['createStorageLocation', 'fetchInfo', 'fetchStorageLocations']), + ownerOverviewRoute, + ...mapActions(['createStorageLocation', 'fetchInfo', 'fetchStorageLocations', 'fetchGroups', + 'fetchGroupMemberships', 'fetchGroupStorageLocations']), + async loadParentOptionsForOwner() { + if (!this.location.owner_group) { + this.groupLocations = [] + return + } + this.groupLocations = await this.fetchGroupStorageLocations({groupHandle: this.location.owner_group}) + }, submitForm() { // Convert empty strings to null for ForeignKey fields const locationData = { @@ -79,15 +104,37 @@ export default { category: this.location.category === "" ? null : this.location.category, parent: this.location.parent === "" ? null : this.location.parent }; - this.createStorageLocation(locationData).then(() => this.$router.push('/storage-location')); + this.createStorageLocation(locationData).then(created => this.$router.push(ownerOverviewRoute(created))); } }, computed: { - ...mapState(["categories", "storage_locations"]), + ...mapState(["categories", "storage_locations", "groups", "groupMemberships"]), + // Groups hosted here plus groups only known via a GroupMembership pointer (see + // Groups.vue's allGroups for the same merge/dedupe). + ownerGroups() { + const hostedHandles = new Set(this.groups.map(group => group.handle)) + const foreign = this.groupMemberships.filter(m => !hostedHandles.has(m.handle)) + return [...this.groups, ...foreign].sort((a, b) => a.handle.localeCompare(b.handle)) + }, + availableParents() { + return this.location.owner_group ? this.groupLocations : this.storage_locations + } + }, + watch: { + 'location.owner_group'() { + // Previously-selected parent likely doesn't belong to the newly-chosen owner. + this.location.parent = null + this.loadParentOptionsForOwner() + } }, async mounted() { await this.fetchInfo(); await this.fetchStorageLocations(); + await this.fetchGroups(); + await this.fetchGroupMemberships(); + if (this.$route.query.group) { + this.location.owner_group = decodeHandleFromUrl(this.$route.query.group) + } } }