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

NPS: Update frequency of the NPS #20492

Merged
merged 2 commits into from
Jun 17, 2024
Merged

Conversation

simotae14
Copy link
Contributor

@simotae14 simotae14 commented Jun 11, 2024

What does it do?

It changes the delays to display the NPS for the first time (from 5 minutes to 30 minutes) and after the first dismissal (from 7 days to 14 days)

Why is it needed?

Many users complain about the frequency of NPS

How to test it?

  • Register for the first time on Strapi and on the Registration page accept the Newsletter checkbox
  • then wait after 30 minutes of activity and you will be able to see the NPS survey at the bottom of the page
  • when the NPS survey is displayed, close it by closing the modal
  • then after 14 days the survey will be proposed again

Related issue(s)/PR(s)

CS-795

@simotae14 simotae14 added source: core:admin Source is core/admin package pr: enhancement This PR adds or updates some part of the codebase or features labels Jun 11, 2024
@simotae14 simotae14 self-assigned this Jun 11, 2024
@joshuaellis
Copy link
Member

10 hours? How do you expect this to show? Does anyone actually leave an instance of Strapi open for 10 hours......?

@simotae14
Copy link
Contributor Author

I know it is a lot, we received a lot of complaints from the users for the frequency of the nps survey, the first request of the Product was to show the survey after 3 days of activity. This delay is not stored in the local storage, instead is counted from the first rendering of the component, product is aware of it, I'll let @yanniskadiri explain his reasons

@remidej
Copy link
Contributor

remidej commented Jun 11, 2024

This should be regardless of people closing their tabs or reloading right? Since we're using usePersistentState

@joshuaellis
Copy link
Member

joshuaellis commented Jun 11, 2024

the time isn't cumulative from what i can see on the code? we set a timeout, now 10 hours and when the component unmounts we don't add "how long its been open for" we just say, bye bye?

https://github.com/strapi/strapi/pull/20492/files#diff-f53ed4891968fac5b0e452d38c0fbbeb478121d6b46395c5a2e467f49654dcadL157-L165

EDIT: i'd love to be proved wrong please 😅

@simotae14
Copy link
Contributor Author

simotae14 commented Jun 11, 2024

yeah we are not using usePersistantState for this timeout, so it is like @joshuaellis says...maybe we can keep track of the time passed since the first registration like we do for the other delays

I mean store the value in the local storage

@joshuaellis
Copy link
Member

yeah we are not using usePersistantState for this timeout, so it is like @joshuaellis says...maybe we can keep track of the time passed since the first registration like we do for the other delays

I mean store the value in the local storage

yeah that would make more sense, if we've not shown the NPS keep track of "spent delay" & then when someone dismisses, you reset that spent time so next time to shows we count again.

@remidej
Copy link
Contributor

remidej commented Jun 11, 2024

I don't think you're wrong @joshuaellis 💀

So the problem is the use of a long setTimeout, right?

Maybe instead we can do some sort of polling solution with setInterval? Something that runs every minute or so to check if the modal should be displayed based on the data in localstorage

@simotae14
Copy link
Contributor Author

simotae14 commented Jun 11, 2024

yes, it can be a solution. Just take another aspect into consideration, the 5 minute timeout is now also used in conjunction with the other timeouts, I mean for example if 90 days have passed since the first response then before showing the nps survey we wait 5 minutes of activity (this it means if we change it to 10 hours we have to wait for 10 hours of activity)... I think we can avoid this behavior and immediately show the NPS survey when 90 days have passed (this value is calculated at the first rendering of the NPS component)

@joshuaellis
Copy link
Member

yes, it can be a solution. Just take another aspect into consideration, the 5 minute timeout is now also used in conjunction with the other timeouts, I mean for example if 90 days have passed since the first response then before showing the nps survey we wait 5 minutes of activity (this it means if we change it to 10 hours we have to wait for 10 hours of activity)... I think we can avoid this behavior and immediately show the NPS survey when 90 days have passed (this value is calculated at the first rendering of the NPS component)

This should be discussed with Yannis, IMO do it in a public channel so everyone has some visibility & we're all on the same page 😄

@simotae14
Copy link
Contributor Author

As a quick win we decided to try with 30 minutes just to see if we can stop the complaints, otherwise we can discuss and plan a better approach for the issue, thanks for the contribution team

@joshuaellis
Copy link
Member

good idea, 30 mins is probably reasonable. Someone should be monitoring the repsonses though, if we're not getting any then we know it's too long

Copy link

vercel bot commented Jun 11, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
contributor-docs ❌ Failed (Inspect) Jun 11, 2024 0:43am

@simotae14 simotae14 merged commit b11829e into develop Jun 17, 2024
29 of 30 checks passed
@simotae14 simotae14 deleted the enhancement/change-nps-delays branch June 17, 2024 08:05
@Convly Convly added this to the 4.25.1 milestone Jun 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: enhancement This PR adds or updates some part of the codebase or features source: core:admin Source is core/admin package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants