✨ `Space` add "Website Settings" section and enforce ssl checkbox #1484

KellyAH · 2023-05-20T03:04:54Z

for 🥔🛠️🔐✨ Spaces: Enforce ssl connection #1473

Changes

Add "Website Settings" section
Add enforce ssl checkbox to Spaces edit page
only space members can see the new "Website Settings" section.
Update space policy with :enforce_ssl permitted param
add logic update the spaces table enforce_ssl with the checkbox value boolean (dependent on to ✨ 🥔 🥗 Spaces add Enforce_SSL flag #1479)
add unit tests

KellyAH · 2023-05-20T03:09:49Z

Test Cases to test on Local

If a user checks the Enforce SSL checkbox on /spaces/<space-name>/edit, and then clicks the Save changes button, the checkbox stays checked and the DB space table enforce_ssl value is true
If a user unchecks the Enforce SSL checkbox on /spaces/<space-name>/edit, and then clicks the Save changes button, the checkbox stays unchecked and the DB space table enforce_ssl value is false
Refresh/force refresh /spaces/<space-name>/edit preserves the Enforce SSL checkbox state.

KellyAH · 2023-05-20T03:11:59Z

@zspencer I forgot to ask you where on the /spaces/<space-name>/edit page do you want the Enforce SSL checkbox to be added? Utilities Section a different section, or a new Security section?

anaulin · 2023-05-20T17:12:10Z

It feels like we're starting to need a new "general space settings" section. (With maybe a better / more concise title, probably.)

zspencer · 2023-05-20T18:24:19Z

For now an "Operator Only Settings" section feels like maybe the right place; that way we could also expose the branded_domain field there

zspencer

This is a great direction! Hopefully the comments I left were useful!

app/controllers/invitations_controller.rb

app/views/space_invitation_mailer/space_invitation_email.text.erb

app/views/spaces/_utilities.html.erb

KellyAH · 2023-05-21T04:22:35Z

Will continue on sunday 5/21

KellyAH · 2023-05-22T04:38:15Z

@zspencer should I add browser tests or any unit tests for the new "Operator Only Settings" section? Or is it too early

zspencer · 2023-05-22T17:57:17Z

It would be good to have a spec/requests/spaces_controller_request_spec#update test to make sure that the enable_ssl flag gets set to true when the form comes through; but I think a full system test would be too much since we're probably going to move this around soon anyway.

zspencer

So long as you have confirmed that the data flips when the form submits I am comfortable with you merging this.

zspencer · 2023-05-22T18:00:10Z

Maybe this is actually "Website Settings" and not "Operator Only Settings" 🙃

KellyAH · 2023-05-23T04:31:26Z

Maybe this is actually "Website Settings" and not "Operator Only Settings" 🙃

Renamed to "Website Settings" in the code and UI in 5910f5c

KellyAH · 2023-05-24T05:41:52Z

app/controllers/spaces_controller.rb

+    elsif params[:enforce_ssl] == 1
+      policy_scope(Space).friendly.find(params[:id]).update(enforce_ssl: true)
+    elsif params[:enforce_ssl] == 0
+      policy_scope(Space).friendly.find(params[:id]).update(enforce_ssl: false)


Not sure if this is the correct way to update Space record based on the checkbox value.

Hmm, this does seem a little off-the-rails to me. Normally, if space_params includes enforce_ssl it will update just fine from the browser without needing to do a comparison check; so you shouldn't have to change the controller since you're permitting the new param in the SpacePolicy.

KellyAH · 2023-05-24T05:44:23Z

spec/requests/spaces_controller_request_spec.rb

+        new_entrance = space.rooms.sample
+        put polymorphic_path(space), params: {space: {entrance_id: new_entrance.id, enforce_ssl: 0}}
+
+        expect(space.enforce_ssl).to be false


Tests still fail. IDK why.

I see the Space record updating enforce_ssl to the correct value in Puma logs when submitting the form on local.

Both the Controller space instance and the spec space instance have the same id (since they're referencing the same piece of data where it's stored in the database) but will have a different location in memory; so changes to the space instance in the Controller will only be reflected in space instance in the spec once the space instance in the spec re-queries the database to get the updated value.

TL/DR: You may want to call space.reload after executing the put because the computer caches the data in the Space record independently between the Controller and spec until it is re-queried..

zspencer

I think this is good progress; and would be happy to merge it once the tests pass. That said; there are a couple ways to bring this closer to "Rails-normal":

Revert the changes in SpacesController
Use true and false in your spaces_controller_request_spec

zspencer · 2023-05-24T20:15:23Z

app/controllers/spaces_controller.rb

+    elsif params[:enforce_ssl] == 1
+      policy_scope(Space).friendly.find(params[:id]).update(enforce_ssl: true)
+    elsif params[:enforce_ssl] == 0
+      policy_scope(Space).friendly.find(params[:id]).update(enforce_ssl: false)


Hmm, this does seem a little off-the-rails to me. Normally, if space_params includes enforce_ssl it will update just fine from the browser without needing to do a comparison check; so you shouldn't have to change the controller since you're permitting the new param in the SpacePolicy.

zspencer · 2023-05-24T20:15:37Z

app/views/spaces/_website_settings.html.erb

+      <%= web_settings_form.check_box :enforce_ssl %>
+        Enforce SSL - Automatically redirects all Visitor traffic to this Space to https.
+      <% end %>
+    <%= web_settings_form.submit %>


zspencer · 2023-05-24T20:19:24Z

spec/requests/spaces_controller_request_spec.rb

+        new_entrance = space.rooms.sample
+        put polymorphic_path(space), params: {space: {entrance_id: new_entrance.id, enforce_ssl: 0}}
+
+        expect(space.enforce_ssl).to be false


Both the Controller space instance and the spec space instance have the same id (since they're referencing the same piece of data where it's stored in the database) but will have a different location in memory; so changes to the space instance in the Controller will only be reflected in space instance in the spec once the space instance in the spec re-queries the database to get the updated value.

TL/DR: You may want to call space.reload after executing the put because the computer caches the data in the Space record independently between the Controller and spec until it is re-queried..

zspencer · 2023-05-24T20:19:44Z

spec/requests/spaces_controller_request_spec.rb

+
+      it "enables Enforce SSL on a Space" do
+        new_entrance = space.rooms.sample
+        put polymorphic_path(space), params: {space: {entrance_id: new_entrance.id, enforce_ssl: 1}}


You can use false and true in your params (and it is generally preferred)

and fix unit tests. Co-authored-by: Zee Spencer <zspencer@users.noreply.github.com> Co-authored-by: Ana <anaulin@users.noreply.github.com> Co-authored-by: Dalton Pruitt <daltonrpruitt@users.noreply.github.com>

KellyAH added 4 commits May 20, 2023 01:53

add Space name to the Space Invitation subject line

9ec2b61

fix unit test

c143aa3

update Invitation Email from text with invitor name

eb7ece3

add enforce ssl checkbox to Space Utilities section

15b1b98

zspencer reviewed May 20, 2023

View reviewed changes

app/controllers/invitations_controller.rb Outdated Show resolved Hide resolved

app/views/space_invitation_mailer/space_invitation_email.text.erb Outdated Show resolved Hide resolved

app/views/spaces/_utilities.html.erb Outdated Show resolved Hide resolved

KellyAH added 2 commits May 22, 2023 03:36

revert work done for 1483 to main

5a0456f

add operator only section for enforce ssl checkbox

14fd46e

KellyAH self-assigned this May 22, 2023

gate operator only settings section display by operator

5ef6307

KellyAH marked this pull request as ready for review May 22, 2023 04:38

KellyAH changed the title ~~Space add enforce ssl checkbox~~ Space add "Operator Only Settings" section and enforce ssl checkbox May 22, 2023

KellyAH added the ✨ feature Reduces Client's Burden or Grants them Benefits label May 22, 2023

KellyAH changed the title ~~Space add "Operator Only Settings" section and enforce ssl checkbox~~ ✨ Space add "Operator Only Settings" section and enforce ssl checkbox May 22, 2023

zspencer approved these changes May 22, 2023

View reviewed changes

KellyAH added 2 commits May 23, 2023 04:06

Merge branch 'main' into 1473-add-Enforce-SSL-checkbox

1e40a9b

rename operator settings to website settings

5910f5c

KellyAH marked this pull request as draft May 23, 2023 04:27

KellyAH changed the title ~~✨ Space add "Operator Only Settings" section and enforce ssl checkbox~~ ✨ Space add "Website Settings" section and enforce ssl checkbox May 23, 2023

KellyAH and others added 3 commits May 23, 2023 04:53

add WIP unit tests

a05d5eb

add todo

187889c

Merge branch 'main' into 1473-add-Enforce-SSL-checkbox

039850d

KellyAH added 3 commits May 23, 2023 22:34

Enforce SSL checkbox positioned left of label text

0343d52

fill out unit tests

e60e394

Enforce SSL checkbox value in params updates Space

a9b5245

KellyAH commented May 24, 2023

View reviewed changes

remove todo

6c67379

zspencer approved these changes May 24, 2023

View reviewed changes

KellyAH and others added 2 commits May 24, 2023 17:51

Space Website Settings is accessable to space members.

f26640c

and fix unit tests. Co-authored-by: Zee Spencer <zspencer@users.noreply.github.com> Co-authored-by: Ana <anaulin@users.noreply.github.com> Co-authored-by: Dalton Pruitt <daltonrpruitt@users.noreply.github.com>

Merge branch 'main' into 1473-add-Enforce-SSL-checkbox

0f9487f

KellyAH marked this pull request as ready for review May 25, 2023 00:54

KellyAH merged commit 66b9fbf into main May 25, 2023

KellyAH deleted the 1473-add-Enforce-SSL-checkbox branch May 25, 2023 00:59

KellyAH mentioned this pull request May 25, 2023

🥔🛠️🔐✨ Spaces: Enforce ssl connection #1473

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ `Space` add "Website Settings" section and enforce ssl checkbox #1484

✨ `Space` add "Website Settings" section and enforce ssl checkbox #1484

KellyAH commented May 20, 2023 •

edited

Loading

KellyAH commented May 20, 2023 •

edited

Loading

KellyAH commented May 20, 2023

anaulin commented May 20, 2023

zspencer commented May 20, 2023

zspencer left a comment

KellyAH commented May 21, 2023

KellyAH commented May 22, 2023

zspencer commented May 22, 2023

zspencer left a comment

zspencer commented May 22, 2023

KellyAH commented May 23, 2023

KellyAH May 24, 2023

zspencer May 24, 2023

KellyAH May 24, 2023 •

edited

Loading

zspencer May 24, 2023

zspencer left a comment

zspencer May 24, 2023

zspencer May 24, 2023

zspencer May 24, 2023

zspencer May 24, 2023

✨ Space add "Website Settings" section and enforce ssl checkbox #1484

✨ Space add "Website Settings" section and enforce ssl checkbox #1484

Conversation

KellyAH commented May 20, 2023 • edited Loading

Changes

KellyAH commented May 20, 2023 • edited Loading

Test Cases to test on Local

KellyAH commented May 20, 2023

anaulin commented May 20, 2023

zspencer commented May 20, 2023

zspencer left a comment

Choose a reason for hiding this comment

KellyAH commented May 21, 2023

KellyAH commented May 22, 2023

zspencer commented May 22, 2023

zspencer left a comment

Choose a reason for hiding this comment

zspencer commented May 22, 2023

KellyAH commented May 23, 2023

KellyAH May 24, 2023

Choose a reason for hiding this comment

zspencer May 24, 2023

Choose a reason for hiding this comment

KellyAH May 24, 2023 • edited Loading

Choose a reason for hiding this comment

zspencer May 24, 2023

Choose a reason for hiding this comment

zspencer left a comment

Choose a reason for hiding this comment

zspencer May 24, 2023

Choose a reason for hiding this comment

zspencer May 24, 2023

Choose a reason for hiding this comment

zspencer May 24, 2023

Choose a reason for hiding this comment

zspencer May 24, 2023

Choose a reason for hiding this comment

✨ `Space` add "Website Settings" section and enforce ssl checkbox #1484

✨ `Space` add "Website Settings" section and enforce ssl checkbox #1484

KellyAH commented May 20, 2023 •

edited

Loading

KellyAH commented May 20, 2023 •

edited

Loading

KellyAH May 24, 2023 •

edited

Loading