From d208a8b1986df3c2c395b4072b3ce1f2510bbd35 Mon Sep 17 00:00:00 2001 From: Mark Bouslog Date: Fri, 30 Aug 2024 08:46:37 -0500 Subject: [PATCH] lib-user: Add loading, empty, and error states to UserStats, MyGroups, Certificate, GroupStats, and Contributors (#6228) * Refactor MyGroups for loading and empty states * Refactor TopProjects with loading state * Add loading state to TopContributors * Add loading state to Certificate * Refactor Contributors for loading and empty states * Refactor Certificate for empty states * Refactor hooks to throw error * Refactor UserStats for error state * Refactor GroupStats for error states * Refactor Certificate with error state * Refactor MyGroups with error states * Refactor Contributors with error states * Refactor getUserGroupStatus l oading test * fix missing prop-type in Certificate * Refactored AuthenticatedUsersPageContainer with styled "please login" and "not authorized" --------- Co-authored-by: Delilah <23665803+goplayoutside3@users.noreply.github.com> --- .../AuthenticatedUsersPageContainer.js | 20 ++++- .../src/components/Certificate/Certificate.js | 40 +++++++-- .../Certificate/CertificateContainer.js | 6 +- .../components/Contributors/Contributors.js | 43 ++++++++-- .../src/components/GroupStats/GroupStats.js | 9 +++ .../TopContributors/TopContributors.js | 81 +++++++++++++------ .../src/components/MyGroups/MyGroups.js | 72 ++++++++++++++--- .../components/MyGroups/MyGroups.stories.js | 22 ++--- .../components/MyGroups/MyGroupsContainer.js | 9 ++- .../src/components/UserStats/UserStats.js | 3 + .../UserStats/UserStatsContainer.js | 2 + .../ContentBox/components/ContentLink.js | 1 + .../shared/GroupContainer/GroupContainer.js | 18 ++++- .../helpers/getUserGroupStatus.js | 3 +- .../helpers/getUserGroupStatus.spec.js | 8 +- .../shared/MainContent/MainContent.js | 15 ++++ .../shared/TopProjects/TopProjects.js | 45 +++++++---- .../shared/TopProjects/TopProjects.stories.js | 8 +- .../src/hooks/usePanoptesMemberships.js | 4 +- .../lib-user/src/hooks/usePanoptesProjects.js | 48 ++++++----- .../lib-user/src/hooks/usePanoptesUser.js | 2 +- .../src/hooks/usePanoptesUserGroup.js | 4 +- packages/lib-user/src/hooks/useStats.js | 4 +- .../lib-user/src/utils/fetchPanoptesUsers.js | 24 +++--- 24 files changed, 357 insertions(+), 134 deletions(-) diff --git a/packages/app-root/src/components/AuthenticatedUsersPageContainer.js b/packages/app-root/src/components/AuthenticatedUsersPageContainer.js index b364239bda..d10c281cf0 100644 --- a/packages/app-root/src/components/AuthenticatedUsersPageContainer.js +++ b/packages/app-root/src/components/AuthenticatedUsersPageContainer.js @@ -1,6 +1,6 @@ 'use client' -import { Loader } from '@zooniverse/react-components' +import { Loader, SpacedText } from '@zooniverse/react-components' import { Box } from 'grommet' function AuthenticatedUsersPageContainer({ @@ -18,12 +18,24 @@ function AuthenticatedUsersPageContainer({ ) } - if (!user) { - return

Please log in.

+ if ((!user || Object.keys(user).length === 0)) { + return ( + + + Please log in. + + + ) } if (user && login !== user?.login && !adminMode) { - return

Not authorized.

+ return ( + + + Not authorized. + + + ) } return <>{children} diff --git a/packages/lib-user/src/components/Certificate/Certificate.js b/packages/lib-user/src/components/Certificate/Certificate.js index 0666e83928..4e994756a8 100644 --- a/packages/lib-user/src/components/Certificate/Certificate.js +++ b/packages/lib-user/src/components/Certificate/Certificate.js @@ -1,6 +1,6 @@ -import { SpacedText, ZooniverseLogo } from '@zooniverse/react-components' +import { Loader, SpacedText, ZooniverseLogo } from '@zooniverse/react-components' import { Box } from 'grommet' -import { number, shape, string } from 'prop-types' +import { bool, number, shape, string } from 'prop-types' import styled from 'styled-components' import { @@ -67,7 +67,9 @@ function handleClickPrint() { } function Certificate({ + error = undefined, hours = 0, + loading = false, login = '', name = '', paramsValidationMessage = '', @@ -104,7 +106,32 @@ function Certificate({ justify='center' pad='medium' > - {paramsValidationMessage} + + {paramsValidationMessage} + + + ) : loading ? ( + + + + ) : error ? ( + + + There was an error. + + + {error?.message} + ) : ( - {projectsCount ? ( - `${projectsCount} projects` - ) : ( + {projectDisplayName ? ( projectDisplayName + ) : ( + `${projectsCount} projects` )} @@ -286,6 +313,7 @@ function Certificate({ Certificate.propTypes = { hours: number, + loading: bool, login: string, name: string, paramsValidationMessage: string, diff --git a/packages/lib-user/src/components/Certificate/CertificateContainer.js b/packages/lib-user/src/components/Certificate/CertificateContainer.js index f18230d800..215a7b0ec3 100644 --- a/packages/lib-user/src/components/Certificate/CertificateContainer.js +++ b/packages/lib-user/src/components/Certificate/CertificateContainer.js @@ -72,14 +72,18 @@ function CertificateContainer({ id: selectedProject }) - const hours = convertStatsSecondsToHours(stats?.time_spent) + const error = userError || statsError || projectsError + const hours = convertStatsSecondsToHours(stats?.time_spent) || 0 + const loading = userLoading || statsLoading || projectsLoading const name = user?.credited_name || user?.display_name || login const projectsCount = stats?.project_contributions?.length || 0 const projectDisplayName = projects?.[0]?.display_name return ( Not authorized) + const error = statsError || usersError || projectsError + const loading = statsLoading || usersLoading || projectsLoading + const disableStatsExport = !showContributors || !!error || loading || !contributors?.length return ( <> @@ -166,17 +168,42 @@ function Contributors({ linkLabel='Export all stats' linkProps={{ as: 'button', + disabled: disableStatsExport, onClick: handleGenerateExport }} title='Full Group Stats' > - {contributors.length > 0 ? ( - - ) :
Loading...
- } + {!showContributors ? ( + + + You do not have permission to view this group's contributors. + + + ) : loading ? ( + + + + ) : error ? ( + + + There was an error. + + + {error?.message} + + + ) : contributors?.length > 0 ? ( + + ) : ( + + + There are no contributors to this group. + + + )} {memberIdsPerStats?.length > CONTRIBUTORS_PER_PAGE ? ( @@ -211,10 +216,13 @@ function GroupStats({ @@ -225,6 +233,7 @@ function GroupStats({ )} diff --git a/packages/lib-user/src/components/GroupStats/components/TopContributors/TopContributors.js b/packages/lib-user/src/components/GroupStats/components/TopContributors/TopContributors.js index 6f862c16e5..8f8bc878e4 100644 --- a/packages/lib-user/src/components/GroupStats/components/TopContributors/TopContributors.js +++ b/packages/lib-user/src/components/GroupStats/components/TopContributors/TopContributors.js @@ -1,5 +1,6 @@ -import { Grid, ResponsiveContext } from 'grommet' -import { arrayOf, number, shape, string } from 'prop-types' +import { Loader, SpacedText } from '@zooniverse/react-components' +import { Box, Grid, ResponsiveContext } from 'grommet' +import { arrayOf, bool, number, shape, string } from 'prop-types' import { useContext } from 'react' import { @@ -10,7 +11,9 @@ import { import MemberCard from '../MemberCard' function TopContributors({ + error, groupId, + loading, stats, topContributors }) { @@ -42,36 +45,62 @@ function TopContributors({ /> } > - - {topContributorsWithStats?.length ? ( - topContributorsWithStats.map((user) => ( -
  • - -
    • - )) - ) : null} -     + {loading ? ( + + + + ) : error ? ( + + + There was an error fetching the top contributors. + + + {error?.message} + + + ) : ( + + {topContributorsWithStats?.length ? ( + topContributorsWithStats.map((user) => ( +
  • + +
    • + )) + ) : null} +     + )} ) } TopContributors.propTypes = { groupId: string, + loading: bool, stats: shape({ top_contributors: arrayOf(shape({ count: number, diff --git a/packages/lib-user/src/components/MyGroups/MyGroups.js b/packages/lib-user/src/components/MyGroups/MyGroups.js index 77a72c5ac0..f4500b1349 100644 --- a/packages/lib-user/src/components/MyGroups/MyGroups.js +++ b/packages/lib-user/src/components/MyGroups/MyGroups.js @@ -1,7 +1,10 @@ -import { Grid } from 'grommet' -import { node } from 'prop-types' +import { Loader, SpacedText } from '@zooniverse/react-components' +import { Box, Grid, Paragraph } from 'grommet' +import { arrayOf, bool, shape, string } from 'prop-types' import styled from 'styled-components' +import GroupCardList from './components/GroupCardList' + const StyledGrid = styled(Grid)` gap: 20px 40px; justify-items: center; @@ -13,20 +16,69 @@ const StyledGrid = styled(Grid)` ` function MyGroups({ - children + error = undefined, + groups = [], + loading = false }) { return ( - - {children} - + <> + {loading ? ( + + + + ) : error ? ( + + + There was an error. + + + {error?.message} + + + ) : !groups?.length ? ( + + + You are not a member of any Groups. + + + Create one below + + + ) : ( + + + + )} + ) } MyGroups.propTypes = { - children: node + groups: arrayOf( + shape({ + id: string, + display_name: string + }) + ), + loading: bool } export default MyGroups diff --git a/packages/lib-user/src/components/MyGroups/MyGroups.stories.js b/packages/lib-user/src/components/MyGroups/MyGroups.stories.js index a9075fbda3..6293792675 100644 --- a/packages/lib-user/src/components/MyGroups/MyGroups.stories.js +++ b/packages/lib-user/src/components/MyGroups/MyGroups.stories.js @@ -1,7 +1,6 @@ import { Box } from 'grommet' import MyGroups from './MyGroups' -import GroupCard from './components/GroupCard/GroupCard' import { getActiveGroupsWithRoles } from './helpers/getActiveGroupsWithRoles' @@ -29,6 +28,7 @@ function ComponentDecorator(Story) { light: 'neutral-6' }} fill + overflow='auto' pad='30px' > @@ -38,20 +38,8 @@ function ComponentDecorator(Story) { } export const Default = { - render: () => ( - - {groups.map(group => ( - - ))} - - ) + args: { + groups: groups, + loading: false + } } diff --git a/packages/lib-user/src/components/MyGroups/MyGroupsContainer.js b/packages/lib-user/src/components/MyGroups/MyGroupsContainer.js index e58816ef41..28ddf0cd8d 100644 --- a/packages/lib-user/src/components/MyGroups/MyGroupsContainer.js +++ b/packages/lib-user/src/components/MyGroups/MyGroupsContainer.js @@ -21,7 +21,6 @@ import { getActiveGroupsWithRoles } from './helpers/getActiveGroupsWithRoles' import MyGroups from './MyGroups' import CreateButton from './components/CreateButton' -import GroupCardList from './components/GroupCardList' import GroupCreateFormContainer from './components/GroupCreateFormContainer' import PreviewLayout from './components/PreviewLayout' @@ -86,9 +85,11 @@ function MyGroupsContainer({ authUser, login, previewLayout = false }) { title='My Groups' pad={{ horizontal: '60px', vertical: '30px' }} > - - - + diff --git a/packages/lib-user/src/components/UserStats/UserStatsContainer.js b/packages/lib-user/src/components/UserStats/UserStatsContainer.js index e28bd8cc4d..1f6342e489 100644 --- a/packages/lib-user/src/components/UserStats/UserStatsContainer.js +++ b/packages/lib-user/src/components/UserStats/UserStatsContainer.js @@ -84,11 +84,13 @@ function UserStatsContainer({ page_size: 100 }) + const error = userError || statsError || projectStatsError || projectsError const loading = userLoading || statsLoading || projectStatsLoading || projectsLoading return ( )} - {status ? (
    {status}
    ) : ( + {status ? ( + + + {status} + + + ) : ( Children.map(children, child => cloneElement( child, diff --git a/packages/lib-user/src/components/shared/GroupContainer/helpers/getUserGroupStatus.js b/packages/lib-user/src/components/shared/GroupContainer/helpers/getUserGroupStatus.js index 142f2ee09e..828ca3245e 100644 --- a/packages/lib-user/src/components/shared/GroupContainer/helpers/getUserGroupStatus.js +++ b/packages/lib-user/src/components/shared/GroupContainer/helpers/getUserGroupStatus.js @@ -1,3 +1,4 @@ +import { Loader } from '@zooniverse/react-components' import { bool, shape, string } from 'prop-types' export function getUserGroupStatus({ @@ -22,7 +23,7 @@ export function getUserGroupStatus({ } if (groupLoading) { - return ('Loading...') + return () } if (groupError) { diff --git a/packages/lib-user/src/components/shared/GroupContainer/helpers/getUserGroupStatus.spec.js b/packages/lib-user/src/components/shared/GroupContainer/helpers/getUserGroupStatus.spec.js index d5b901e071..0d7e2b445c 100644 --- a/packages/lib-user/src/components/shared/GroupContainer/helpers/getUserGroupStatus.spec.js +++ b/packages/lib-user/src/components/shared/GroupContainer/helpers/getUserGroupStatus.spec.js @@ -1,3 +1,5 @@ +import { render, screen } from '@testing-library/react' + import { getUserGroupStatus } from './getUserGroupStatus' describe('components > shared > GroupContainer > getUserGroupStatus', function () { @@ -17,8 +19,10 @@ describe('components > shared > GroupContainer > getUserGroupStatus', function ( }) it('should return a message when loading the group', function () { - const result = getUserGroupStatus({ groupLoading: true }) - expect(result).to.equal('Loading...') + render(
    {getUserGroupStatus({ groupLoading: true })}
    ) + const result = screen.getByLabelText('Loading') + + expect(result).to.be.ok() }) it('should return a message when there is a group error', function () { diff --git a/packages/lib-user/src/components/shared/MainContent/MainContent.js b/packages/lib-user/src/components/shared/MainContent/MainContent.js index 69e58f60db..c689927826 100644 --- a/packages/lib-user/src/components/shared/MainContent/MainContent.js +++ b/packages/lib-user/src/components/shared/MainContent/MainContent.js @@ -39,6 +39,7 @@ const DEFAULT_SOURCE = { } function MainContent({ + error = undefined, loading = false, paramsValidationMessage = '', projects = [], @@ -255,6 +256,20 @@ function MainContent({ >     + ) : error ? ( + + + There was an error. + + + {error?.message} + + ) : noStats ? ( - - {topProjects.map(topProject => { - return ( -
  • - -
    • - ) - })} -     + {loading ? ( + + + + ) : ( + + {topProjects.map(topProject => { + return ( +
  • + +
    • + ) + })} +     + )} ) } @@ -99,6 +111,7 @@ TopProjects.propTypes = { })) }), grid: bool, + loading: bool, projects: arrayOf(shape({ avatar_src: string, description: string, diff --git a/packages/lib-user/src/components/shared/TopProjects/TopProjects.stories.js b/packages/lib-user/src/components/shared/TopProjects/TopProjects.stories.js index 102d4ca601..babd99b7a6 100644 --- a/packages/lib-user/src/components/shared/TopProjects/TopProjects.stories.js +++ b/packages/lib-user/src/components/shared/TopProjects/TopProjects.stories.js @@ -39,4 +39,10 @@ export const Grid = { grid: true, projects: PROJECTS } -} \ No newline at end of file +} + +export const Loading = { + args: { + loading: true + } +} diff --git a/packages/lib-user/src/hooks/usePanoptesMemberships.js b/packages/lib-user/src/hooks/usePanoptesMemberships.js index fa0e52c287..d74705a427 100644 --- a/packages/lib-user/src/hooks/usePanoptesMemberships.js +++ b/packages/lib-user/src/hooks/usePanoptesMemberships.js @@ -25,8 +25,8 @@ async function fetchMemberships({ query }) { const { body } = await panoptes.get('/memberships', query, { authorization }) return body } catch (error) { - console.log(error) - return null + console.error(error) + throw error } } diff --git a/packages/lib-user/src/hooks/usePanoptesProjects.js b/packages/lib-user/src/hooks/usePanoptesProjects.js index f695b28b87..7d6a89fed2 100644 --- a/packages/lib-user/src/hooks/usePanoptesProjects.js +++ b/packages/lib-user/src/hooks/usePanoptesProjects.js @@ -27,29 +27,35 @@ async function fetchProjects(query) { let projectsAccumulator = [] async function getProjects (page = 1) { - const response = await panoptesProjects.get({ - query: { - page, - ...query - }, - authorization - }) - const { meta, projects } = response?.body || {} + try { + const response = await panoptesProjects.get({ + query: { + page, + ...query + }, + authorization + }) - if (meta?.projects?.page_count > 5) { - console.warn('There are more than 500 projects related to this request. This is likely an error.') - return [] - } - - if (projects && projects.length) { - projectsAccumulator = projectsAccumulator.concat(projects) - } - - if (meta?.projects?.next_page) { - return getProjects(meta.projects.next_page) + const { meta, projects } = response?.body || {} + + if (meta?.projects?.page_count > 5) { + console.warn('There are more than 500 projects related to this request. This is likely an error.') + return [] + } + + if (projects && projects.length) { + projectsAccumulator = projectsAccumulator.concat(projects) + } + + if (meta?.projects?.next_page) { + return getProjects(meta.projects.next_page) + } + + return projectsAccumulator + } catch (error) { + console.error(error) + throw error } - - return projectsAccumulator } await getProjects(1) diff --git a/packages/lib-user/src/hooks/usePanoptesUser.js b/packages/lib-user/src/hooks/usePanoptesUser.js index ba1f4a94c8..bae26fcd60 100644 --- a/packages/lib-user/src/hooks/usePanoptesUser.js +++ b/packages/lib-user/src/hooks/usePanoptesUser.js @@ -26,7 +26,7 @@ async function getUser({ query }) { return users } catch (error) { console.error(error) - return null + throw error } } diff --git a/packages/lib-user/src/hooks/usePanoptesUserGroup.js b/packages/lib-user/src/hooks/usePanoptesUserGroup.js index 3d827e571c..2d52016d1f 100644 --- a/packages/lib-user/src/hooks/usePanoptesUserGroup.js +++ b/packages/lib-user/src/hooks/usePanoptesUserGroup.js @@ -32,8 +32,8 @@ async function fetchPanoptesUserGroup({ groupId }) { const user_group = user_groups?.[0] return user_group } catch (error) { - console.log(error) - return null + console.error(error) + throw error } } diff --git a/packages/lib-user/src/hooks/useStats.js b/packages/lib-user/src/hooks/useStats.js index 8081aeccd5..97671f1e41 100644 --- a/packages/lib-user/src/hooks/useStats.js +++ b/packages/lib-user/src/hooks/useStats.js @@ -44,8 +44,8 @@ async function fetchStats({ const data = await response.json() return data } catch (error) { - console.log(error) - return null + console.error(error) + throw error } } diff --git a/packages/lib-user/src/utils/fetchPanoptesUsers.js b/packages/lib-user/src/utils/fetchPanoptesUsers.js index 1042dffad2..b9dde9030c 100644 --- a/packages/lib-user/src/utils/fetchPanoptesUsers.js +++ b/packages/lib-user/src/utils/fetchPanoptesUsers.js @@ -4,18 +4,24 @@ export async function fetchPanoptesUsers(query) { let usersAccumulator = [] async function getUsers(page = 1) { - const response = await panoptes.get('/users', { page, ...query }) - const { meta, users } = response?.body || {} + try { + const response = await panoptes.get('/users', { page, ...query }) - if (users && users.length) { - usersAccumulator = usersAccumulator.concat(users) - } + const { meta, users } = response?.body || {} - if (meta?.users?.next_page) { - return getUsers(meta.users.next_page) - } + if (users && users.length) { + usersAccumulator = usersAccumulator.concat(users) + } - return usersAccumulator + if (meta?.users?.next_page) { + return getUsers(meta.users.next_page) + } + + return usersAccumulator + } catch (error) { + console.error(error) + throw error + } } await getUsers(1)