diff --git a/backend/toolshed/api/group.py b/backend/toolshed/api/group.py index e682934..74e8e47 100644 --- a/backend/toolshed/api/group.py +++ b/backend/toolshed/api/group.py @@ -40,11 +40,18 @@ class GroupDetail(APIView, ViewSetMixin): authentication_classes = [SignatureAuthentication] permission_classes = [IsAuthenticated] - 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'}) + 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/// 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) @@ -54,12 +61,8 @@ class GroupDetail(APIView, ViewSetMixin): @api_view(['DELETE']) @authentication_classes([SignatureAuthentication]) @permission_classes([IsAuthenticated]) -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) +def removeGroupMember(request, pk, identity_id, format=None): # /api/groups//members// + group = get_object_or_404(Group, pk=pk) 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) @@ -73,12 +76,8 @@ def removeGroupMember(request, handle, identity_id, format=None): # /api/groups @api_view(['POST']) @authentication_classes([SignatureAuthentication]) @permission_classes([IsAuthenticated]) -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) +def createGroupInvite(request, pk, format=None): # /api/groups//invites/ + group = get_object_or_404(Group, pk=pk) if not group.is_member(request.user): return Response(status=status.HTTP_404_NOT_FOUND) invitee = request.data.get('invitee') @@ -198,9 +197,10 @@ 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//members//', removeGroupMember, name='remove_group_member'), - path('groups//invites/', createGroupInvite, name='create_group_invite'), + 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('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 3f32258..833c00e 100644 --- a/backend/toolshed/api/inventory.py +++ b/backend/toolshed/api/inventory.py @@ -15,21 +15,6 @@ 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() @@ -69,10 +54,13 @@ class InventoryItemViewSet(viewsets.ModelViewSet): if identity.user.exists(): return InventoryItem.objects.filter(owner=identity.user.get()) | group_items return group_items - 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): + 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): return InventoryItem.objects.none() return InventoryItem.objects.filter(owner_group=group) if identity.user.exists(): @@ -80,11 +68,12 @@ class InventoryItemViewSet(viewsets.ModelViewSet): return InventoryItem.objects.none() def perform_create(self, serializer): - group_handle = self.request.data.get('owner_group') + group_id = self.request.data.get('owner_group') with transaction.atomic(): - if group_handle: - group = resolve_group_by_handle(group_handle) - if not group: + if group_id: + try: + group = Group.objects.get(pk=group_id) + except Group.DoesNotExist: raise NotFound('No such group') if not group.is_member(self.request.user): raise PermissionDenied('Not a member of this group') @@ -166,53 +155,21 @@ class StorageLocationViewSet(viewsets.ModelViewSet): lookup_url_kwarg = 'pk' def get_queryset(self): - # 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()) + if type(self.request.user) == KnownIdentity and self.request.user.user.exists(): + return StorageLocation.objects.filter(owner=self.request.user.user.get()) return StorageLocation.objects.none() def perform_create(self, serializer): - group_handle = self.request.data.get('owner_group') with transaction.atomic(): - 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) + serializer.save(owner=self.request.user.user.get()) def perform_update(self, serializer): with transaction.atomic(): - if self._is_authorized(self.request, serializer.instance): - serializer.save().clean() + if serializer.instance.owner == self.request.user.user.get(): + serializer.save() def perform_destroy(self, instance): - if self._is_authorized(self.request, instance): + if instance.owner == self.request.user.user.get(): 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 deleted file mode 100644 index 6ef08a2..0000000 --- a/backend/toolshed/migrations/0023_remove_storagelocation_storagelocation_unique_owner_scoped_id_and_more.py +++ /dev/null @@ -1,63 +0,0 @@ -# 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 9989cba..6136a86 100644 --- a/backend/toolshed/models.py +++ b/backend/toolshed/models.py @@ -170,25 +170,15 @@ class ItemTag(models.Model): class OwnerStorageLocationSequence(models.Model): - """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='+') + """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) 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=None, owner_group=None): + def allocate(cls, *, owner): with transaction.atomic(): - seq, _ = cls.objects.select_for_update().get_or_create(owner=owner, owner_group=owner_group) + seq, _ = cls.objects.select_for_update().get_or_create(owner=owner) seq.last_id += 1 seq.save(update_fields=['last_id']) return seq.last_id @@ -196,7 +186,7 @@ class OwnerStorageLocationSequence(models.Model): class StorageLocation(models.Model): internal_id = models.AutoField(primary_key=True) - # Externally visible id, sequential/gapless within the owner/owner_group's own locations (see + # Externally visible id, sequential/gapless within the owner's own locations (see # OwnerStorageLocationSequence), never internal_id; always allocate via create_for_owner, not # .objects.create(). id = models.PositiveIntegerField(editable=False) @@ -205,32 +195,24 @@ 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, 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') + owner = models.ForeignKey(ToolshedUser, on_delete=models.CASCADE, related_name='storage_locations') class Meta: constraints = [ - models.UniqueConstraint(fields=['owner', 'owner_group', 'id'], - name='storagelocation_unique_owner_scoped_id'), + models.UniqueConstraint(fields=['owner', '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=None, owner_group=None, **kwargs): + def create_for_owner(cls, *, owner, **kwargs): """The only supported way to create a StorageLocation: atomically allocates the next id - for this owner/owner_group's scope.""" + for this owner's scope.""" with transaction.atomic(): - next_id = OwnerStorageLocationSequence.allocate(owner=owner, owner_group=owner_group) - return cls.objects.create(owner=owner, owner_group=owner_group, id=next_id, **kwargs) + next_id = OwnerStorageLocationSequence.allocate(owner=owner) + return cls.objects.create(owner=owner, id=next_id, **kwargs) class WorkflowInstance(models.Model): diff --git a/backend/toolshed/serializers.py b/backend/toolshed/serializers.py index 846117b..0bfe6b0 100644 --- a/backend/toolshed/serializers.py +++ b/backend/toolshed/serializers.py @@ -1,5 +1,4 @@ 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 @@ -178,13 +177,9 @@ class OwnerScopedPrimaryKeyRelatedField(serializers.PrimaryKeyRelatedField): def get_queryset(self): queryset = super().get_queryset() request = self.context.get('request') - 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) + 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() def to_internal_value(self, data): queryset = self.get_queryset() @@ -203,7 +198,6 @@ 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) @@ -211,7 +205,7 @@ class StorageLocationSerializer(serializers.ModelSerializer): class Meta: model = StorageLocation - fields = ['id', 'name', 'description', 'path', 'category', 'owner', 'owner_group', 'parent'] + fields = ['id', 'name', 'description', 'path', 'category', 'owner', 'parent'] read_only_fields = ['id', 'path'] @staticmethod diff --git a/backend/toolshed/tests/test_group.py b/backend/toolshed/tests/test_group.py index f3f1405..bf9ac7a 100644 --- a/backend/toolshed/tests/test_group.py +++ b/backend/toolshed/tests/test_group.py @@ -53,47 +53,56 @@ class GroupApiTestCase(UserTestMixin, GroupTestMixin, ToolshedTestCase): def test_group_detail_member(self): self.prepare_groups() - group = self.f['group1'] - reply = client.get('/api/groups/{}@{}/'.format(group.name, group.domain), self.f['local_user1']) + reply = client.get('/api/groups/{}/'.format(self.f['group1'].id), self.f['local_user1']) self.assertEqual(reply.status_code, 200) - self.assertEqual(reply.json()['handle'], str(group)) + self.assertEqual(reply.json()['handle'], str(self.f['group1'])) self.assertEqual(len(reply.json()['members']), 1) def test_group_detail_non_member(self): self.prepare_groups() - group = self.f['group1'] - reply = client.get('/api/groups/{}@{}/'.format(group.name, group.domain), self.f['local_user2']) + reply = client.get('/api/groups/{}/'.format(self.f['group1'].id), self.f['local_user2']) self.assertEqual(reply.status_code, 404) - def test_group_detail_no_such_group(self): - reply = client.get('/api/groups/nonexistent@example.com/', self.f['local_user1']) + 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']) self.assertEqual(reply.status_code, 404) def test_remove_member(self): self.prepare_groups() - 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.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']) self.assertEqual(reply.status_code, 204) - self.assertEqual(group.members.count(), 1) + self.assertEqual(self.f['group1'].members.count(), 1) def test_remove_last_member_blocked(self): self.prepare_groups() - 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']) + reply = client.delete('/api/groups/{}/members/{}/'.format( + self.f['group1'].id, self.f['local_user1'].public_identity.id), self.f['local_user1']) self.assertEqual(reply.status_code, 400) - self.assertEqual(group.members.count(), 1) + self.assertEqual(self.f['group1'].members.count(), 1) def test_remove_member_non_member_denied(self): self.prepare_groups() - 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.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']) self.assertEqual(reply.status_code, 404) - self.assertEqual(group.members.count(), 2) + self.assertEqual(self.f['group1'].members.count(), 2) class GroupInviteApiTestCase(UserTestMixin, GroupTestMixin, ToolshedTestCase): @@ -107,8 +116,7 @@ class GroupInviteApiTestCase(UserTestMixin, GroupTestMixin, ToolshedTestCase): inviter = self.f['local_user1'] invitee = self.f['local_user2'] - reply1 = client.post('/api/groups/{}@{}/invites/'.format(group.name, group.domain), inviter, - {'invitee': str(invitee)}) + reply1 = client.post('/api/groups/{}/invites/'.format(group.id), inviter, {'invitee': str(invitee)}) self.assertEqual(reply1.status_code, 201) secret = reply1.json()['secret'] self.assertEqual(GroupInvite.objects.count(), 1) @@ -144,7 +152,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.name, group.domain), self.f['local_user2'], + reply = client.post('/api/groups/{}/invites/'.format(group.id), self.f['local_user2'], {'invitee': str(self.f['ext_user1'])}) self.assertEqual(reply.status_code, 404) self.assertEqual(GroupInvite.objects.count(), 0) @@ -152,7 +160,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.name, group.domain), self.f['local_user1'], + reply = client.post('/api/groups/{}/invites/'.format(group.id), self.f['local_user1'], {'invitee': str(self.f['local_user2'])}) self.assertEqual(reply.status_code, 208) @@ -169,7 +177,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.name, group.domain), inviter, {'invitee': str(invitee)}) + client.post('/api/groups/{}/invites/'.format(group.id), 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' @@ -181,8 +189,7 @@ 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.name, group.domain), inviter, - {'invitee': str(invitee)}) + reply1 = client.post('/api/groups/{}/invites/'.format(group.id), inviter, {'invitee': str(invitee)}) secret = reply1.json()['secret'] reply = client.post('/api/group_invites/accept/', invitee, { @@ -199,8 +206,7 @@ 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.name, group.domain), inviter, - {'invitee': str(invitee)}) + reply1 = client.post('/api/groups/{}/invites/'.format(group.id), 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 8a1b1a5..eba0c12 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': str(self.f['group1'])[1:], + 'owner_group': self.f['group1'].id, }) 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': str(self.f['group1'])[1:], + 'owner_group': self.f['group1'].id, }) 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(str(self.f['group1'])[1:]), self.f['local_user2']) + reply = client.get('/api/inventory_items/?group={}'.format(self.f['group1'].id), 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(str(self.f['group1'])[1:]), self.f['ext_user1']) + reply = client.get('/api/inventory_items/?group={}'.format(self.f['group1'].id), 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': str(self.f['group1'])[1:], + 'owner_group': self.f['group1'].id, }) self.assertEqual(reply.status_code, 201) item = InventoryItem.objects.get(name='drill') @@ -451,21 +451,20 @@ 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': str(self.f['group1'])[1:], + 'availability_policy': 'private', 'owned_quantity': 1, 'owner_group': self.f['group1'].id, }) 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': 'nonexistent@example.com', + 'name': 'drill', 'owned_quantity': 1, 'availability_policy': 'private', 'owner_group': 999999, }) 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=nonexistent@example.com', self.f['local_user1']) + reply = client.get('/api/inventory_items/?group=999999', self.f['local_user1']) self.assertEqual(reply.status_code, 200) self.assertEqual(len(reply.json()), 0) @@ -496,7 +495,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': str(self.f['group1'])[1:], + 'owner_group': self.f['group1'].id, }) item_id = reply.json()['id'] reply = client.patch('/api/inventory_items/{}/'.format(item_id), self.f['local_user2'], { @@ -510,7 +509,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': str(self.f['group1'])[1:], + 'storage_location': self.f['loc1'].id, 'owner_group': self.f['group1'].id, }) self.assertEqual(reply.status_code, 201) item = InventoryItem.objects.get(name='drill') @@ -520,7 +519,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': str(self.f['group1'])[1:], + 'files': [self.f['test_file1'].id], 'owner_group': self.f['group1'].id, }) self.assertEqual(reply.status_code, 201) item = InventoryItem.objects.get(name='drill') @@ -530,7 +529,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': str(self.f['group1'])[1:], + 'owner_group': self.f['group1'].id, }) 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 6850db4..438775d 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, GroupTestMixin, ToolshedTestCase +from authentication.tests import SignatureAuthClient, UserTestMixin, ToolshedTestCase from files.tests import FilesTestMixin from toolshed.models import InventoryItem, Category, StorageLocation from toolshed.tests import InventoryTestMixin, LocationTestMixin @@ -129,110 +129,3 @@ 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 fd8d9d1..b37ece4 100644 --- a/frontend/src/router.js +++ b/frontend/src/router.js @@ -51,11 +51,6 @@ 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'; } @@ -69,15 +64,12 @@ const EXPANDED_ROUTE_BUILDERS = { const handle = store.getters.groupHandleById[group_id]; return handle ? `/groups/${encodeHandleForUrl(handle)}` : null; }, - 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), + storage_location: ({storage_location_id}) => `/storage-locations/${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', 'storage_location', 'group_storage_location']); +export const NEEDS_IDMAP = new Set(['item', 'group_item', 'group']); export function expandedRoute({kind, ...fields}) { const buildRoute = EXPANDED_ROUTE_BUILDERS[kind]; @@ -196,12 +188,12 @@ const routes = [{path: '/', component: Dashboard, meta: {requiresAuth: true}}, { component: StorageLocation, meta: {requiresAuth: true} }, { - path: '/storage-locations/:handle/:id', + path: '/storage-locations/:id', component: StorageLocationDetail, meta: {requiresAuth: true}, props: true }, { - path: '/storage-locations/:handle/:id/edit', + path: '/storage-locations/:id/edit', component: StorageLocationEdit, meta: {requiresAuth: true}, props: true diff --git a/frontend/src/short-id.js b/frontend/src/short-id.js index 1f79672..f26e4f7 100644 --- a/frontend/src/short-id.js +++ b/frontend/src/short-id.js @@ -40,12 +40,6 @@ 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 b89377d..162b980 100644 --- a/frontend/src/store.js +++ b/frontend/src/store.js @@ -83,7 +83,6 @@ export default createStore({ idmap: {identities: [], groups: []}, idmapLoaded: false, item_map: {}, - location_map: {}, home_servers: null, all_friends_servers: null, messages: [], @@ -106,9 +105,6 @@ 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; }, @@ -401,17 +397,16 @@ export default createStore({ return items }, async createInventoryItem({state, dispatch, getters}, 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 servers = await dispatch('getHomeServers') + const data = {availability_policy: 'private', ...item} 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}) @@ -470,10 +465,16 @@ 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 items = await dispatch('fetchGroupInventoryItems', {groupHandle: handle}) + const group = await dispatch('fetchGroup', {handle}) + const items = await dispatch('fetchGroupInventoryItems', {groupId: group.id, domain: group.domain}) return items.find(item => item.id === parseInt(id)) || null } catch (error) { console.error(`Failed to fetch group item ${id} for ${handle}:`, error); @@ -529,6 +530,14 @@ 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/') @@ -543,19 +552,17 @@ export default createStore({ }, // handle is "#name@domain". async fetchGroup({dispatch, getters}, {handle}) { - const {domain} = splitGroupHandle(handle) + const {name, domain} = splitGroupHandle(handle) const servers = await dispatch('getFriendServers', {username: 'x@' + domain}) - return await servers.get(getters.signAuth, `/api/groups/${handle.slice(1)}/`) + return await servers.get(getters.signAuth, `/api/groups/handle/${name}/${domain}/`) }, async createGroup({dispatch, getters}, {name}) { const servers = await dispatch('getHomeServers') return await servers.post(getters.signAuth, '/api/groups/', {name}) }, - async removeGroupMember({dispatch, getters}, {groupHandle, identityId}) { - const {domain} = splitGroupHandle(groupHandle) + async removeGroupMember({dispatch, getters}, {groupId, domain, identityId}) { const servers = await dispatch('getFriendServers', {username: 'x@' + domain}) - return await servers.delete( - getters.signAuth, '/api/groups/' + groupHandle.slice(1) + '/members/' + identityId + '/') + return await servers.delete(getters.signAuth, '/api/groups/' + groupId + '/members/' + identityId + '/') }, async fetchGroupInvites({commit, dispatch, getters}) { const servers = await dispatch('getHomeServers') @@ -569,11 +576,10 @@ export default createStore({ commit('setGroupMemberships', data) return data }, - async inviteToGroup({state, dispatch, getters}, {groupHandle, invitee}) { - const {domain} = splitGroupHandle(groupHandle) + async inviteToGroup({state, dispatch, getters}, {groupId, domain, groupHandle, invitee}) { const group_servers = await dispatch('getFriendServers', {username: 'x@' + domain}) const group_reply = await group_servers.post( - getters.signAuth, '/api/groups/' + groupHandle.slice(1) + '/invites/', {invitee}) + getters.signAuth, '/api/groups/' + groupId + '/invites/', {invitee}) if (!group_reply.secret) { return false } @@ -604,14 +610,13 @@ export default createStore({ const servers = await dispatch('getHomeServers') return await servers.delete(getters.signAuth, '/api/groupinvites/' + invite.id + '/') }, - async fetchGroupInventoryItems({commit, dispatch, getters}, {groupHandle}) { - const {domain} = splitGroupHandle(groupHandle) + async fetchGroupInventoryItems({commit, dispatch, getters}, {groupId, domain}) { const servers = await dispatch('getFriendServers', {username: 'x@' + domain}) - 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 + 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 // backend's database, so two different domains could otherwise collide on the same - // item_map key - the handle already carries the domain, so no separate namespacing is needed. - commit('setInventoryItems', {url: '/group/' + groupHandle, items}) + // item_map key. + commit('setInventoryItems', {url: '/group/' + domain + '/' + groupId, items}) return items }, async fetchFiles({state, commit, dispatch, getters}) { @@ -717,50 +722,23 @@ 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 = location.owner_group - ? await dispatch('getFriendServers', {username: 'x@' + splitGroupHandle(location.owner_group).domain}) - : await dispatch('getHomeServers') + const servers = 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 = 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} + const servers = await dispatch('getHomeServers') + const data = {...location} 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 = location.owner_group - ? await dispatch('getFriendServers', {username: 'x@' + splitGroupHandle(location.owner_group).domain}) - : await dispatch('getHomeServers') + const servers = 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) @@ -870,14 +848,15 @@ export default createStore({ inventory_items(state) { return state.item_map['/'] || [] }, - groupInventoryItems: (state) => (groupHandle) => state.item_map['/group/' + groupHandle] || [], - groupStorageLocations: (state) => (groupHandle) => state.location_map['/group/' + groupHandle] || [], + groupInventoryItems: (state) => (groupId, domain) => state.item_map['/group/' + domain + '/' + groupId] || [], 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 8e92fb5..df53525 100644 --- a/frontend/src/tests/short-id.js +++ b/frontend/src/tests/short-id.js @@ -21,7 +21,6 @@ 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 b079b59..21eaaa9 100644 --- a/frontend/src/views/GroupDetail.vue +++ b/frontend/src/views/GroupDetail.vue @@ -127,55 +127,7 @@
- Add - -
- -
-
-
-
Group storage locations
-
- - - - - - - - - - - - - - - - - -
NamePathCategoryActions
- {{ location.name }} - - {{ location.path }} - - {{ location.category }} - - - - - - - - - - - - -
-
-
- - Add
@@ -187,7 +139,7 @@ diff --git a/frontend/src/views/InventoryNew.vue b/frontend/src/views/InventoryNew.vue index 7042431..3c1065e 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 {decodeHandleFromUrl, ownerOverviewRoute} from "@/router"; +import {ownerOverviewRoute} from "@/router"; export default { name: "InventoryNew", @@ -107,34 +107,23 @@ 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', - 'fetchGroupMemberships']) + ...mapActions(['createInventoryItem', 'fetchInfo', 'fetchStorageLocations', 'fetchGroups']) }, computed: { - ...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)) - } + ...mapState(["availability_policies", "storage_locations", "groups"]), }, async mounted() { await this.fetchInfo(); await this.fetchStorageLocations(); await this.fetchGroups(); - await this.fetchGroupMemberships(); if (this.$route.query.group) { - this.item.owner_group = decodeHandleFromUrl(this.$route.query.group) + this.item.owner_group = parseInt(this.$route.query.group, 10) } } } diff --git a/frontend/src/views/Print.vue b/frontend/src/views/Print.vue index 4b1ffab..bca75a7 100644 --- a/frontend/src/views/Print.vue +++ b/frontend/src/views/Print.vue @@ -464,18 +464,10 @@ export default { } if (f.userHandle.startsWith("#")) { const owner_group_id = this.groupIdByHandle[f.userHandle]; - if (owner_group_id === undefined) { + if (owner_group_id === undefined || !f.itemId) { return null; } - 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; + return shortenedRoute({kind: "group_item", owner_group_id, item_local_id: f.itemId}).slice(1); } 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 719f058..918bf92 100644 --- a/frontend/src/views/Scan.vue +++ b/frontend/src/views/Scan.vue @@ -154,7 +154,7 @@ export default { }; }, methods: { - ...mapActions(["fetchItemByHandle", "fetchStorageLocationByHandle", "fetchIdMap"]), + ...mapActions(["fetchItemByHandle", "fetchStorageLocations", "fetchIdMap"]), resolveDescription(entry) { const {link, text} = entry; @@ -219,19 +219,12 @@ export default { } return handle === undefined ? null : handle; } - 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]; + if (decoded.kind === "storage_location") { + if (!this.$store.state.storage_locations.length) { + await this.fetchStorageLocations(); } - return handle === undefined ? null : this.describeLocation(handle, decoded.storage_location_id); + const location = this.$store.state.storage_locations.find(l => l.id === decoded.storage_location_id); + return location ? `[#${location.id}] ${location.name}` : null; } return null; // workflow, category, file: no per-item title lookup wired up yet }, @@ -241,11 +234,6 @@ 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 d71ba81..03fff50 100644 --- a/frontend/src/views/ShortId.vue +++ b/frontend/src/views/ShortId.vue @@ -73,7 +73,6 @@ 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 579788d..3d23bb7 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 || location.owner_group || '-' }}
+
{{ location.owner?.username || '-' }}
-
- -
@@ -52,8 +47,7 @@ diff --git a/frontend/src/views/StorageLocationEdit.vue b/frontend/src/views/StorageLocationEdit.vue index 6d330c3..b792950 100644 --- a/frontend/src/views/StorageLocationEdit.vue +++ b/frontend/src/views/StorageLocationEdit.vue @@ -55,7 +55,6 @@ 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", @@ -64,56 +63,32 @@ export default { ...BIcons }, props: { - handle: { - type: String, - required: true - }, id: { type: String, required: true } }, - data() { - return { - location: { + computed: { + ...mapState(["storage_locations", "categories"]), + location() { + return { name: "", description: "", category: null, parent: null, - }, - // 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) + ...this.storage_locations.find(loc => loc.id === parseInt(this.id)) + } }, availableParents() { // Filter out self and children to prevent circular references - return this.siblingLocations.filter(loc => + return this.storage_locations.filter(loc => loc.id !== parseInt(this.id) && !this.isChildOf(loc, parseInt(this.id)) ) } }, methods: { - 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() - } - }, + ...mapActions(["fetchStorageLocations", "updateStorageLocation", "fetchInfo"]), submitForm() { // Convert empty strings to null for ForeignKey fields const locationData = { @@ -121,25 +96,21 @@ export default { category: this.location.category === "" ? null : this.location.category, parent: this.location.parent === "" ? null : this.location.parent }; - this.updateStorageLocation(locationData).then(updated => this.$router.push(ownerOverviewRoute(updated))); + this.updateStorageLocation(locationData); }, isChildOf(location, parentId) { // Simple check to prevent circular references let current = location; while (current && current.parent) { if (current.parent === parentId) return true; - current = this.siblingLocations.find(loc => loc.id === current.parent); + current = this.storage_locations.find(loc => loc.id === current.parent); } return false; } }, - watch: { - handle: 'loadLocation', - id: 'loadLocation' - }, async mounted() { await this.fetchInfo(); - await this.loadLocation(); + await this.fetchStorageLocations(); } } diff --git a/frontend/src/views/StorageLocationNew.vue b/frontend/src/views/StorageLocationNew.vue index 074afdc..0bbc130 100644 --- a/frontend/src/views/StorageLocationNew.vue +++ b/frontend/src/views/StorageLocationNew.vue @@ -26,21 +26,12 @@ -
- - -
@@ -62,7 +53,6 @@ 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", @@ -76,27 +66,12 @@ export default { name: "", description: "", category: 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: [] + parent: null + } } }, methods: { - 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}) - }, + ...mapActions(['createStorageLocation', 'fetchInfo', 'fetchStorageLocations']), submitForm() { // Convert empty strings to null for ForeignKey fields const locationData = { @@ -104,37 +79,15 @@ export default { category: this.location.category === "" ? null : this.location.category, parent: this.location.parent === "" ? null : this.location.parent }; - this.createStorageLocation(locationData).then(created => this.$router.push(ownerOverviewRoute(created))); + this.createStorageLocation(locationData).then(() => this.$router.push('/storage-location')); } }, computed: { - ...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() - } + ...mapState(["categories", "storage_locations"]), }, 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) - } } }