Skip to content

Allow FeatureService to switch features per form#865

Merged
jamie-o-wilkinson merged 1 commit into
mainfrom
spike/feature-flag-by-form
Aug 7, 2024
Merged

Allow FeatureService to switch features per form#865
jamie-o-wilkinson merged 1 commit into
mainfrom
spike/feature-flag-by-form

Conversation

@jamie-o-wilkinson
Copy link
Copy Markdown
Contributor

@jamie-o-wilkinson jamie-o-wilkinson commented Aug 6, 2024

What problem does this pull request solve?

This commit allows features to be toggled on a per form basis in addition to the global feature flag. This is done in a similar way as per org switching in forms-admin. You can override the default value of a feature flag by providing key value pairs under a forms object, where the key is the form ID. For example:

features:
  flag_test:
    enabled: false
    forms:
      "123": true

We'd like to use this to do some user research for additional submission types with specific forms.

Trello card: https://trello.com/c/5xPfvcTJ

Things to consider when reviewing

  • Ensure that you consider the wider context.
  • Does it work when run on your machine?
  • Is it clear what the code is doing?
  • Do the commit messages explain why the changes were made?
  • Are there all the unit tests needed?
  • Has all relevant documentation been updated?

@jamie-o-wilkinson jamie-o-wilkinson self-assigned this Aug 6, 2024
@jamie-o-wilkinson jamie-o-wilkinson force-pushed the spike/feature-flag-by-form branch from 8694309 to 86b424a Compare August 6, 2024 10:52
@jamie-o-wilkinson jamie-o-wilkinson marked this pull request as ready for review August 6, 2024 10:55
@jamie-o-wilkinson jamie-o-wilkinson marked this pull request as draft August 6, 2024 10:56
@jamie-o-wilkinson jamie-o-wilkinson marked this pull request as ready for review August 6, 2024 11:13
@jamie-o-wilkinson jamie-o-wilkinson force-pushed the spike/feature-flag-by-form branch from 86b424a to fe1c173 Compare August 6, 2024 11:14
Copy link
Copy Markdown
Contributor

@stephencdaly stephencdaly left a comment

Choose a reason for hiding this comment

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

Changes look good. I wonder slightly whether it might be better to use the form ID rather than the slug. The reason for this is that if a form is renamed, the slug changes. I would have thought that if a form is renamed, we wouldn't want features to be disabled. Thoughts?

@DavidBiddle
Copy link
Copy Markdown
Contributor

I'm inclined to agree with Stephen on using ID instead of slug - as well as being changeable I don't think we enforce uniqueness on form slugs, so ID would target the form we want more specifically

@jamie-o-wilkinson jamie-o-wilkinson force-pushed the spike/feature-flag-by-form branch from fe1c173 to 16bb71e Compare August 6, 2024 14:19
@jamie-o-wilkinson
Copy link
Copy Markdown
Contributor Author

Good points, thanks both! I've updated to use form ID instead of slug. This makes the settings file less readable, but we can just go to admin to find out which form maps to which ID for now if need be.

@jamie-o-wilkinson jamie-o-wilkinson force-pushed the spike/feature-flag-by-form branch from 16bb71e to 3f25522 Compare August 6, 2024 14:42
Copy link
Copy Markdown
Contributor

@stephencdaly stephencdaly left a comment

Choose a reason for hiding this comment

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

Changes look good and I've tested it locally by modifying an existing feature flag to use this.
Just one minor comment.

Comment thread app/services/feature_service.rb Outdated
DavidBiddle
DavidBiddle previously approved these changes Aug 6, 2024
Copy link
Copy Markdown
Contributor

@DavidBiddle DavidBiddle left a comment

Choose a reason for hiding this comment

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

Works for me locally, code looks good, nice readable tests 🎉

This commit allows features to be toggled on a per form basis in
addition to the global feature flag. This is done in a similar way as
per org switching in forms-admin. You can override the default value of
a feature flag by providing key value pairs under a forms object, where
the key is the form ID. For example:

```
features:
  flag_test:
    enabled: false
    forms:
      "123": true
```

We'd like to use this to do some user research for additional
submission types with specific forms.
@jamie-o-wilkinson jamie-o-wilkinson force-pushed the spike/feature-flag-by-form branch from 97b4741 to 1c479c3 Compare August 6, 2024 15:06
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Aug 6, 2024

@jamie-o-wilkinson jamie-o-wilkinson merged commit c91dc01 into main Aug 7, 2024
@jamie-o-wilkinson jamie-o-wilkinson deleted the spike/feature-flag-by-form branch August 7, 2024 08:53
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