Show group details to all logged-in users (#1344)

* show group details to all logged-in users * fix possible null access and return 404 when group does not exist * linter fixes * fix type checker error * review changes * show message instead of redirect
loculus-project · Mar 15, 2024 · baf93ed · baf93ed
1 parent 3884110
commit baf93ed
Show file tree

Hide file tree

Showing 5 changed files with 82 additions and 78 deletions.
diff --git a/backend/src/main/kotlin/org/loculus/backend/controller/GroupManagementController.kt b/backend/src/main/kotlin/org/loculus/backend/controller/GroupManagementController.kt
@@ -33,16 +33,15 @@ class GroupManagementController(
         ) @RequestBody group: Group,
     ) = groupManagementDatabaseService.createNewGroup(group, username)
 
-    @Operation(description = "Get details of a group that the user is a member of.")
+    @Operation(description = "Get details of a group.")
     @ResponseStatus(HttpStatus.OK)
     @GetMapping("/groups/{groupName}", produces = [MediaType.APPLICATION_JSON_VALUE])
     fun getUsersOfGroup(
-        @UsernameFromJwt username: String,
         @Parameter(
             description = "The name of the group to get details of.",
         ) @PathVariable groupName: String,
     ): GroupDetails {
-        return groupManagementDatabaseService.getDetailsOfGroup(groupName, username)
+        return groupManagementDatabaseService.getDetailsOfGroup(groupName)
     }
 
     @Operation(description = "Get all groups the user is a member of.")

diff --git a/...main/kotlin/org/loculus/backend/service/groupmanagement/GroupManagementDatabaseService.kt b/...main/kotlin/org/loculus/backend/service/groupmanagement/GroupManagementDatabaseService.kt
@@ -11,7 +11,9 @@ import org.jetbrains.exposed.sql.selectAll
 import org.loculus.backend.api.Address
 import org.loculus.backend.api.Group
 import org.loculus.backend.api.GroupDetails
+import org.loculus.backend.api.User
 import org.loculus.backend.controller.ConflictException
+import org.loculus.backend.controller.NotFoundException
 import org.loculus.backend.model.UNIQUE_CONSTRAINT_VIOLATION_SQL_STATE
 import org.springframework.stereotype.Service
 import org.springframework.transaction.annotation.Transactional
