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: update suggest group titles #10568

Merged
merged 11 commits into from
Dec 18, 2024
Merged

feat: update suggest group titles #10568

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
8931593
generate group titles with chatgpt
nickoferrall Nov 28, 2024
33f525e
generate ai title
nickoferrall Dec 2, 2024
6334abd
fix race confition
nickoferrall Dec 3, 2024
391916b
clean up
nickoferrall Dec 3, 2024
3a3e56b
show elipsis when loading
nickoferrall Dec 4, 2024
1044c0d
remove redundant ai check
nickoferrall Dec 4, 2024
b073840
simplify elipsis span
nickoferrall Dec 4, 2024
ecd3f1b
update suggest group titles
nickoferrall Dec 9, 2024
1429224
fix merge conflicts
nickoferrall Dec 11, 2024
b98b2d7
dont show loading state after removing title
nickoferrall Dec 11, 2024
8617536
update prompt and handle custom group edge case
nickoferrall Dec 17, 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
4 changes: 3 additions & 1 deletion packages/client/components/ReflectionGroup/ReflectionGroupTitleEditor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,11 @@ const ReflectionGroupTitleEditor = (props: Props) => {
const {id: reflectionGroupId, title} = reflectionGroup
const dirtyRef = useRef(false)
const initialTitleRef = useRef(title)
const isLoading = title === ''

const isLoading = title === '' && !dirtyRef.current
Dschoordsch marked this conversation as resolved.
Show resolved Hide resolved

const onChange = (e: React.ChangeEvent<HTMLInputElement>) => {
dirtyRef.current = true
const title = e.target.value
commitLocalUpdate(atmosphere, (store) => {
const reflectionGroup = store.get(reflectionGroupId)
Expand Down
1 change: 1 addition & 0 deletions packages/client/mutations/AutogroupMutation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ graphql`
reflectionGroups {
id
title
smartTitle
reflections {
id
plaintextContent
Expand Down
24 changes: 9 additions & 15 deletions packages/server/graphql/mutations/helpers/updateReflectionLocation/addReflectionToGroup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import dndNoise from 'parabol-client/utils/dndNoise'
import getKysely from '../../../../postgres/getKysely'
import updateGroupTitle from '../updateGroupTitle'
import {GQLContext} from './../../../graphql'
import updateSmartGroupTitle from './updateSmartGroupTitle'

const addReflectionToGroup = async (
reflectionId: string,
Expand All @@ -14,10 +15,9 @@ const addReflectionToGroup = async (
const reflection = await dataLoader.get('retroReflections').load(reflectionId)
if (!reflection) throw new Error('Reflection not found')
const {reflectionGroupId: oldReflectionGroupId, meetingId: reflectionMeetingId} = reflection
const [reflectionGroup, oldReflectionGroup] = await Promise.all([
dataLoader.get('retroReflectionGroups').loadNonNull(reflectionGroupId),
dataLoader.get('retroReflectionGroups').loadNonNull(oldReflectionGroupId)
])
const reflectionGroup = await dataLoader
.get('retroReflectionGroups')
.loadNonNull(reflectionGroupId)
dataLoader.get('retroReflectionGroups').clear(reflectionGroupId)
dataLoader.get('retroReflectionGroups').clear(oldReflectionGroupId)

Expand Down Expand Up @@ -56,17 +56,11 @@ const addReflectionToGroup = async (
.get('retroReflectionsByGroupId')
.load(oldReflectionGroupId)

const oldGroupHasSingleReflectionCustomTitle =
oldReflectionGroup.title !== oldReflectionGroup.smartTitle && oldReflections.length === 0
const newGroupHasSmartTitle = reflectionGroup.title === reflectionGroup.smartTitle

if (oldGroupHasSingleReflectionCustomTitle && newGroupHasSmartTitle) {
// Edge case of dragging a single card with a custom group name on a group with smart name
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the demo, I go through this edge case, which now works well

Copy link
Contributor

Choose a reason for hiding this comment

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

-1 I think the intention here is: you have 2 groups: group 1 has a smart title and group 2 has only one reflection and a custom title. When we move the one card over to group 1, we want to also apply the custom title to group 1, at least that's happening on master.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated. Here's a Loom demo with that edge case: https://www.loom.com/share/e669008de75447ddafc7a303c14eca33

await pg
.updateTable('RetroReflectionGroup')
.set({title: oldReflectionGroup.title, smartTitle: smartTitle ?? ''})
.where('id', '=', reflectionGroupId)
.execute()
if (smartTitle) {
// smartTitle exists when autogrouping or resetting groups
await updateSmartGroupTitle(reflectionGroupId, smartTitle)
reflectionGroup.smartTitle = smartTitle
reflectionGroup.title = smartTitle
} else {
const meeting = await dataLoader.get('newMeetings').loadNonNull(meetingId)
await updateGroupTitle({
Expand Down
Loading