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(experiments): Always allow refreshing experiment results #26811

Merged
merged 1 commit into from
Dec 13, 2024

Conversation

danielbachhuber
Copy link
Contributor

Changes

Restores the UI to refresh experiment results when an experiment is completed or archived:

CleanShot 2024-12-10 at 10 25 19@2x

It looks like the conditional was originally added in 8ff6f2a (#21686) but it's not super clear why.

I wanted to refresh the results for https://posthoghelp.zendesk.com/agent/tickets/21723, but it's a completed experiment so I don't have the UI for it.

How did you test this code?

I verified that each of the buttons worked as expected when an experiment is completed.

@danielbachhuber danielbachhuber requested a review from a team December 10, 2024 18:28
Copy link
Contributor

Size Change: 0 B

Total Size: 1.11 MB

ℹ️ View Unchanged
Filename Size
frontend/dist/toolbar.js 1.11 MB

compressed-size-action

Copy link
Contributor

@andehen andehen left a comment

Choose a reason for hiding this comment

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

Looks good. I guess the reasoning behind it was that experiment results shouldn't change after it's stopped or archived, as the data should not change. But our calculations can change (bug fixing f.ex), and now with the data warehouse integration, data that the customer can control (let's say the refresh a table there to update data that is being joined in), it makes sense to always allow refresh.

@danielbachhuber
Copy link
Contributor Author

Requested a second opinion from @jurajmajerik, just in case there's some reason this shouldn't be there.

Copy link
Contributor

@jurajmajerik jurajmajerik left a comment

Choose a reason for hiding this comment

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

Makes sense allow this 👍

@danielbachhuber danielbachhuber merged commit 5dd300a into master Dec 13, 2024
97 checks passed
@danielbachhuber danielbachhuber deleted the experiments/allow-refresh branch December 13, 2024 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants