diff --git a/app/controllers/stories/feeds_controller.rb b/app/controllers/stories/feeds_controller.rb index 11b75addb..84b7cfdb1 100644 --- a/app/controllers/stories/feeds_controller.rb +++ b/app/controllers/stories/feeds_controller.rb @@ -65,8 +65,8 @@ module Stories end def signed_out_base_feed - strategy = AbExperiment.get(experiment: AbExperiment::CURRENT_FEED_STRATEGY_EXPERIMENT, controller: self, user: current_user, - default_value: AbExperiment::ORIGINAL_VARIANT) + strategy = AbExperiment.get(experiment: AbExperiment::CURRENT_FEED_STRATEGY_EXPERIMENT, controller: self, + user: current_user, default_value: AbExperiment::ORIGINAL_VARIANT) feed = if strategy.weighted_query_strategy? Articles::Feeds::WeightedQueryStrategy.new(user: current_user, page: @page, tags: params[:tag]) elsif Settings::UserExperience.feed_strategy == "basic" diff --git a/app/javascript/admin/controllers/config_controller.js b/app/javascript/admin/controllers/config_controller.js index fc4f3ba4a..5aeb63a79 100644 --- a/app/javascript/admin/controllers/config_controller.js +++ b/app/javascript/admin/controllers/config_controller.js @@ -16,6 +16,8 @@ const emailAuthModalBody = `

If you disable Email address as a registration option, people cannot create an account with their email address.

However, people who have already created an account using their email address can continue to login.

`; +// NOTE: In an effort to move away from Stimulus and create consistency across the codebase +// we are using vanilla JavaScript (app/javascript/packs/admin/config) to handle any new interactions. export default class ConfigController extends Controller { static targets = [ 'authenticationProviders', diff --git a/app/javascript/packs/admin/config/smtp.js b/app/javascript/packs/admin/config/smtp.js new file mode 100644 index 000000000..e37a07596 --- /dev/null +++ b/app/javascript/packs/admin/config/smtp.js @@ -0,0 +1,23 @@ +document + .getElementById('settings_smtp_own_email_server') + ?.addEventListener('change', (event) => { + const [customSMTPSection] = document.getElementsByClassName( + 'js-custom-smtp-section', + ); + + if (event.target.checked) { + customSMTPSection.classList.remove('hidden'); + } else { + // when the user indicates that they do not want to use their own server + // we clear the form values except for those fields that have a default value. + const inputs = customSMTPSection.getElementsByTagName('input'); + const haveDefaultValues = ['authentication']; + + for (const input of inputs) { + if (!haveDefaultValues.some((el) => input.name.includes(el))) { + input.value = ''; + } + } + customSMTPSection.classList.add('hidden'); + } + }); diff --git a/app/lib/constants/settings/smtp.rb b/app/lib/constants/settings/smtp.rb index 909506d0f..42e5b131a 100644 --- a/app/lib/constants/settings/smtp.rb +++ b/app/lib/constants/settings/smtp.rb @@ -4,7 +4,7 @@ module Constants DETAILS = { address: { description: "Address of the remote mail server", - placeholder: "ie. smtp.gmail.com" + placeholder: "i.e. smtp.gmail.com" }, port: { description: "The port that your mail server runs on", @@ -12,8 +12,9 @@ module Constants }, authentication: { description: " If your mail server requires authentication, " \ - "you need to specify the authentication type here", - placeholder: "ie. plain, login, or cram_md5" + "you need to specify the authentication type here. " \ + " i.e. plain, login, or cram_md5", + placeholder: "i.e. plain, login, or cram_md5" }, user_name: { description: "If your mail server requires authentication, copy the username from your server", diff --git a/app/models/forem_instance.rb b/app/models/forem_instance.rb index 008a67d89..accd82e9d 100644 --- a/app/models/forem_instance.rb +++ b/app/models/forem_instance.rb @@ -32,6 +32,14 @@ class ForemInstance Settings::SMTP.provided_minimum_settings? || ENV["SENDGRID_API_KEY"].present? end + def self.sendgrid_enabled? + ENV["SENDGRID_API_KEY"].present? + end + + def self.only_sendgrid_enabled? + ForemInstance.sendgrid_enabled? && !Settings::SMTP.provided_minimum_settings? + end + def self.invitation_only? Settings::Authentication.invite_only_mode? end diff --git a/app/models/settings/smtp.rb b/app/models/settings/smtp.rb index da2e8b246..220c397d1 100644 --- a/app/models/settings/smtp.rb +++ b/app/models/settings/smtp.rb @@ -2,11 +2,12 @@ module Settings class SMTP < Base self.table_name = :settings_smtp - OPTIONS = %i[address port authentication user_name password domain].freeze + OPTIONS = %i[user_name password address authentication domain port].freeze + AUTHENTICATION_METHODS = %w[plain login cram_md5].freeze setting :address, type: :string, default: ApplicationConfig["SMTP_ADDRESS"].presence setting :authentication, type: :string, default: ApplicationConfig["SMTP_AUTHENTICATION"].presence, - validates: { inclusion: %w[plain login cram_md5] } + validates: { inclusion: AUTHENTICATION_METHODS } setting :domain, type: :string, default: ApplicationConfig["SMTP_DOMAIN"].presence setting :password, type: :string, default: ApplicationConfig["SMTP_PASSWORD"].presence setting :port, type: :integer, default: ApplicationConfig["SMTP_PORT"].presence || 25 diff --git a/app/services/articles/feeds/weighted_query_strategy.rb b/app/services/articles/feeds/weighted_query_strategy.rb index 5268f639d..2a29a9bd1 100644 --- a/app/services/articles/feeds/weighted_query_strategy.rb +++ b/app/services/articles/feeds/weighted_query_strategy.rb @@ -119,7 +119,7 @@ module Articles # Weight to give to the number of comments on the article. comments_count_factor: { clause: "articles.comments_count", - cases: (0..9).map { |n| [n, 0.8 + 0.02 * n] }, + cases: (0..9).map { |n| [n, 0.8 + (0.02 * n)] }, fallback: 1, requires_user: false, group_by: "articles.comments_count" @@ -190,7 +190,7 @@ module Articles # user follows and the article has. matching_tags_factor: { clause: "LEAST(10.0, SUM(followed_tags.points))::integer", - cases: (0..9).map { |n| [n, 0.70 + 0.0303 * n] }, + cases: (0..9).map { |n| [n, 0.70 + (0.0303 * n)] }, fallback: 1, requires_user: true, joins: ["LEFT OUTER JOIN taggings diff --git a/app/views/admin/settings/forms/_smtp.html.erb b/app/views/admin/settings/forms/_smtp.html.erb index eb57f6763..02f3ecb91 100644 --- a/app/views/admin/settings/forms/_smtp.html.erb +++ b/app/views/admin/settings/forms/_smtp.html.erb @@ -1,22 +1,47 @@ +<%= javascript_packs_with_chunks_tag "admin/config/smtp", defer: true %> + <%= form_for(Settings::SMTP.new, url: admin_settings_smtp_settings_path, - html: { data: { action: "submit->config#updateConfigurationSettings" } }) do |f| %> + html: { data: { + action: "submit->config#updateConfigurationSettings", + testid: "emailServerSettings" + } }) do |f| %>
<%= render partial: "admin/shared/card_header", - locals: { header: "SMTP Settings", state: "collapse", target: "smtpSettingsBodyContainer", expanded: false } %> + locals: { header: "Email Server Settings (SMTP)", state: "collapse", target: "smtpSettingsBodyContainer", expanded: false } %>
- <% Settings::SMTP::OPTIONS.each do |config_key| %> -
- <%= admin_config_label config_key %> - <%= admin_config_description Constants::Settings::SMTP::DETAILS[config_key][:description] %> - <%= f.text_field config_key, - class: "crayons-textfield", - value: Settings::SMTP.public_send(config_key), - placeholder: Constants::Settings::SMTP::DETAILS[config_key][:placeholder] %> + <% if ForemInstance.sendgrid_enabled? %> +
+ <%= f.check_box :own_email_server, + checked: Settings::SMTP.provided_minimum_settings?, + class: "crayons-checkbox" %> + <%= f.label :own_email_server, class: "crayons-field__label" do %> + Use my own email server + <% end %>
<% end %> + + <% if ForemInstance.only_sendgrid_enabled? %> +

