-
Notifications
You must be signed in to change notification settings - Fork 13
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
make privacy and legal links configurable #461
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some input mostly around feature flags
feature_privacy_imprint_enabled = | ||
Enum.member?( | ||
["true"], | ||
String.trim(System.get_env("MW_FEATURE_LEGAL_PRIVACY_LINKS") || "") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need the whole Enum.member?
stuff here unless I'm missing something:
enabled? = "true" == "MW_FEATURE_LEGAL_PRIVACY_LINKS" |> System.get_env("") |> String.trim()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd probably also wrap the feature flagging code for consistent code. I.e. the above feature also enabled for ""
which is a bit odd
@@ -21,4 +21,29 @@ defmodule MindwendelWeb.StaticPageController do | |||
form: form | |||
) | |||
end | |||
|
|||
def legal(conn, _params) do | |||
if Application.fetch_env!(:mindwendel, :options)[:feature_privacy_imprint_enabled] do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should be able to access it with the .syntax
(.feature_privacy_imprint...
) as they key should always be there and in these circumstances .
access is preferred
@@ -171,6 +177,7 @@ config :mindwendel, :options, | |||
), | |||
feature_file_upload: feature_file_upload, | |||
feature_brainstorming_removal_after_days: delete_brainstormings_after_days, | |||
feature_privacy_imprint_enabled: feature_privacy_imprint_enabled, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd usually suggest grouping the feature flags together under a key, grouping them. The key can then also be handled by a module that wraps it and the access. I.e. I usually have a module like FeatureFlag.enabled?(flag_name)
@@ -63,6 +69,11 @@ | |||
<% end %> | |||
</main> | |||
<footer class="mt-auto"> | |||
<p>Made with ♥ by Gerardo, Jannik and Nicho</p> | |||
<span class="me-2">Made with ❤️ in the 🇪🇺</span> | |||
<%= if Application.fetch_env!(:mindwendel, :options)[:feature_privacy_imprint_enabled] do %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having a module that wraps feature flags would avoid leaking the path/keys to the feature flag throughout the application :)
closes #355