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.

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

fix: empty discussion stages #10836

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

nickoferrall
Copy link
Contributor

@nickoferrall nickoferrall commented Feb 10, 2025

Fix #10799

Demo: https://www.loom.com/share/ed780b147c314f5ea88bcf632441ed7c

To test

  • Use Suggest Groups, reset groups, group & ungroup reflections, and see that the Discuss Stages all have reflections attached

Comment on lines 20 to 27
const groupReflectionCounts = reflections.reduce((counts: Record<string, number>, reflection) => {
counts[reflection.reflectionGroupId] = (counts[reflection.reflectionGroupId] || 0) + 1
return counts
}, {})

const activeGroups = reflectionGroups.filter(
(group) => (groupReflectionCounts[group.id] ?? 0) > 0
)
Copy link
Contributor

Choose a reason for hiding this comment

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

-1 we already call removeEmptyReflections in handleCompletedRetrospectiveStage,

const data: Record<string, any> = await removeEmptyReflections(meeting, dataLoader)

We should also remove empty reflection groups there, i.e. set them to isActive: false.
Otherwise if we cannot rely on this field being accurate when calling the retroReflectionGroupsByMeetingId loader, then we should get rid of the field completely and modify the loader to do a join with reflections to check there are some in the group.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, makes sense. I've updated removeEmptyReflections to set reflection groups to be inactive if they don't contact any reflections

Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we still need this logic here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, removed!

Comment on lines 20 to 27
const groupReflectionCounts = reflections.reduce((counts: Record<string, number>, reflection) => {
counts[reflection.reflectionGroupId] = (counts[reflection.reflectionGroupId] || 0) + 1
return counts
}, {})

const activeGroups = reflectionGroups.filter(
(group) => (groupReflectionCounts[group.id] ?? 0) > 0
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we still need this logic here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

grouped cards still become discussion topics when using suggest groups
2 participants