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

Hide UI elements when Flipper is in read-only mode #772

Merged

Conversation

radar
Copy link
Contributor

@radar radar commented Nov 13, 2023

We have a particular business-critical application that uses Flipper flags, and we would like to provide a completely read-only UI with Flipper. I know of a way to do this, which is to set Flipper::UI's configuration to be read only. However, this same application is a bit a "nexus" for some smaller apps, and therefore we have multiple instances of Flipper::UI at play here. The different instances each control the flipper flags for the different applications.

I was able to see that there's a ReadOnly adapter however:

readonly_app = Flipper::UI.app(Flipper.new(Flipper::Adapters::ReadOnly.new(connection)))

But this doesn't hide all the UI buttons in Flipper UI. Here's what I see in that app by default:

image

Having all the buttons and forms present makes our security people uncomfortable.


So what I've tried to do in this PR is to cut out all of the UI that performs a write action, which then cuts the page down to this:

image

And the homepage to this:

image

I've also updated the "nooooope" error screen to show that the ReadOnly adapter might be in use:

image

@jnunemaker
Copy link
Collaborator

This is awesome! I'll try to review/merge soon but wanted you to know I appreciate the contribution!

@radar
Copy link
Contributor Author

radar commented Nov 13, 2023

Thanks for making the Flipper code so easy to read and reason about. It was a pleasure to contribute :)

Copy link
Collaborator

@jnunemaker jnunemaker left a comment

Choose a reason for hiding this comment

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

Looks great. I left a couple questions. If you have time to answer/address go for it. If you don't, just let me know and I'll get this over the finish line.

@@ -23,7 +23,7 @@ def get
end

def post
read_only if Flipper::UI.configuration.read_only
read_only if read_only?
Copy link
Collaborator

@jnunemaker jnunemaker Nov 17, 2023

Choose a reason for hiding this comment

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

Should this be render_read_only also?

@@ -3,15 +3,16 @@
<% if Flipper::UI.configuration.fun %>
<h4>But I've got a blank space baby...</h4>
<p>And I'll flip your features.</p>
<%- if !Flipper::UI.configuration.read_only && Flipper::UI.configuration.feature_creation_enabled -%>
<%= read_only? %>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Did you mean to leave this?

spec/flipper/ui/actions/add_feature_spec.rb Show resolved Hide resolved
@radar radar force-pushed the hide-ui-elements-in-read-only-mode branch from 8fe8574 to 15283bc Compare November 17, 2023 20:44
@radar
Copy link
Contributor Author

radar commented Nov 17, 2023

  1. Removed the leftover view code.
  2. Fixed up the add feature spec which wasn't actually using the read_only? query method.
  3. Added a regression test for ActorsGate, and then fixed the code.

@jnunemaker jnunemaker merged commit 3be8f72 into flippercloud:main Nov 18, 2023
32 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.

2 participants