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(nimbus): Approval and rejection flow #12090

Open
wants to merge 32 commits into
base: main
Choose a base branch
from

Conversation

yashikakhurana
Copy link
Contributor

@yashikakhurana yashikakhurana commented Jan 22, 2025

Because

  • We want to add approval and rejection flow on the new summary page

This commit

  • Add option to approve and Launch
  • Add the option to reject the experiment with the message
  • Shows the timeout message

Fixes #12072
Note: This currently does not show the rejection reason-which I will add in the separate PR

Approval flow

Untitled.mov

Rejection flow

Screen.Recording.2025-01-22.at.3.38.00.PM.mov

Timeout message
Screenshot 2025-01-22 at 10 27 54 AM

@yashikakhurana
Copy link
Contributor Author

Blocked on #11932

@jaredlockhart
Copy link
Collaborator

Okay! This is already looking great 🎉 🎉 🎉

Couple things I notice from local testing:

  • doesn't display rejection note
  • doesn't immediately call remote settings sync

Look at what the V5 serializer does here on save:

https://github.com/mozilla/experimenter/blob/main/experimenter/experimenter/experiments/api/v5/serializers.py#L1044-L1055

We need to do these same things.

@yashikakhurana
Copy link
Contributor Author

Okay! This is already looking great 🎉 🎉 🎉

Couple things I notice from local testing:

* doesn't display rejection note

* doesn't immediately call remote settings sync

Look at what the V5 serializer does here on save:

https://github.com/mozilla/experimenter/blob/main/experimenter/experimenter/experiments/api/v5/serializers.py#L1044-L1055

We need to do these same things.

@jaredlockhart rejection message not showing up is related to changelog issue and that's why it's not included. I have added all the sync issues which was happening on the serializer to the forms, it should be called immediately now. I will investigate the changelog issue and made a point to implement this rejection message functionality with it.

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.

Experiment approval/rejection process UI
2 participants