+ As a Forem Cloud client, we provide an email server managed by the Forem team. All settings are managed by us and the from and reply email addresses are set as <%= ForemInstance.email %>. However, you can override this to use your own email server. +

+ <% end %> + +
"> + <% Settings::SMTP::OPTIONS.each do |config_key| %> +
+ <%= admin_config_label config_key, model: Settings::SMTP %> + <%= admin_config_description Constants::Settings::SMTP::DETAILS[config_key][:description] %> + <%= f.text_field config_key, + class: "crayons-textfield", + value: Settings::SMTP.public_send(config_key), + placeholder: Constants::Settings::SMTP::DETAILS[config_key][:placeholder], + "data-config-target": "smtpSetting#{config_key.to_s.camelize(:upper)}" %> +
+ <% end %> +
<%= render "update_setting_button", f: f %>
diff --git a/app/views/admin/settings/show.html.erb b/app/views/admin/settings/show.html.erb index 487ec1538..cd597b625 100644 --- a/app/views/admin/settings/show.html.erb +++ b/app/views/admin/settings/show.html.erb @@ -34,6 +34,7 @@ <%= render partial: "forms/community" %> <%= render partial: "forms/credits" %> <%= render partial: "forms/emails" %> + <%= render partial: "forms/smtp" %> <%= render partial: "forms/google_analytics" %> <%= render partial: "forms/images", locals: { logo_allowed_types: @logo_allowed_types, logo_max_file_size: @logo_max_file_size } %> <%= render partial: "forms/mascot" %> @@ -42,7 +43,6 @@ <%= render partial: "forms/newsletter" %> <%= render partial: "forms/onboarding" %> <%= render partial: "forms/rate_limit" %> - <%= render partial: "forms/smtp" %> <%= render partial: "forms/sponsors" %> <%= render partial: "forms/tags" %> <%= render partial: "forms/user_experience" %> diff --git a/cypress/integration/seededFlows/adminFlows/config/emailServerSettingsSection.spec.js b/cypress/integration/seededFlows/adminFlows/config/emailServerSettingsSection.spec.js new file mode 100644 index 000000000..b7afde51d --- /dev/null +++ b/cypress/integration/seededFlows/adminFlows/config/emailServerSettingsSection.spec.js @@ -0,0 +1,36 @@ +describe('Email Server Settings Section', () => { + beforeEach(() => { + cy.testSetup(); + cy.fixture('users/adminUser.json').as('user'); + + cy.get('@user').then((user) => { + cy.loginAndVisit(user, '/admin/customization/config'); + }); + }); + + describe('email server settings', () => { + it('updates the smtp fields', () => { + cy.findByTestId('emailServerSettings').as('emailServerSettings'); + + cy.get('@emailServerSettings') + .findByText('Email Server Settings (SMTP)') + .click(); + + cy.get('@emailServerSettings').within(() => { + cy.findByText('Email Server Settings (SMTP)').click(); + cy.findByLabelText('User name').clear().type('jane_doe'); + cy.findByLabelText('Password').clear().type('abc123456'); + cy.findByLabelText('Address').clear().type('smtp.gmail.com'); + cy.findByLabelText('Authentication').clear().type('plain'); + cy.findByText('Update Settings').click(); + }); + + cy.url().should('contains', '/admin/customization/config'); + cy.findByText('Successfully updated settings.').should('be.visible'); + cy.findByLabelText('User name').should('have.value', 'jane_doe'); + cy.findByLabelText('Password').should('have.value', 'abc123456'); + cy.findByLabelText('Address').should('have.value', 'smtp.gmail.com'); + cy.findByLabelText('Authentication').should('have.value', 'plain'); + }); + }); +}); diff --git a/cypress/integration/seededFlows/adminFlows/pages/landingPage.spec.js b/cypress/integration/seededFlows/adminFlows/pages/landingPage.spec.js index 7bf702b1b..8a184bd21 100644 --- a/cypress/integration/seededFlows/adminFlows/pages/landingPage.spec.js +++ b/cypress/integration/seededFlows/adminFlows/pages/landingPage.spec.js @@ -112,8 +112,7 @@ describe('Set a landing page from the admin portal', () => { cy.findByRole('main').within(() => { cy.findByRole('checkbox', { name: "Use as 'Locked Screen' Determines if this page will be used as a landing page for anonymous viewers.", - }) - .check(); + }).check(); cy.findAllByRole('button', { name: 'Update Page' }).first().click(); }); @@ -139,8 +138,7 @@ describe('Set a landing page from the admin portal', () => { cy.findByRole('main').within(() => { cy.findByRole('checkbox', { name: "Use as 'Locked Screen' Determines if this page will be used as a landing page for anonymous viewers.", - }) - .check(); + }).check(); cy.findAllByRole('button', { name: 'Cancel' }).first().click(); diff --git a/spec/models/forem_instance_spec.rb b/spec/models/forem_instance_spec.rb index c368a497e..444a0053e 100644 --- a/spec/models/forem_instance_spec.rb +++ b/spec/models/forem_instance_spec.rb @@ -108,4 +108,29 @@ RSpec.describe ForemInstance, type: :model do expect(described_class.contact_email).to be(email) end end + + describe ".only_sendgrid_enabled?" do + it "returns false when the minimum SMTP settings are provided" do + allow(Settings::SMTP).to receive(:user_name).and_return("something") + allow(Settings::SMTP).to receive(:password).and_return("something") + allow(Settings::SMTP).to receive(:address).and_return("something") + + expect(described_class.only_sendgrid_enabled?).to be(false) + end + + it "returns false when Sendgrid is not enabled" do + allow(described_class).to receive(:sendgrid_enabled?).and_return(false) + expect(described_class.only_sendgrid_enabled?).to be(false) + end + + it "returns true if Sendgrid is enabled and the minimum SMTP settings are not provided" do + allow(described_class).to receive(:sendgrid_enabled?).and_return(true) + + allow(Settings::SMTP).to receive(:user_name).and_return(nil) + allow(Settings::SMTP).to receive(:password).and_return(nil) + allow(Settings::SMTP).to receive(:address).and_return(nil) + + expect(described_class.only_sendgrid_enabled?).to be(true) + end + end end diff --git a/spec/system/admin/config/admin_updates_smtp_settings_spec.rb b/spec/system/admin/config/admin_updates_smtp_settings_spec.rb new file mode 100644 index 000000000..556620301 --- /dev/null +++ b/spec/system/admin/config/admin_updates_smtp_settings_spec.rb @@ -0,0 +1,111 @@ +require "rails_helper" + +RSpec.describe "Admin updates SMTP Settings", type: :system do + let(:admin) { create(:user, :super_admin) } + + before do + sign_in admin + end + + # We're unable to set and unset an ENV variable in Cypress to test different scenarios + # hence, we test the view layouts with Capybara, and we test successful updates with Cypress. + context "when Sendgrid is not enabled and SMTP is not enabled" do + before do + allow(ForemInstance).to receive(:sendgrid_enabled?).and_return(false) + allow(Settings::SMTP).to receive(:address).and_return(nil) + allow(Settings::SMTP).to receive(:user_name).and_return(nil) + allow(Settings::SMTP).to receive(:password).and_return(nil) + visit admin_config_path + end + + it "does not show the 'Use my own email server' checkbox" do + within("form[data-testid='emailServerSettings']") do + expect(page).not_to have_content("Use my own email server") + end + end + + it "shows the SMTP Form", :aggregate_failures do + within("form[data-testid='emailServerSettings']") do + expect(page).to have_selector(".js-custom-smtp-section") + expect(page).not_to have_selector(".js-custom-smtp-section.hidden") + end + end + end + + context "when Sendgrid is enabled and SMTP is not enabled" do + before do + allow(ForemInstance).to receive(:sendgrid_enabled?).and_return(true) + allow(ForemInstance).to receive(:email).and_return("yo@forem.com") + allow(Settings::SMTP).to receive(:address).and_return(nil) + allow(Settings::SMTP).to receive(:user_name).and_return(nil) + allow(Settings::SMTP).to receive(:password).and_return(nil) + visit admin_config_path + end + + it "shows the checkbox to allow one to toggle ones own server" do + within("form[data-testid='emailServerSettings']") do + expect(page).to have_content("Use my own email server") + end + end + + it "shows a description" do + within("form[data-testid='emailServerSettings']") do + # rubocop:disable Layout/LineLength + expect(page).to have_content("As a Forem Cloud client, we provide an email server managed by the Forem team. All settings are managed by us and the from and reply email addresses are set as yo@forem.com. However, you can override this to use your own email server.") + # rubocop:enable Layout/LineLength + end + end + + it "does not show an SMTP Form" do + within("form[data-testid='emailServerSettings']") do + expect(page).to have_selector(".js-custom-smtp-section.hidden") + end + end + end + + context "when Sendgrid is not enabled and SMTP is enabled" do + before do + allow(ForemInstance).to receive(:sendgrid_enabled?).and_return(false) + allow(Settings::SMTP).to receive(:address).and_return("smtp.gmail.com") + allow(Settings::SMTP).to receive(:user_name).and_return("jane_doe") + allow(Settings::SMTP).to receive(:password).and_return("abc123456") + visit admin_config_path + end + + it "does not show the 'Use my own email server' checkbox" do + within("form[data-testid='emailServerSettings']") do + expect(page).not_to have_content("Use my own email server") + end + end + + it "shows the SMTP Form", :aggregate_failures do + within("form[data-testid='emailServerSettings']") do + expect(page).to have_selector(".js-custom-smtp-section") + expect(page).not_to have_selector(".js-custom-smtp-section.hidden") + end + end + end + + context "when Sendgrid is enabled and SMTP is enabled" do + before do + allow(ForemInstance).to receive(:sendgrid_enabled?).and_return(true) + allow(Settings::SMTP).to receive(:address).and_return("smtp.gmail.com") + allow(Settings::SMTP).to receive(:user_name).and_return("jane_doe") + allow(Settings::SMTP).to receive(:password).and_return("abc123456") + visit admin_config_path + end + + it "shows the 'Use my own email server' checkbox" do + within("form[data-testid='emailServerSettings']") do + expect(page).to have_content("Use my own email server") + end + end + + it "shows an SMTP Form", :aggregate_failures do + within("form[data-testid='emailServerSettings']") do + expect(page).to have_selector(".js-custom-smtp-section") + expect(page).not_to have_selector(".js-custom-smtp-section.hidden") + end + end + end +end