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

[Bug]: Update button in Rule is not working #2231

Closed
1 task done
satoren opened this issue Oct 12, 2023 · 4 comments · Fixed by #2235
Closed
1 task done

[Bug]: Update button in Rule is not working #2231

satoren opened this issue Oct 12, 2023 · 4 comments · Fixed by #2235
Labels

Comments

@satoren
Copy link

satoren commented Oct 12, 2023

Bug Description

After changing the Rule of Variant type and pressing the Update button,
I get a toast saying that I succeeded, but nothing is reflected.

Version Info

v1.28.1

Search

  • I searched for other open and closed issues before opening this

Steps to Reproduce

  1. create flag as Variant
  2. create segment
  3. create two variants a and b in flag
  4. add rule as Single Variant with a
  5. change Variant a to b and press update button
a.mov

Expected Behavior

Changes will be reflected

Additional Context

No response

@satoren satoren added the bug label Oct 12, 2023
@GeorgeMac
Copy link
Contributor

Thanks for bringing this issue in @satoren ! And the video and repro are super helpful. I will dig into this shortly.

@GeorgeMac
Copy link
Contributor

GeorgeMac commented Oct 12, 2023

Yep, I have replicated it exactly as described. I will dig into this some more.

Popping open the console I don't see any requests going to the backend.
Seems to suggest the issue is isolated to the frontend.

@GeorgeMac
Copy link
Contributor

I have a feeling this big old TODO is related:

// TODO: enable once we allow user to change variant of existing single dist rule
// } else if (ruleType === DistributionType.Single && selectedVariant) {
// // update variant if changed
// if (rule.rollouts[0].distribution.variantId !== selectedVariant.id) {
// try {
// await updateDistribution(
// namespace.key,
// flag.key,
// rule.id,
// rule.rollouts[0].distribution.id,
// {
// variantId: selectedVariant.id,
// rollout: 100
// }
// );
// } catch (err) {
// setError(err as Error);
// return;
// }
// }
// }

Going to wait for @markphelps @yquansah to come online and perhaps give some backstory.

@markphelps
Copy link
Collaborator

yeah, i dont think there was any well defined reason why we dont allow this. I just know that on the server side it is checked for and we have a TODO there as well: https:/flipt-io/flipt/blob/main/internal/storage/sql/common/rule.go#L685C1-L685C1

We have an internal issue tracking this (FLI-497). Perhaps is time we just go ahead and fix these TODOs on both the backend and frontend, as its been a while and no reasons why this should be dis-allowed have come to mind.

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 a pull request may close this issue.

3 participants