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

feat: insights feedback functionality #10342

Merged

Conversation

nickoferrall
Copy link
Contributor

Fix #10243

To test

  • Add insights feature flag, and go to a team where you're the team lead
  • Go to the Insights tab and click "Give us feedback"
  • See that it opens a modal
  • Enter info and click submit. See it stores it in amplitude

@github-actions github-actions bot requested a review from tianrunhe October 15, 2024 16:10
@nickoferrall
Copy link
Contributor Author

Hey @tianrunhe, would you mind reviewing this one? 🙏 Curious to get your thoughts on the useManualClientSideTrack hook. I couldn't use the useClientSideTrack hook because we don't want to send the data when the modal opens, but it also seemed excessive to create a mutation when we're not updating the db.

Hopefully, useManualClientSideTrack could be a helpful pattern.

@nickoferrall nickoferrall linked an issue Oct 15, 2024 that may be closed by this pull request
1 task
@tianrunhe
Copy link
Contributor

Sorry I didn't quite follow why you need a new hook? Can you explain a little bit more? Thank you!

@nickoferrall
Copy link
Contributor Author

@tianrunhe the useClientSideTrack tracks the event when the component loads, so when a modal is opened, for instance, we could track Modal Opened.

But what if we don't want to track the event when the component loads, but when a user does an action, like submitting a form.

At the moment, I think we can only handle this on the server, but creating a server mutation seems like overkill if you're not updating the db. So I created this hook so that we can track the action once an action happens, like submitting a form.

(event: string, options: Record<string, any>) => {
SendClientSideEvent(atmosphere, event, {
...options,
eventId: ++eventId
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 need eventId 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.

We have it in the other hook as there seemed to be a bug where some users were sending it non-stop, and we were trying to debug whether it was happening from that hook or elsewhere.

If it is happening in the other hook, it's more likely to be related to the useEffect which isn't in this hook. I'll remove it, and if the event fires non-stop too, I'll add eventId back.

packages/server/utils/analytics/analytics.ts Outdated Show resolved Hide resolved
@nickoferrall nickoferrall merged commit 4020908 into feat/10242/add-feedback-ui Oct 18, 2024
6 checks passed
@nickoferrall nickoferrall deleted the feat/10243/insights-feedback-func branch October 18, 2024 14:49
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.

Insights: send feedback functionality
2 participants