maverick/docbrown
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

[15-Minute Fix] Ensure "Locked Screen" Checkbox Conditionally Renders (#14161)

Browse source 
* Uses Settings::UserExperience.public to conditionally render btn

* Updates landing page-related specs to account for private forem

* Uses Settings::UserExperience in place of ForemInstance in admin_creates_new_page_spec.rb

* Reverts Settings::UserExperience.public? changes throughout codebase
  - Removes any changes to ForemInstance.private?
  - Adds a new helper method, self.invitation_only?
  - Updates self.private? logic

* Updates landing page specs and reverts unnecessary changes
  - Reverts any changes that removed the check for
ForemInstance.private?
  - Updates landingPage.spec.js to use findByRole rather
than findByText

* Reverts a change to admin_manages_pages_spec.rb

* Updates landingPage.spec.js per PR review comment

* Updates #private_forem_or_no_enabled_auth_options and spec

* Uses .invite_only? in _providers_registration_form.html.erb

* Adjusts authenticationSection.spec.js to test that FB is enabled

* Updates necessary specs to check .invitation_only?

* Consistently uses invite_only_mode_or_no_enabled_auth_options
  - Updates all necessary places within the codebase to use
reverted method name, invite_only_mode_or_no_enabled_auth_providers
  - Updates #self.enabled to check ForemInstance.invitation_only?
rather than ForemInstance.private?
  - Reverts change to Facebook assertion within authenticationSection
e2e test

* Removes superfluous .to from authenticationSection.spec.js
This commit is contained in:
Julianna Tetreault 2021-07-12 14:14:43 -06:00 • committed by GitHub
parent 9e320afe78
commit e1e18aa0aa
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 50 additions and 37 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
app/helpers/authentication_helper.rb
View file

@ -45,22 +45,22 @@ module AuthenticationHelper
Settings::General.waiting_on_first_user
end
def private_forem_or_no_enabled_auth_options
ForemInstance.private? ||
def invite_only_mode_or_no_enabled_auth_options
ForemInstance.invitation_only? ||
(authentication_enabled_providers.none? &&
!Settings::Authentication.allow_email_password_registration)
end
def tooltip_class_on_auth_provider_enablebtn
private_forem_or_no_enabled_auth_options ? "crayons-tooltip" : ""
invite_only_mode_or_no_enabled_auth_options ? "crayons-tooltip" : ""
end
def disabled_attr_on_auth_provider_enable_btn
private_forem_or_no_enabled_auth_options ? "disabled" : ""
invite_only_mode_or_no_enabled_auth_options ? "disabled" : ""
end
def tooltip_text_email_or_auth_provider_btns
if private_forem_or_no_enabled_auth_options
if invite_only_mode_or_no_enabled_auth_options
"You cannot do this until you disable Invite Only Mode"
else
""

6
app/models/forem_instance.rb
View file

@ -28,7 +28,11 @@ class ForemInstance
(Settings::SMTP.user_name.present? && Settings::SMTP.password.present?) || ENV["SENDGRID_API_KEY"].present?
end
def self.private?
def self.invitation_only?
Settings::Authentication.invite_only_mode?
end
def self.private?
!Settings::UserExperience.public?
end
end

2
app/services/authentication/providers.rb
View file

@ -44,7 +44,7 @@ module Authentication
# TODO: [@forem/oss] ideally this should be "available - disabled"
# we can get there once we have feature flags
def self.enabled
return [] if ForemInstance.private?
return [] if ForemInstance.invitation_only?
Settings::Authentication.providers.map(&:to_sym).sort
end

4
app/views/admin/settings/forms/_authentication.html.erb
View file

@ -13,10 +13,10 @@
General settings
</h3>
<div
class="crayons-field crayons-field--checkbox <%= private_forem_or_no_enabled_auth_options ? "crayons-tooltip" : "" %>"
class="crayons-field crayons-field--checkbox <%= invite_only_mode_or_no_enabled_auth_options ? "crayons-tooltip" : "" %>"
data-tooltip="Unchecking this will enable Email Registration">
<%= f.check_box :invite_only_mode,
checked: private_forem_or_no_enabled_auth_options,
checked: invite_only_mode_or_no_enabled_auth_options,
data: { action: "config#adjustAuthenticationOptions", "config-target": "inviteOnlyMode" },
class: "crayons-checkbox" %>
<div class="mt-0">

2
app/views/devise/registrations/_registration_form.html.erb
View file

@ -2,7 +2,7 @@
<div class="registration crayons-card">
<div class="registration__content">
<h1 class="registration__title">
<% if params[:state] == "new-user" && ForemInstance.private? %>
<% if params[:state] == "new-user" && ForemInstance.invitation_only? %>
<%= community_name %> is invite only.
<% else %>
Welcome to <%= community_name %>

2
app/views/shared/authentication/_providers_registration_form.html.erb
View file

@ -8,7 +8,7 @@
<% end %>
<% end %>
<% end %>
<% if params[:state] == "new-user" && Settings::Authentication.allow_email_password_registration && !ForemInstance.private? %>
<% if params[:state] == "new-user" && Settings::Authentication.allow_email_password_registration && !ForemInstance.invitation_only? %>
<%= link_to "#{inline_svg_tag('email.svg', aria_hidden: true, class: 'crayons-icon', title: 'email')}Sign up with Email".html_safe,
request.params.merge(state: "email_signup").except("i"),
class: "crayons-btn crayons-btn--l crayons-btn--brand-email crayons-btn--icon-left whitespace-nowrap",

17
cypress/integration/adminFlows/config/authenticationSection.spec.js
View file

@ -23,9 +23,11 @@ describe('Authentication Section', () => {
cy.visit('/admin/customization/config');
cy.findByTestId('authSectionForm').as('authSectionForm');
cy.get('@authSectionForm').findByText('Authentication').click();
cy.get('@authSectionForm')
.findByLabelText('Invite-only mode')
.findByRole('heading', { name: 'Authentication' })
.click();
cy.get('@authSectionForm')
.findByRole('checkbox', { name: 'Invite-only mode' })
.should('not.be.checked')
.check();
@ -34,7 +36,9 @@ describe('Authentication Section', () => {
.type(
`My username is @${username} and this action is 100% safe and appropriate.`,
);
cy.get('@authSectionForm').findByText('Update Settings').click();
cy.get('@authSectionForm')
.findByRole('button', { name: 'Update Settings' })
.click();
cy.url().should('contains', '/admin/customization/config');
@ -43,10 +47,11 @@ describe('Authentication Section', () => {
cy.findByText('Successfully updated settings.').should('be.visible');
cy.get('@authSectionForm').findByText('Authentication').click();
cy.get('@authSectionForm')
.findByLabelText('Invite-only mode')
.findByRole('heading', { name: 'Authentication' })
.click();
cy.get('@authSectionForm')
.findByRole('checkbox', { name: 'Invite-only mode' })
.should('be.checked');
// Ensure that none of the authentication providers are enabled.

20
cypress/integration/adminFlows/pages/landingPage.spec.js
View file

@ -4,23 +4,27 @@ describe('Set a landing page from the admin portal', () => {
cy.fixture('users/adminUser.json').as('user');
cy.get('@user').then((user) => {
cy.loginAndVisit(user, '/admin/customization/config').then(() => {
cy.get('#new_settings_user_experience').as('userExperienceSectionForm');
// Ensure Forem instance is private
// NOTE: @citizen428 - We may need to find a better situation for this
// long-term.
cy.findByTestId('authSectionForm').as('authSectionForm');
cy.get('@authSectionForm').findByText('Authentication').click();
cy.get('@authSectionForm')
.findByLabelText('Invite-only mode')
.should('not.be.checked')
.check();
cy.get('@userExperienceSectionForm')
.findByRole('heading', { name: 'User Experience and Brand' })
.click();
cy.get('@userExperienceSectionForm')
.findByRole('checkbox', { name: 'Public' })
.should('be.checked')
.uncheck();
cy.get('@authSectionForm')
cy.get('@userExperienceSectionForm')
.findByPlaceholderText('Confirmation text')
.type(
`My username is @${user.username} and this action is 100% safe and appropriate.`,
);
cy.get('@authSectionForm').findByText('Update Settings').click();
cy.get('@userExperienceSectionForm')
.findByRole('button', { name: 'Update Settings' })
.click();
cy.visit('/admin/customization/pages');
});

8
spec/system/admin/admin_manages_pages_spec.rb
View file

@ -124,7 +124,7 @@ RSpec.describe "Admin manages pages", type: :system do
it "allows a landing page to be updated", :aggregate_failures do
visit edit_admin_page_path(current_landing_page.id)
expect(page).to have_content("Use as 'Locked Screen")
expect(page).to have_content("Use as 'Locked Screen'")
uncheck "Use as 'Locked Screen'"
click_on("Update Page")
expect(page).to have_current_path(admin_pages_path)
@ -132,7 +132,7 @@ RSpec.describe "Admin manages pages", type: :system do
it "allows an Admin to click through to the current landing page via the modal", :aggregate_failures do
visit edit_admin_page_path(new_landing_page.id)
expect(page).to have_content("Use as 'Locked Screen")
expect(page).to have_content("Use as 'Locked Screen'")
check "Use as 'Locked Screen'"
expect(page).to have_link("Current Locked Screen: #{new_landing_page.title}")
click_on("Current Locked Screen")
@ -142,7 +142,7 @@ RSpec.describe "Admin manages pages", type: :system do
it "allows an Admin to overwrite the current landing page via the checkbox and modal", :aggregate_failures do
visit edit_admin_page_path(new_landing_page.id)
expect(page).to have_content("Use as 'Locked Screen")
expect(page).to have_content("Use as 'Locked Screen'")
check "Use as 'Locked Screen'"
expect(page).to have_link("Current Locked Screen: #{new_landing_page.title}")
click_on("Overwrite current locked screen")
@ -155,7 +155,7 @@ RSpec.describe "Admin manages pages", type: :system do
it "does not give admins the option to set a lock screen" do
allow(ForemInstance).to receive(:private).and_return(false)
visit edit_admin_page_path(new_landing_page.id)
expect(page).not_to have_content("Use as 'Locked Screen")
expect(page).not_to have_content("Use as 'Locked Screen'")
end
end
end

4
spec/system/authentication/user_logs_in_with_email_spec.rb
View file

@ -114,9 +114,9 @@ RSpec.describe "Authenticating with Email" do
end
end
context "when community is in invite only mode" do
context "when community is in invite-only mode" do
before do
allow(ForemInstance).to receive(:private?).and_return(true)
allow(ForemInstance).to receive(:invitation_only?).and_return(true)
end
it "doesn't present the authentication option" do

4
spec/system/authentication/user_logs_in_with_facebook_spec.rb
View file

@ -232,9 +232,9 @@ RSpec.describe "Authenticating with Facebook" do
end
end
context "when community is in invite only mode" do
context "when community is in invite-only mode" do
before do
allow(ForemInstance).to receive(:private?).and_return(true)
allow(ForemInstance).to receive(:invitation_only?).and_return(true)
end
it "doesn't present the authentication option" do

4
spec/system/authentication/user_logs_in_with_github_spec.rb
View file

@ -198,9 +198,9 @@ RSpec.describe "Authenticating with GitHub" do
end
end
context "when community is in invite only mode" do
context "when community is in invite-only mode" do
before do
allow(ForemInstance).to receive(:private?).and_return(true)
allow(ForemInstance).to receive(:invitation_only?).and_return(true)
end
it "doesn't present the authentication option" do

4
spec/system/authentication/user_logs_in_with_twitter_spec.rb
View file

@ -190,9 +190,9 @@ RSpec.describe "Authenticating with Twitter" do
end
end
context "when community is in invite only mode" do
context "when community is in invite-only mode" do
before do
allow(ForemInstance).to receive(:private?).and_return(true)
allow(ForemInstance).to receive(:invitation_only?).and_return(true)
end
it "doesn't present the authentication option" do