@@ -22,35 +24,31 @@ class GroupManagementDatabaseService(
     private val groupManagementPreconditionValidator: GroupManagementPreconditionValidator,
 ) {
 
-    fun getDetailsOfGroup(groupName: String, username: String): GroupDetails {
-        val users = groupManagementPreconditionValidator.validateUserInExistingGroupAndReturnUserList(
-            groupName,
-            username,
+    fun getDetailsOfGroup(groupName: String): GroupDetails {
+        return GroupDetails(
+            group = GroupsTable
+                .select { GroupsTable.groupNameColumn eq groupName }
+                .map {
+                    Group(
+                        groupName = it[GroupsTable.groupNameColumn],
+                        institution = it[GroupsTable.institutionColumn],
+                        address = Address(
+                            line1 = it[GroupsTable.addressLine1],
+                            line2 = it[GroupsTable.addressLine2],
+                            postalCode = it[GroupsTable.addressPostalCode],
+                            city = it[GroupsTable.addressCity],
+                            state = it[GroupsTable.addressState],
+                            country = it[GroupsTable.addressCountry],
+                        ),
+                        contactEmail = it[GroupsTable.contactEmailColumn],
+                    )
+                }
+                .firstOrNull()
+                ?: throw NotFoundException("Group $groupName does not exist."),
+            users = UserGroupsTable
+                .select { UserGroupsTable.groupNameColumn eq groupName }
+                .map { User(it[UserGroupsTable.userNameColumn]) },
         )
-
-        return GroupDetails(getDetailsOfGroup(groupName), users)
-    }
-
-    fun getDetailsOfGroup(groupName: String): Group {
-        return GroupsTable
-            .select { GroupsTable.groupNameColumn eq groupName }
-            .map {
-                Group(
-                    groupName = it[GroupsTable.groupNameColumn],
-                    institution = it[GroupsTable.institutionColumn],
-                    address = Address(
-                        line1 = it[GroupsTable.addressLine1],
-                        line2 = it[GroupsTable.addressLine2],
-                        postalCode = it[GroupsTable.addressPostalCode],
-                        city = it[GroupsTable.addressCity],
-                        state = it[GroupsTable.addressState],
-                        country = it[GroupsTable.addressCountry],
-                    ),
-                    contactEmail = it[GroupsTable.contactEmailColumn],
-                )
-            }
-            .firstOrNull()
-            ?: throw IllegalArgumentException("Group $groupName does not exist.")
     }
 
     fun createNewGroup(group: Group, username: String) {

diff --git a/...st/kotlin/org/loculus/backend/controller/groupmanagement/GroupManagementControllerTest.kt b/...st/kotlin/org/loculus/backend/controller/groupmanagement/GroupManagementControllerTest.kt
@@ -109,7 +109,7 @@ class GroupManagementControllerTest(
     }
 
     @Test
-    fun `WHEN I query details of a non-existing group THEN to find no group`() {
+    fun `WHEN I query details of a non-existing group THEN expect error that group does not exist`() {
         client.getDetailsOfGroup()
             .andExpect(status().isNotFound)
             .andExpect(content().contentType(MediaType.APPLICATION_JSON_VALUE))

diff --git a/website/src/components/User/GroupPage.tsx b/website/src/components/User/GroupPage.tsx
@@ -93,41 +93,43 @@ const InnerGroupPage: FC<GroupPageProps> = ({ prefetchedGroupDetails, clientConf
                 </table>
             </div>
 
-            <h2 className='text-lg font-bold py-4'> Users </h2>
-            <form onSubmit={handleAddUser}>
-                <div className='flex mb-4'>
-                    <input
-                        type='text'
-                        value={newUserName}
-                        onChange={(e) => setNewUserName(e.target.value.trim())}
-                        placeholder='Enter new user name'
-                        className='p-2 border border-gray-300 rounded mr-2'
-                        required
-                    />
-                    <button type='submit' className='px-4 py-2 loculusColor text-white rounded'>
-                        Add User
-                    </button>
+            {(groupDetails.data?.users.some((user) => user.name === username) ?? false) && (
+                <div>
+                    <h2 className='text-lg font-bold py-4'> Users </h2>
+                    <form onSubmit={handleAddUser}>
+                        <div className='flex mb-4'>
+                            <input
+                                type='text'
+                                value={newUserName}
+                                onChange={(e) => setNewUserName(e.target.value.trim())}
+                                placeholder='Enter new user name'
+                                className='p-2 border border-gray-300 rounded mr-2'
+                                required
+                            />
+                            <button type='submit' className='px-4 py-2 loculusColor text-white rounded'>
+                                Add User
+                            </button>
+                        </div>
+                    </form>
+                    <div className='flex-1 overflow-y-auto'>
+                        <ul>
+                            {groupDetails.data?.users.map((user) => (
+                                <li key={user.name} className='flex items-center gap-6 bg-gray-100 p-2 mb-2 rounded'>
+                                    <span className='text-lg'>{user.name}</span>
+                                    <button
+                                        onClick={() => handleOpenConfirmationDialog(user)}
+                                        className='px-2 py-1 bg-red-500 text-white rounded'
+                                        title='Remove user from group'
+                                        aria-label={`Remove User ${user.name}`}
+                                    >
+                                        <DeleteIcon className='w-4 h-4' />
+                                    </button>
+                                </li>
+                            ))}
+                        </ul>
+                    </div>
                 </div>
-            </form>
-
-            <div className='flex-1 overflow-y-auto'>
-                <ul>
-                    {groupDetails.data &&
-                        groupDetails.data.users.map((user) => (
-                            <li key={user.name} className='flex items-center gap-6 bg-gray-100 p-2 mb-2 rounded'>
-                                <span className='text-lg'>{user.name}</span>
-                                <button
-                                    onClick={() => handleOpenConfirmationDialog(user)}
-                                    className='px-2 py-1 bg-red-500 text-white rounded'
-                                    title='Remove user from group'
-                                    aria-label={`Remove User ${user.name}`}
-                                >
-                                    <DeleteIcon className='w-4 h-4' />
-                                </button>
-                            </li>
-                        ))}
-                </ul>
-            </div>
+            )}
         </div>
     );
 };

diff --git a/website/src/pages/group/[groupName]/index.astro b/website/src/pages/group/[groupName]/index.astro
@@ -1,14 +1,15 @@
 ---
 import { GroupPage } from '../../../components/User/GroupPage';
 import ErrorBox from '../../../components/common/ErrorBox.astro';
+import NeedToLogin from '../../../components/common/NeedToLogin.astro';
 import { getRuntimeConfig } from '../../../config';
 import BaseLayout from '../../../layouts/BaseLayout.astro';
 import { GroupManagementClient } from '../../../services/groupManagementClient';
 import { getAccessToken } from '../../../utils/getAccessToken';
 
 const session = Astro.locals.session!;
 const accessToken = getAccessToken(session)!;
-const username = session.user!.username!;
+const username = session.user?.username ?? '';
 const groupName = Astro.params.groupName!;
 const clientConfig = getRuntimeConfig().public;
 
@@ -20,17 +21,21 @@ const groupDetailsResult = await GroupManagementClient.create().getGroupDetails(
         <h1 class='title'>Group: {groupName}</h1>
     </div>
     {
-        groupDetailsResult.match(
-            (groupDetails) => (
-                <GroupPage
-                    prefetchedGroupDetails={groupDetails}
-                    accessToken={accessToken}
-                    clientConfig={clientConfig}
-                    username={username}
-                    client:load
-                />
-            ),
-            () => <ErrorBox message='Failed to fetch group details, sorry for the inconvenience!' />,
+        !accessToken ? (
+            <NeedToLogin message='You need to be logged in to view group information.' />
+        ) : (
+            groupDetailsResult.match(
+                (groupDetails) => (
+                    <GroupPage
+                        prefetchedGroupDetails={groupDetails}
+                        accessToken={accessToken}
+                        clientConfig={clientConfig}
+                        username={username}
+                        client:load
+                    />
+                ),
+                () => <ErrorBox message='Failed to fetch group details, sorry for the inconvenience!' />,
+            )
         )
     }
 </BaseLayout>