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

Activate split_checkout feature by default #10693

Conversation

jibees
Copy link
Contributor

@jibees jibees commented Apr 13, 2023

What? Why?

What should we test?

  • As admin, visit /admin/feature-toggle/features deactivate or delete split_checkout feature
  • Deploy PR
  • Check that after deployment, feature split_checkout exists and is activate

Release notes

Changelog Category: User facing changes

The title of the pull request will be included in the release notes.

Dependencies

Documentation updates

@jibees jibees self-assigned this Apr 13, 2023
Copy link
Member

@mkllnk mkllnk left a comment

Choose a reason for hiding this comment

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

Nice, very simple.

Long-term, I was wondering if we should expand our current list of features with two more lists. We could have:

  1. current features with descriptions
  2. active features
  3. retired features

Then we always enable the active features and delete the retired features. Devs can then just move those feature names around. But the big downside would be that we enable the feature whenever the server is restarted even if we deactivated it again because there was a problem.

Another idea would be to have a rake task which can be triggered with ofn-install for all instances. That seems much better to me to control the timing or repeat it if necessary. Anyway, migrations are an easy way as well and don't require any additional work for now.

Copy link
Collaborator

@rioug rioug left a comment

Choose a reason for hiding this comment

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

@jibees
Copy link
Contributor Author

jibees commented Apr 17, 2023

Shouldn't you be updating this file as well https://github.com/openfoodfoundation/openfoodnetwork/blob/master/lib/open_food_network/feature_toggle.rb ?

You mean, by changing the description of the feature for example? Or anything else?

@rioug
Copy link
Collaborator

rioug commented Apr 17, 2023

I meant by moving it to RETIRED_FEATURE, but I realize now that I am jumping the gun, ignore me.

@drummer83 drummer83 self-assigned this Apr 18, 2023
@drummer83 drummer83 added pr-staged-uk staging.openfoodnetwork.org.uk pr-staged-au staging.openfoodnetwork.org.au and removed pr-staged-uk staging.openfoodnetwork.org.uk labels Apr 18, 2023
@drummer83
Copy link
Contributor

Hi @jibees,
As proposed by you I disabled split checkout in the feature toggles and verified that it is switched off:
image

After staging the PR the flipper is activated automatically again and I see the split checkout:
image

I can still disable the feature manually and use the legacy checkout.

I have also tried deleting the flipper and re-staged the PR. This still showed the legacy checkout:
image

So I switched from UK to AU and immediately deleted the flipper (while split checkout was active). This brought back the legacy checkout:
image

Then I staged the PR and this brought split checkout back and activated automatically.
image

I think this is ready to go but leaving for you to decide and merge.
Thanks!

@drummer83 drummer83 removed the pr-staged-au staging.openfoodnetwork.org.au label Apr 18, 2023
@jibees
Copy link
Contributor Author

jibees commented Apr 18, 2023

I have also tried deleting the flipper and re-staged the PR. This still showed the legacy checkout:

Yes, because migration has already been executed: that's why when you switch to AU (instead of UK) the migration can run, and then the split_checkout feature is activated.
So, to me, it's good.

@jibees jibees merged commit dab2a4a into openfoodfoundation:master Apr 18, 2023
51 checks passed
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.

Move all managed instances to "Fully Enabled" split_checkout toggle
4 participants