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

[2020 EOY Issue] Modal alert when enabling Auth Provider without keys (#12135)

Browse source 
* Start hooking things up

* further generalize adminModal, hook up relevant code

* Complete implementation

* remove comments

* Address QA suggestions

* Address QA suggestions

* Use object parameter for adminModal

* starting the admin cypress tests

* Code review suggestions

* remove incomplete test

* rename data attributes to make more sense

* complete the work

* use destructuring

* edit warning modal text
This commit is contained in:
Arit Amana 2021-01-13 13:54:39 -05:00 • committed by GitHub
parent 0f2c0c70b1
commit 3a984c8ee7
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 213 additions and 135 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
app/assets/stylesheets/admin.scss
View file

@ -147,10 +147,6 @@ label {
gap: var(--su-4);
padding-top: var(--su-6);
padding-bottom: var(--su-2);
.auth-warning {
padding: var(--su-2); // Override default crayons notice padding
}
}
.admin-config-checkmark {
@ -180,3 +176,7 @@ label {
.mod-actions__data {
max-width: 300px;
}
.capitalize {
text-transform: capitalize;
}

7
app/helpers/authentication_helper.rb
View file

@ -23,11 +23,6 @@ module AuthenticationHelper
Authentication::Providers.available.map(&:to_s)
end
def provider_keys_configured?(provider)
SiteConfig.public_send("#{provider}_key").present? &&
SiteConfig.public_send("#{provider}_secret").present?
end
def forem_creator_flow_enabled?
FeatureFlag.enabled?(:creator_onboarding) && waiting_on_first_user?
end
@ -45,7 +40,7 @@ module AuthenticationHelper
invite_only_mode_or_no_enabled_auth_options ? "crayons-tooltip" : ""
end
def disabled_attr_on_auth_provider_enablebtn
def disabled_attr_on_auth_provider_enable_btn
invite_only_mode_or_no_enabled_auth_options ? "disabled" : ""
end

92
app/javascript/admin/adminModal.js
View file

@ -2,54 +2,62 @@
* A function to generate the HTML for a Crayons modal within the /admin/ space.
*
* @function adminModal
* @param {string} title The title of the modal.
* @param {string} body The modal's content. May use HTML tags for styling.
* @param {string} confirmBtnText The text for the modal's "Confirm" button.
* @param {string} confirmBtnAction The function that fires when "Confirm" button is clicked.
* @param {string} cancelBtnText The text for the modal's "Cancel" button.
* @param {string} cancelBtnAction The function that fires when "Cancel" button is clicked.
* @param {string} customAttr A custom data attribute name. Will be apprended to the "data-" part.
* @param {string} customAttrValue The value of the custom attribute "customAttr".
* @param {Object} modalProps Properties of the Modal
* @param {string} modalProps.title The title of the modal.
* @param {string} modalProps.body The modal's content. May use HTML tags for styling.
* @param {string} modalProps.leftBtnText The text for the modal's left button.
* @param {string} modalProps.leftBtnAction The function that fires when left button is clicked.
* @param {string} modalProps.rightBtnText The text for the modal's right button.
* @param {string} modalProps.rightBtnAction The function that fires when right button is clicked.
* @param {string} modalProps.leftBtnClasses Classes applied to left button.
* @param {string} modalProps.rightBtnClasses Classes applied to right button.
* @param {string} modalProps.leftCustomDataAttr A custom data attribute for the left button.
* @param {string} modalProps.rightCustomDataAttr A custom data attribute for the right button.
*/
const adminModal = (
const adminModal = function ({
title,
body,
confirmBtnText,
confirmBtnAction,
cancelBtnText,
cancelBtnAction,
customAttr = null,
customAttrValue = null,
) => `
<div class="crayons-modal crayons-modal--s">
<div class="crayons-modal__box">
<header class="crayons-modal__box__header">
<p class="fw-bold fs-l">${title}</p>
<button type="button" class="crayons-btn crayons-btn--icon crayons-btn--ghost" data-action="click->config#closeAdminConfigModal">
<svg width="24" height="24" viewBox="0 0 24 24" class="crayons-icon" xmlns="http://www.w3.org/2000/svg">
<path d="M12 10.586l4.95-4.95 1.414 1.414-4.95 4.95 4.95 4.95-1.414 1.414-4.95-4.95-4.95 4.95-1.414-1.414 4.95-4.95-4.95-4.95L7.05 5.636l4.95 4.95z" />
</svg>
</button>
</header>
<div class="crayons-modal__box__body flex flex-col gap-4">
${body}
<div class="flex gap-2">
<button
class="crayons-btn crayons-btn--danger"
data-action="click->config#${confirmBtnAction}"
data-${customAttr}="${customAttrValue}">
${confirmBtnText}
</button>
<button
class="crayons-btn crayons-btn--secondary"
data-action="click->config#${cancelBtnAction}">
${cancelBtnText}
leftBtnText,
leftBtnAction,
rightBtnText,
rightBtnAction,
leftBtnClasses,
rightBtnClasses,
leftCustomDataAttr = null,
rightCustomDataAttr = null,
}) {
return `
<div class="crayons-modal crayons-modal--s">
<div class="crayons-modal__box">
<header class="crayons-modal__box__header">
<p class="fw-bold fs-l">${title}</p>
<button type="button" class="crayons-btn crayons-btn--icon crayons-btn--ghost" data-action="click->config#closeAdminModal">
<svg width="24" height="24" viewBox="0 0 24 24" class="crayons-icon" xmlns="http://www.w3.org/2000/svg">
<path d="M12 10.586l4.95-4.95 1.414 1.414-4.95 4.95 4.95 4.95-1.414 1.414-4.95-4.95-4.95 4.95-1.414-1.414 4.95-4.95-4.95-4.95L7.05 5.636l4.95 4.95z" />
</svg>
</button>
</header>
<div class="crayons-modal__box__body flex flex-col gap-4">
${body}
<div class="flex gap-2">
<button
class="crayons-btn ${leftBtnClasses}"
data-action="click->config#${leftBtnAction}"
${leftCustomDataAttr}>
${leftBtnText}
</button>
<button
class="crayons-btn ${rightBtnClasses}"
data-action="click->config#${rightBtnAction}"
${rightCustomDataAttr}>
${rightBtnText}
</button>
</div>
</div>
</div>
<div class="crayons-modal__overlay"></div>
</div>
<div class="crayons-modal__overlay"></div>
</div>
`;
`;
};
export default adminModal;

179
app/javascript/admin/controllers/config_controller.js
View file

@ -20,6 +20,8 @@ const emailAuthModalBody = `
export default class ConfigController extends Controller {
static targets = [
'authenticationProviders',
'authSectionForm',
'collectiveNoun',
'configModalAnchor',
'emailAuthSettingsBtn',
'enabledIndicator',
@ -66,10 +68,23 @@ export default class ConfigController extends Controller {
}
}
closeAdminConfigModal() {
closeAdminModal() {
// per forem/internalEngineering#336, need to short-circuit the
// "Update Site Configuration" button submit action; chose not to
// define Target on actual "Update" button (since it's a partial).
// The Target is defined on the Authentication form, and that section's
// "Update" button is queried.
const submitBtn = this.authSectionFormTarget.querySelector(
'input[type="submit"]',
);
this.configModalAnchorTarget.innerHTML = '';
document.body.style.height = 'inherit';
document.body.style.overflowY = 'inherit';
if (submitBtn.hasAttribute('disabled')) {
submitBtn.removeAttribute('disabled');
}
}
positionModalOnPage() {
@ -110,21 +125,23 @@ export default class ConfigController extends Controller {
activateEmailAuthModal(event) {
event.preventDefault();
this.configModalAnchorTarget.innerHTML = adminModal(
emailAuthModalTitle,
emailAuthModalBody,
'Confirm disable',
'disableEmailAuthFromModal',
'Cancel',
'closeAdminConfigModal',
);
this.configModalAnchorTarget.innerHTML = adminModal({
title: emailAuthModalTitle,
body: emailAuthModalBody,
leftBtnText: 'Confirm disable',
leftBtnAction: 'disableEmailAuthFromModal',
rightBtnText: 'Cancel',
rightBtnAction: 'closeAdminModal',
leftBtnClasses: 'crayons-btn--danger',
rightBtnClasses: 'crayons-btn--secondary',
});
this.positionModalOnPage();
}
disableEmailAuthFromModal(event) {
event.preventDefault();
this.disableEmailAuth(event);
this.closeAdminConfigModal(event);
this.closeAdminModal();
}
disableEmailAuth(event) {
@ -142,13 +159,13 @@ export default class ConfigController extends Controller {
enableOrEditAuthProvider(event) {
event.preventDefault();
const provider = event.target.dataset.authProviderEnable;
const providerName = event.target.dataset.providerName;
const enabledIndicator = document.getElementById(
`${provider}-enabled-indicator`,
`${providerName}-enabled-indicator`,
);
document
.getElementById(`${provider}-auth-settings`)
.getElementById(`${providerName}-auth-settings`)
.classList.remove('hidden');
event.target.classList.add('hidden');
@ -161,16 +178,16 @@ export default class ConfigController extends Controller {
disableAuthProvider(event) {
event.preventDefault();
const provider = event.target.dataset.authProvider;
const providerName = event.target.dataset.providerName;
const enabledIndicator = document.getElementById(
`${provider}-enabled-indicator`,
`${providerName}-enabled-indicator`,
);
const authEnableButton = document.querySelector(
`[data-auth-provider-enable="${provider}"]`,
const authEnableButton = document.getElementById(
`${providerName}-auth-btn`,
);
authEnableButton.setAttribute('data-enable-auth', 'false');
enabledIndicator.classList.remove('visible');
this.listAuthToBeEnabled(event);
this.listAuthToBeEnabled();
this.hideAuthProviderSettings(event);
}
@ -184,36 +201,37 @@ export default class ConfigController extends Controller {
activateAuthProviderModal(event) {
event.preventDefault();
const provider = event.target.dataset.authProvider;
const official_provider = event.target.dataset.authProviderOfficial;
this.configModalAnchorTarget.innerHTML = adminModal(
this.authProviderModalTitle(official_provider),
this.authProviderModalBody(official_provider),
'Confirm disable',
'disableAuthProviderFromModal',
'Cancel',
'closeAdminConfigModal',
'auth-provider',
provider,
);
const providerName = event.target.dataset.providerName;
const providerOfficialName = event.target.dataset.providerOfficialName;
this.configModalAnchorTarget.innerHTML = adminModal({
title: this.authProviderModalTitle(providerOfficialName),
body: this.authProviderModalBody(providerOfficialName),
leftBtnText: 'Confirm disable',
leftBtnAction: 'disableAuthProviderFromModal',
rightBtnText: 'Cancel',
rightBtnAction: 'closeAdminModal',
leftBtnClasses: 'crayons-btn--danger',
rightBtnClasses: 'crayons-btn--secondary',
leftCustomDataAttr: `data-provider-name=${providerName}`,
});
this.positionModalOnPage();
}
disableAuthProviderFromModal(event) {
event.preventDefault();
const provider = event.target.dataset.authProvider;
const authEnableButton = document.querySelector(
`[data-auth-provider-enable="${provider}"]`,
const providerName = event.target.dataset.providerName;
const authEnableButton = document.getElementById(
`${providerName}-auth-btn`,
);
const enabledIndicator = document.getElementById(
`${provider}-enabled-indicator`,
`${providerName}-enabled-indicator`,
);
authEnableButton.setAttribute('data-enable-auth', 'false');
this.listAuthToBeEnabled(event);
this.checkForAndGuardSoleAuthProvider();
enabledIndicator.classList.remove('visible');
this.hideAuthProviderSettings(event);
this.closeAdminConfigModal(event);
this.closeAdminModal();
}
checkForAndGuardSoleAuthProvider() {
@ -237,11 +255,13 @@ export default class ConfigController extends Controller {
hideAuthProviderSettings(event) {
event.preventDefault();
const provider = event.target.dataset.authProvider;
const providerName = event.target.dataset.providerName;
document
.getElementById(`${provider}-auth-settings`)
.getElementById(`${providerName}-auth-settings`)
.classList.add('hidden');
document.getElementById(`${provider}-auth-btn`).classList.remove('hidden');
document
.getElementById(`${providerName}-auth-btn`)
.classList.remove('hidden');
}
listAuthToBeEnabled() {
@ -249,7 +269,7 @@ export default class ConfigController extends Controller {
document
.querySelectorAll('[data-enable-auth="true"]')
.forEach((provider) => {
enabledProviderArray.push(provider.dataset.authProviderEnable);
enabledProviderArray.push(provider.dataset.providerName);
});
document.getElementById(
'auth_providers_to_enable',
@ -265,4 +285,85 @@ export default class ConfigController extends Controller {
}
}
// AUTH PROVIDERS FUNCTIONS END
enabledProvidersWithMissingKeys() {
const providersWithMissingKeys = [];
document
.querySelectorAll('[data-enable-auth="true"]')
.forEach((provider) => {
const { providerName } = provider.dataset;
if (
!document.getElementById(`site_config_${providerName}_key`).value ||
!document.getElementById(`site_config_${providerName}_secret`).value
) {
providersWithMissingKeys.push(providerName);
}
});
return providersWithMissingKeys;
}
generateProvidersList(providers) {
const list = providers.reduce((html, provider) => {
return `${html}<li class="capitalize">${provider}</li>`;
}, '');
return list;
}
missingAuthKeysModalBody(providers) {
return `
<p>You haven't filled out all of the required fields to enable the following authentication providers:</p>
<ul class="mb-0">${this.generateProvidersList(providers)}</ul>
<p class="mb-0">You may continue editing these authentication providers, or you may cancel.</p>
`;
}
submitForm() {
this.authSectionFormTarget.submit();
}
activateMissingKeysModal(providers) {
this.configModalAnchorTarget.innerHTML = adminModal({
title: 'Setup not complete',
body: this.missingAuthKeysModalBody(providers),
leftBtnText: 'Continue editing',
leftBtnAction: 'closeAdminModal',
rightBtnText: 'Cancel',
rightBtnAction: 'cancelAuthProviderEnable',
rightBtnClasses: 'crayons-btn--secondary',
});
}
configUpdatePrecheck(event) {
if (this.enabledProvidersWithMissingKeys().length > 0) {
event.preventDefault();
this.activateMissingKeysModal(this.enabledProvidersWithMissingKeys());
} else {
event.target.submit();
}
}
cancelAuthProviderEnable() {
const providers = this.enabledProvidersWithMissingKeys();
providers.forEach((provider) => {
const enabledIndicator = document.getElementById(
`${provider}-enabled-indicator`,
);
const authEnableButton = document.getElementById(`${provider}-auth-btn`);
authEnableButton.setAttribute('data-enable-auth', 'false');
enabledIndicator.classList.remove('visible');
this.listAuthToBeEnabled();
document
.getElementById(`${provider}-auth-settings`)
.classList.add('hidden');
document
.getElementById(`${provider}-auth-btn`)
.classList.remove('hidden');
this.closeAdminModal();
});
}
}

10
app/views/admin/configs/_apple_auth_provider_settings.html.erb
View file

@ -54,21 +54,21 @@ requires a custom partial
class="crayons-btn crayons-btn--danger"
data-action="click->config#activateAuthProviderModal"
data-tooltip="<%= tooltip_text_email_or_auth_provider_btns %>"
data-auth-provider="<%= provider.provider_name %>"
data-auth-provider-official="<%= provider.official_name %>"
<%= disabled_attr_on_auth_provider_enablebtn %>>
data-provider-name="<%= provider.provider_name %>"
data-provider-official-name="<%= provider.official_name %>"
<%= disabled_attr_on_auth_provider_enable_btn %>>
Disable
</button>
<button
class="crayons-btn crayons-btn--secondary"
data-auth-provider="<%= provider.provider_name %>"
data-provider-name="<%= provider.provider_name %>"
data-action="click->config#hideAuthProviderSettings">
Close
</button>
<% else %>
<button
class="crayons-btn crayons-btn--secondary"
data-auth-provider="<%= provider.provider_name %>"
data-provider-name="<%= provider.provider_name %>"
data-action="click->config#disableAuthProvider">
Undo
</button>

14
app/views/admin/configs/_auth_provider_settings.html.erb
View file

@ -1,8 +1,4 @@
<fieldset class="config-authentication-form">
<div
class="crayons-notice crayons-notice--warning auth-warning <%= provider_keys_configured?(provider.provider_name) ? "hidden" : "" %>">
<strong>Note:</strong> This authentication provider will <strong>not</strong> be enabled if the key or secret are missing. Please make sure that you enter your key and secret correctly.
</div>
<div class="crayons-field">
<%= admin_config_label :"#{provider.provider_name}_key" %>
<p class="crayons-field__description">
@ -29,21 +25,21 @@
class="crayons-btn crayons-btn--danger"
data-action="click->config#activateAuthProviderModal"
data-tooltip="<%= tooltip_text_email_or_auth_provider_btns %>"
data-auth-provider="<%= provider.provider_name %>"
data-auth-provider-official="<%= provider.official_name %>"
<%= disabled_attr_on_auth_provider_enablebtn %>>
data-provider-name="<%= provider.provider_name %>"
data-provider-official-name="<%= provider.official_name %>"
<%= disabled_attr_on_auth_provider_enable_btn %>>
Disable
</button>
<button
class="crayons-btn crayons-btn--secondary"
data-auth-provider="<%= provider.provider_name %>"
data-provider-name="<%= provider.provider_name %>"
data-action="click->config#hideAuthProviderSettings">
Close
</button>
<% else %>
<button
class="crayons-btn crayons-btn--secondary"
data-auth-provider="<%= provider.provider_name %>"
data-provider-name="<%= provider.provider_name %>"
data-action="click->config#disableAuthProvider">
Undo
</button>

13
app/views/admin/configs/show.html.erb
View file

@ -147,7 +147,7 @@
</div>
<% end %>
<%= form_for(SiteConfig.new, url: admin_config_path) do |f| %>
<%= form_for(SiteConfig.new, url: admin_config_path, html: { data: { action: "submit->config#configUpdatePrecheck", "config-target": "authSectionForm" } }) do |f| %>
<div class="card mt-3">
<%= render partial: "admin/shared/card_header",
locals: {
@ -211,7 +211,7 @@
data-button-text="<%= SiteConfig.allow_email_password_registration ? "edit" : "enable" %>"
data-config-target="emailAuthSettingsBtn"
data-action="click->config#enableOrEditEmailAuthSettings"
<%= disabled_attr_on_auth_provider_enablebtn %>>
<%= disabled_attr_on_auth_provider_enable_btn %>>
<%= SiteConfig.allow_email_password_registration ? "Edit" : "Enable" %>
</button>
@ -308,7 +308,7 @@
readonly: true %>
</div>
<% authentication_available_providers.each do |provider| %>
<% next if provider.provider_name == :apple && !Flipper.enabled?(:apple_auth) %>
<% next if provider.provider_name == :apple && !Flipper.enabled?(:apple_auth) %>
<section class="config-authentication__item mb-0">
<figure class="config-authentication__item--icon">
<%= inline_svg_tag("#{provider.provider_name}.svg", class: "crayons-icon", aria: true, title: "#{provider.official_name} logo") %>
@ -333,9 +333,10 @@
id="<%= provider.provider_name %>-auth-btn"
data-button-text="<%= authentication_enabled_providers.include?(provider) ? "edit" : "enable" %>"
data-action="click->config#enableOrEditAuthProvider"
data-auth-provider-enable="<%= provider.provider_name %>"
data-provider-name="<%= provider.provider_name %>"
data-provider-official-name="<%= provider.official_name %>"
data-enable-auth="<%= authentication_enabled_providers.include?(provider) ? "true" : "false" %>"
<%= disabled_attr_on_auth_provider_enablebtn %>>
<%= disabled_attr_on_auth_provider_enable_btn %>>
<%= authentication_enabled_providers.include?(provider) ? "Edit" : "Enable" %>
</button>
</div>
@ -1334,5 +1335,5 @@
</div>
</div>
</div>
<div data-target="config.configModalAnchor"></div>
<div data-config-target="configModalAnchor"></div>
</div>

25
spec/helpers/authentication_helper_spec.rb
View file

@ -38,29 +38,6 @@ RSpec.describe AuthenticationHelper, type: :helper do
end
end
describe "#provider_keys_configured?(provider)" do
let(:provider) { "facebook" }
it "returns true if provider key and secret both present" do
allow(SiteConfig).to receive(:"#{provider}_key").and_return("someKey")
allow(SiteConfig).to receive(:"#{provider}_secret").and_return("someSecret")
expect(provider_keys_configured?(provider)).to be(true)
end
it "returns false if either provider key or secret is missing" do
allow(SiteConfig).to receive(:"#{provider}_key").and_return("someKey")
allow(SiteConfig).to receive(:"#{provider}_secret").and_return("")
expect(provider_keys_configured?(provider)).to be(false)
allow(SiteConfig).to receive(:"#{provider}_key").and_return("")
allow(SiteConfig).to receive(:"#{provider}_secret").and_return("someSecret")
expect(provider_keys_configured?(provider)).to be(false)
end
end
describe "tooltip classes, attributes and content" do
context "when invite-only-mode enabled and no enabled registration options" do
before do
@ -74,7 +51,7 @@ RSpec.describe AuthenticationHelper, type: :helper do
end
it "returns 'disabled' attribute for relevant helper" do
expect(disabled_attr_on_auth_provider_enablebtn).to eq("disabled")
expect(disabled_attr_on_auth_provider_enable_btn).to eq("disabled")
end
it "returns appropriate text for 'tooltip_text_email_or_auth_provider_btns' helper" do