Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: show default insight #10283

Merged
merged 20 commits into from
Oct 16, 2024
Merged

feat: show default insight #10283

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
5af1ce4
add insights tab
nickoferrall Sep 18, 2024
571ff2b
update tabs logic
nickoferrall Sep 18, 2024
4b938b7
add team insight root and query
nickoferrall Sep 19, 2024
e80e457
add comment
nickoferrall Sep 19, 2024
7894e90
show team insight
nickoferrall Sep 20, 2024
0eabf01
add bottom margin
nickoferrall Sep 23, 2024
292f3f8
feat: team insights skeleton, minor style nits (#10274)
ackernaut Sep 30, 2024
8137177
Merge branch 'feat/10238/add-ui-skeleton' into feat/10239/show-defaul…
nickoferrall Oct 1, 2024
fc6400a
add meetingsCount and latestInsightByTeamId dataloader
nickoferrall Oct 1, 2024
a9218e4
rename link-styles
nickoferrall Oct 1, 2024
485e984
only team lead can view insights tab
nickoferrall Oct 1, 2024
1a8833e
fix merge conflict
nickoferrall Oct 1, 2024
9274437
use insightId
nickoferrall Oct 8, 2024
1581dbb
fix insight id type
nickoferrall Oct 8, 2024
90cb781
Merge branch 'master' into feat/10238/add-ui-skeleton
nickoferrall Oct 14, 2024
75173ad
fix merge conflicts
nickoferrall Oct 14, 2024
57dde60
update migration order
nickoferrall Oct 14, 2024
cfd8c78
feat: generate insight from empty state (#10323)
nickoferrall Oct 16, 2024
1f4b937
fix merge conflicts
nickoferrall Oct 16, 2024
78204ca
update migration order
nickoferrall Oct 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions packages/client/modules/teamDashboard/components/TeamDashHeader/TeamDashHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,9 @@ const TeamDashHeader = (props: Props) => {
id
name
}
isViewerTeamLead
viewerTeamMember {
isLead
}
teamMembers(sortBy: "preferredName") {
...InviteTeamMemberAvatar_teamMembers
...DashboardAvatar_teamMember
Expand All @@ -110,7 +112,8 @@ const TeamDashHeader = (props: Props) => {
`,
teamRef
)
const {organization, id: teamId, name: teamName, teamMembers, isViewerTeamLead} = team
const {organization, id: teamId, name: teamName, teamMembers, viewerTeamMember} = team
const isViewerTeamLead = viewerTeamMember?.isLead
const {name: orgName, id: orgId} = organization
const {history} = useRouter()

Expand Down
7 changes: 7 additions & 0 deletions packages/client/shared/gqlIds/InsightId.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export const InsightId = {
join: (ownerId: string, databaseId: number) => `insight:${ownerId}:${databaseId}`,
split: (id: string) => {
const [, ownerId, databaseId] = id.split(':')
return {ownerId, databaseId}
}
}
5 changes: 0 additions & 5 deletions packages/server/graphql/public/typeDefs/Team.graphql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,11 +187,6 @@ type Team {
"""
viewerTeamMember: TeamMember

"""
If the viewer is the team lead
"""
isViewerTeamLead: Boolean!

"""
The team member that is the team lead
"""
Expand Down
13 changes: 6 additions & 7 deletions packages/server/graphql/public/types/Team.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import TeamInsightsId from 'parabol-client/shared/gqlIds/TeamInsightsId'
import {InsightId} from '../../../../client/shared/gqlIds/InsightId'
import toTeamMemberId from '../../../../client/utils/relay/toTeamMemberId'
import {getUserId, isTeamMember} from '../../../utils/authorization'
import {getFeatureTier} from '../../types/helpers/getFeatureTier'
Expand Down Expand Up @@ -52,15 +53,13 @@ const Team: TeamResolvers = {
const teamMembers = await dataLoader.get('teamMembersByTeamId').load(teamId)
return teamMembers.find((teamMember) => teamMember.isLead)!
},
isViewerTeamLead: async ({id: teamId}, _args, {authToken, dataLoader}) => {
const viewerId = getUserId(authToken)
const teamMemberId = toTeamMemberId(teamId, viewerId)
const teamMember = await dataLoader.get('teamMembers').load(teamMemberId)
return teamMember?.isLead || false
},
insight: async ({id: teamId}, _args, {dataLoader}) => {
const insight = await dataLoader.get('latestInsightByTeamId').load(teamId)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm getting a type error. The problem seems to be that the id is an Int because I'm using SERIAL here.

Insight.id is defined as type ID here.

But I get a type error, that's resolved when I do:

  insight: async ({id: teamId}, _args, {dataLoader}) => {
    const insight = await dataLoader.get('latestInsightByTeamId').load(teamId)
    if (!insight) return null
    return {
      ...insight,
      id: 'hey'
    }
  }

So the problem seems to be the id is an int, but it's expecting a string. I could change the type Insight.id to Int here, but that doesn't seem clean when it's an ID.

Any ideas?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-1 Yes, the type error is correct and caught an error. We don't want to return the database id directly (of type int) because IDs should be globally unique (something to do with Relay caching). So we should create an GQL ID type for insights where we do the usual join/split and something like insights:${teamId}:${databaseId}.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah that makes sense, thanks!

return insight || null
if (!insight) return null
return {
...insight,
id: InsightId.join(teamId, insight.id)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 The nicer solution would be to have a resolver for the insights type. Define the DB type as source for it and then have a single id field doing this conversion. This way it will work in all occurrences where we would add this field.

}
}
}

Expand Down
Loading