RFC #74, Pt. 2: Private Forem Landing Page (#13819)

* Adds landing_page checkbox for Landing Page

* Adds landing_page to page_params in Admin::Pages::Controller

* Adds landing_page to admin_manages_pages and admin_creates_pages specs

* Adjusts copy for Landing Page in pages/_form.html.erb

* Places #post_a_job before #privacy in Pages::Controller
  - Ensures that all page-related methods are properly
 alphabetized

* Alphabetizes remaining out-of-order page-related methods
  - Ensures that all page-related methods within the
 Pages::Controller are in alphabetical order, following
 the order of the other methods

* Adds landing_page check to ApplicationController#verify_private_forem

* Adds a single_landing_page validation to the Page model

* Adds a lock.svg to /images and uses it in the pages index

* Adjusts buttons and checkboxes to use crayons class in pages partial

* Ensures that pages/_form.html.erb destroy button uses crayons

* Adds a landing_page validation-related test to page_spec.rb

* Updates #single_landing_page validation and error message

* WIP: Adds JS modal upon landing_page error in pages/_form.html.erb

* Adds a working modal upon error in pages/_form.html.erb
- Removes redundant #single_landing_page validation from Page
- Adds #has_a_landing_page? in place of #single_landing_page
- Conditionally renders the landing_page checkbox in _form.html.erb
- Adds styles to locked screen/landing page modal
- Adds copy to locked screen/landing page modal
- Beings adding buttons to overwrite current landing_page

* Updates the data-modal-selectors and ids in pages/_form.html.erb

* Uses #landing_page_path within pages/_form.html.erb modal
- Adds a link_to to link to the current landing page in the modal
- Adds styles to buttons and link_to in modal

* Adds back single_landing_page validation to Page.rb

* Reverts changes and updates methods within Page.rb
 - Reverts changes to #single_landing_page
 - Refactors #has_a_landing_page? and #landing_page_path
 - Updates div id in pages/_form.html.erb

* Adjusts #single_landing_page error message slightly

* Adds a test for landing_pages in stories_index_spec.rb

* Adds an overwrite attribute to Pages for the modal in _form
  - Fixes anonymous spelling

* Refactors pages/_form.html.erb and cleans up spelling

* Adds a safe operator to landing_page.path in Page
  - Adjusts the placement of the hidden value in pages/_form.html.erb

* Updates styling for Current Locked Screen link

* Adds the ability to overwrite an active landing page on click
  - Adds Admin::PagesController#update_and_overwrite and refactors
AdminPages::Controller#udpate to work with new method
  - Adds a remote attribute, overwrite, to Admin::PagesController
via a hidden_field in pages/_form.html.erb and an attr_accessor in
the Page model
  - Adds a toggleOverwrite() function to pages/_form.html.erb to
toggle the (soon to be hidden) overwrite checkbox in the form --
the checkbox is toggled on button click

* Adds a guard clause to #landing_page_path in Page.rb

* Adds a safe naviagtor to Admin::PagesController#update_and_overwrite_landing_page

* Adds a type and return to Overwrite button to prevent submit

* Adds a hidden class to the overwrite checkbox field in _form.html.erb
  - Hides the overwrite checkbox since it is only used to overwrite
the landing_page boolean on click of the "Overwrite current locked
screen" button

* Adds Admin::PagesController#create_and_overwrite and removes hidden class

* Adds landing page-related tests to admin_manages_pages_spec.rb

* Remove messy commented out code and place it in GH comment instead

* PR Review Feedback: Renames remote attr, defines class methods, updates views
  - Renames remote attribute to overwrite_landing_page
  - Updates all necessary views, methods, etc. w/ new attr
  - Defines two class methods to make code more extensible
  - Updates all necessary views, methods, etc. w/ new methods

* Remove unnecessary comments and revert changes to ternary in _form.html.erb

* Adjustments for accessibility within _form and index
  - Adds a descriptive id to <p> in _form.html.erb
  - Adds an ariadescribedby field to _form.html.erb
  - Adjusts title within index.html.erb

* Refactors landing_page-related code per PR review requests
  - Adds a landing_page_modal_controller.js that handles
and further customizes the dismiss and overwrite button, and
handles opening the modal
  - Adds a _landing_page_modal partial to extract modal-specific
html outside of the _form.html.erb partial
  - Refactors _form.html.erb to account for the _landing_page_modal
partial and the landing_page_modal_controller.js
  - Refactors the overwrite checkbox to be a hidden_field rather than
a hidden checkbox and removes existing hidden_field to accomodate it
  - Loads the @landing_page within Admin::PagesController#edit for
use by the _landing_page_modal.html.erb partial
  - Refactors Page#single_landing_page to be more readable
  - Removes useless JS from _form.html.erb

* Removes #landing_page_exists? class method since it isnt used now

* Uses the #landing_page class method within ApplicationController

* Removes pending status from test in admin_manages_pages_spec.rb

* Adds an e2e test for landing pages and adds to seeds_e2e
  - Adds a landingPage.spec.js to test landing pages
  - Adds a Page to seeds_e2e.rb for landingPage.spec.js

* Removes references to articles from e2e test and adds clickable link

* Adds another Page to seeds_e2e.rb and updates link names in test

* Adds the Current Locked Screen title to modal and opens in new link

* Renames Dismiss to Cancel and updates all necessary code

* Adjusts landingPage.spec.js

* check for existing landing page before showing modal

* Update Cypress tests and fix bug in LandingPageModalController

* Apply suggestions from code review

Co-authored-by: Suzanne Aitchison <suzanne@forem.com>

* Orders Pages by created_at desc in /admin/pages/index.html.erb for consistency

* Targets specific edit button in landingPage.spec.js

Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
Co-authored-by: rhymes <github@rhymes.dev>
This commit is contained in:
Julianna Tetreault 2021-06-10 11:46:24 -06:00 committed by GitHub
parent 625e7b0170
commit b12ebfa0e5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
15 changed files with 355 additions and 27 deletions

View file

@ -0,0 +1,3 @@
<svg width="24" height="24" viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg">
<path d="M19 10h1a1 1 0 0 1 1 1v10a1 1 0 0 1-1 1H4a1 1 0 0 1-1-1V11a1 1 0 0 1 1-1h1V9a7 7 0 0 1 14 0v1zm-2 0V9A5 5 0 0 0 7 9v1h10zm-6 4v4h2v-4h-2z" fill="#08090A"/>
</svg>

After

Width:  |  Height:  |  Size: 272 B

View file

@ -3,13 +3,14 @@ module Admin
layout "admin"
def index
@pages = Page.all
@pages = Page.all.order(created_at: :desc)
@code_of_conduct = Page.find_by(slug: "code-of-conduct")
@privacy = Page.find_by(slug: "privacy")
@terms = Page.find_by(slug: "terms")
end
def new
@landing_page = Page.find_by(landing_page: true)
if (slug = params[:slug])
prepopulate_new_form(slug)
else
@ -19,13 +20,12 @@ module Admin
def edit
@page = Page.find(params[:id])
@landing_page = Page.find_by(landing_page: true)
end
def update
@page = Page.find(params[:id])
@page.assign_attributes(page_params)
if @page.valid?
@page.update!(page_params)
if update_and_overwrite_landing_page
flash[:success] = "Page has been successfully updated."
redirect_to admin_pages_path
else
@ -36,8 +36,7 @@ module Admin
def create
@page = Page.new(page_params)
if @page.valid?
@page.save!
if create_and_overwrite_landing_page
flash[:success] = "Page has been successfully created."
redirect_to admin_pages_path
else
@ -57,7 +56,7 @@ module Admin
def page_params
allowed_params = %i[title slug body_markdown body_html body_json description template is_top_level_path
social_image]
social_image landing_page overwrite_landing_page]
params.require(:page).permit(allowed_params)
end
@ -96,5 +95,31 @@ module Admin
Page.new
end
end
def update_and_overwrite_landing_page
if page_params["overwrite_landing_page"] == "true"
Page.transaction do
current_landing_page = Page.find_by(landing_page: true)
current_landing_page&.update(landing_page: false)
@page.update(page_params)
end
else
@page.update(page_params)
end
end
def create_and_overwrite_landing_page
if page_params["overwrite_landing_page"] == "true"
Page.transaction do
current_landing_page = Page.find_by(landing_page: true)
current_landing_page&.update(landing_page: false)
@page.save
end
else
@page.save
end
end
end
end

View file

@ -54,8 +54,12 @@ class ApplicationController < ActionController::Base
return if self.class.module_parent.to_s == "Admin"
return if user_signed_in? || Settings::UserExperience.public
@page = Page.landing_page
if api_action?
authenticate!
elsif @page
render template: "pages/show"
else
@user ||= User.new
render template: "devise/registrations/new"

View file

@ -22,6 +22,12 @@ class PagesController < ApplicationController
set_surrogate_key_header "about_listings_page"
end
def badge
@html_variant = HtmlVariant.find_for_test([], "badge_landing_page")
render layout: false
set_surrogate_key_header "badge_page"
end
def bounty
@page = Page.find_by(slug: "security")
render :show if @page
@ -52,22 +58,16 @@ class PagesController < ApplicationController
set_surrogate_key_header "faq_page"
end
def privacy
@page = Page.find_by(slug: "privacy")
render :show if @page
set_surrogate_key_header "privacy_page"
end
def post_a_job
@page = Page.find_by(slug: "post-a-job")
render :show if @page
set_surrogate_key_header "post_a_job_page"
end
def terms
@page = Page.find_by(slug: "terms")
def privacy
@page = Page.find_by(slug: "privacy")
render :show if @page
set_surrogate_key_header "terms_page"
set_surrogate_key_header "privacy_page"
end
def tag_moderation
@ -76,10 +76,10 @@ class PagesController < ApplicationController
set_surrogate_key_header "tag_moderation_page"
end
def badge
@html_variant = HtmlVariant.find_for_test([], "badge_landing_page")
render layout: false
set_surrogate_key_header "badge_page"
def terms
@page = Page.find_by(slug: "terms")
render :show if @page
set_surrogate_key_header "terms_page"
end
def report_abuse

View file

@ -0,0 +1,26 @@
import ModalController from './modal_controller';
export default class LandingPageModalController extends ModalController {
static targets = ['overwrite', 'landingPageCheckbox'];
openModal() {
if (this.landingPageCheckboxTarget.checked) {
this.toggleModal();
}
}
confirm(event) {
event.preventDefault();
this.overwriteTarget.value = true;
this.closeModal();
}
cancel(event) {
event.preventDefault();
this.landingPageCheckboxTarget.checked = false;
this.overwriteTarget.value = false;
this.closeModal();
}
}

View file

@ -1,4 +1,6 @@
class Page < ApplicationRecord
attr_accessor :overwrite_landing_page
TEMPLATE_OPTIONS = %w[contained full_within_layout json].freeze
validates :title, presence: true
@ -7,6 +9,7 @@ class Page < ApplicationRecord
validates :template, inclusion: { in: TEMPLATE_OPTIONS }
validate :body_present
validate :unique_slug_including_users_and_orgs, if: :slug_changed?
validate :single_landing_page, if: :will_save_change_to_landing_page?
before_validation :set_default_template
before_save :evaluate_markdown
@ -23,6 +26,10 @@ class Page < ApplicationRecord
"page_#{slug}"
end
def self.landing_page
find_by(landing_page: true)
end
private
def evaluate_markdown
@ -56,6 +63,17 @@ class Page < ApplicationRecord
errors.add(:slug, "is taken.")
end
def single_landing_page
# Only add errors if we are trying to modify a landing page
# while another landing page is already being used to ensure
# that only one can be set to "true" at a time.
landing_page = Page.where.not(id: id).find_by(landing_page: true)
return unless landing_page
errors.add(:base, "Only one page at a time can be used as a 'locked screen.'")
end
def bust_cache
Pages::BustCacheWorker.perform_async(slug)
end

View file

@ -1,4 +1,9 @@
<div class="crayons-card p-6">
<div class="crayons-card p-6"
data-controller="landing-page-modal"
data-landing-page-modal-root-selector-value="#add-landing-page-link-modal-root"
data-landing-page-modal-content-selector-value="#add-landing-page-link-modal"
data-landing-page-modal-title-value="There's another locked screen..."
data-landing-page-modal-size-value="default">
<%= form_for [:admin, @page] do |form| %>
<div class="form-group">
<%= form.label :title %>
@ -38,9 +43,31 @@
</div>
<div class="form-group">
<%= form.label :is_top_level_path %>
<%= form.check_box :is_top_level_path %>
<p>(Determines if it is accessible by <code>/page-slug</code> vs <code>/page/page-slug</code>) Be careful! ⚠️</p>
<%= form.check_box :is_top_level_path, class: "crayons-checkbox" %>
<p>Determines if it is accessible by <code>/page-slug</code> vs <code>/page/page-slug</code> Be careful! ⚠️</p>
</div>
<div class="form-group">
<%= form.label :landing_page, "Use as 'Locked Screen'" %>
<%= form.check_box :landing_page, class: "crayons-checkbox", "aria-describedby": "lock-screen-description",
data: {
"landing-page-modal-target": "landingPageCheckbox",
action: (@landing_page.present? ? "landing-page-modal#openModal" : "")
} %>
<%= form.hidden_field :overwrite_landing_page, data: { "landing-page-modal-target": "overwrite" } %>
<p id="lock-screen-description">Determines if this page will be used as a landing page for anonymous viewers.</p>
</div>
<% if @page.errors.count > 0 %>
<div id="add-landing-page-link-modal-root">
<% @page.errors %>
</div>
<% end %>
<% if @landing_page %>
<%= render partial: "landing_page_modal", locals: { page: @landing_page } %>
<% end %>
<div class="form-group">
<p>
<b><%= link_to "Feature Flag", "/admin/feature_flags" %></b>
@ -58,12 +85,12 @@
<br>
</p>
</div>
<%= form.submit class: "btn btn-primary" %>
<%= form.submit class: "crayons-btn" %>
<% end %>
<% if @page.persisted? %>
<%= form_with model: [:admin, @page], local: true, method: :delete, class: "mt-3" do |f| %>
<%= f.submit "Delete Page", class: "btn btn-danger", data: { confirm: "Are you sure?" } %>
<%= f.submit "Delete Page", class: "crayons-btn crayons-btn--danger", data: { confirm: "Are you sure?" } %>
<% end %>
<% end %>
</div>

View file

@ -0,0 +1,18 @@
<div id="add-landing-page-link-modal" class="hidden">
<div class="form-group grid p-6 mb-6 gap-1">
<p>
Only <b>one</b> page at a time can be used as a "locked screen." If you proceed, this page will no longer show as "locked screen":
<br><br>
<span style="display:block; border:none; padding:20px; background-color:#F9FAFA;">
<%= link_to "Current Locked Screen: #{page.title}", page.path, target: :_blank, rel: :noopener %>
</span>
<br>
<button type="button" class="crayons-btn" data-action="landing-page-modal#confirm">Overwrite current locked screen</button>&nbsp;
<button class="crayons-btn crayons-btn--secondary" data-action="landing-page-modal#cancel">Cancel</button>
<p>
</div>
</div>
<div id="add-landing-page-link-modal-root"></div>

View file

@ -7,7 +7,10 @@
<div class="crayons-card p-6 pages__table">
<% @pages.each do |page| %>
<div class="flex py-2 items-center">
<div class="flex py-2 items-center" data-testid="page">
<% if page.landing_page? %>
<%= inline_svg_tag("lock.svg", class: "crayons-icon", aria: true, title: "Current locked screen") %>
<% end %>
<%= link_to page.title, page.path %>
<%= link_to "Edit", edit_admin_page_path(page.id), class: "ml-auto crayons-btn crayons-btn--s crayons-btn--secondary" %>
</div>

View file

@ -0,0 +1,139 @@
describe('Set a landing page from the admin portal', () => {
beforeEach(() => {
cy.testSetup();
cy.fixture('users/adminUser.json').as('user');
cy.get('@user').then((user) => {
cy.loginUser(user).then(() => {
cy.visit('/admin/customization/pages');
});
});
});
it('should set a landing page when no other landing page exists', () => {
cy.findAllByRole('link', { name: 'Edit' }).first().click();
cy.findByRole('checkbox', { name: "Use as 'Locked Screen'" }).check();
cy.findByRole('button', { name: 'Update Page' }).click();
// Verify that the form has submitted and the page has changed to the confirmation page
cy.url().should('contain', '/admin/customization/pages');
cy.findByRole('img', { name: 'Current locked screen' }).should(
'be.visible',
);
});
it('should overwrite the landing page when choosing to set a new landing page', () => {
cy.findAllByRole('link', { name: 'Edit' }).first().click();
cy.findByRole('checkbox', { name: "Use as 'Locked Screen'" });
// Set landing page
cy.findByRole('main').within(() => {
cy.findByRole('checkbox', { name: "Use as 'Locked Screen'" }).check();
cy.findByRole('button', { name: 'Update Page' }).click();
});
cy.url().should('contain', '/admin/customization/pages');
// Retrieve the title of the landing page
let landingPageTitle;
cy.findByRole('main').within(() => {
cy.findAllByTestId('page').should((elements) => {
for (let i = elements.length - 1; i >= 0; i--) {
const el = elements[i];
const isLandingPage =
el.getElementsByClassName('crayons-icon').length > 0;
if (isLandingPage) {
landingPageTitle = el.querySelector('a').innerHTML;
}
}
});
cy.findAllByRole('link', { name: 'Edit' }).eq(1).click();
});
cy.findByRole('checkbox', { name: "Use as 'Locked Screen'" });
// Change landing page
cy.findByRole('main').within(() => {
cy.findByRole('checkbox', { name: "Use as 'Locked Screen'" }).check();
cy.findAllByRole('button', {
name: 'Overwrite current locked screen',
}).click();
cy.findByRole('button', { name: 'Update Page' }).click();
});
// Check the title of the landing page has changed
cy.findByRole('main').within(() => {
let newLandingPageTitle;
cy.findAllByTestId('page').should((elements) => {
for (let i = elements.length - 1; i >= 0; i--) {
const el = elements[i];
const isLandingPage =
el.getElementsByClassName('crayons-icon').length > 0;
if (isLandingPage) {
newLandingPageTitle = el.querySelector('a').innerHTML;
}
}
assert.notEqual(landingPageTitle, newLandingPageTitle);
});
});
});
it('should not change the landing page when clicking cancel', () => {
cy.findAllByRole('link', { name: 'Edit' }).first().click();
// Set landing page
cy.findByRole('main').within(() => {
cy.findAllByRole('checkbox', { name: "Use as 'Locked Screen'" })
.first()
.check();
cy.findAllByRole('button', { name: 'Update Page' }).first().click();
});
// Retrieve the title of the landing page
let landingPageTitle;
cy.findByRole('main').within(() => {
cy.findAllByTestId('page').should((elements) => {
for (let i = elements.length - 1; i >= 0; i--) {
const el = elements[i];
const isLandingPage =
el.getElementsByClassName('crayons-icon').length > 0;
if (isLandingPage) {
landingPageTitle = el.querySelector('a').innerHTML;
}
}
});
cy.findAllByRole('link', { name: 'Edit' }).eq(1).click();
});
// Change landing page but then Cancel
cy.findByRole('main').within(() => {
cy.findAllByRole('checkbox', { name: "Use as 'Locked Screen'" })
.first()
.check();
cy.findAllByRole('button', { name: 'Cancel' }).first().click();
cy.findAllByRole('button', { name: 'Update Page' }).first().click();
});
// Check the title of the landing page has not changed
cy.findByRole('main').within(() => {
let newLandingPageTitle;
cy.findAllByTestId('page').should((elements) => {
for (let i = elements.length - 1; i >= 0; i--) {
const el = elements[i];
const isLandingPage =
el.getElementsByClassName('crayons-icon').length > 0;
if (isLandingPage) {
newLandingPageTitle = el.querySelector('a').innerHTML;
}
}
assert.equal(landingPageTitle, newLandingPageTitle);
});
});
});
});

View file

@ -36,6 +36,13 @@ RSpec.describe Page, type: :model do
expect(page).not_to be_valid
expect(page.errors[:slug].to_s.include?("taken")).to be true
end
it "only allows a single landing_page to be set to true" do
create(:page, landing_page: true)
page = build(:page, landing_page: true)
expect(page).not_to be_valid
expect(page.errors[:base].to_s.include?("Only one page")).to be true
end
end
context "when callbacks are triggered before save" do

View file

@ -49,6 +49,14 @@ RSpec.describe "StoriesIndex", type: :request do
expect(response.body).to include("Continue with")
end
it "renders a landing page if one is active and if the site config is set to private" do
allow(Settings::UserExperience).to receive(:public).and_return(false)
create(:page, title: "This is a landing page!", landing_page: true)
get root_path
expect(response.body).to include("This is a landing page!")
end
it "renders all display_ads when published and approved" do
org = create(:organization)
ad = create(:display_ad, published: true, approved: true, organization: org)

View file

@ -230,3 +230,18 @@ seeder.create_if_none(Badge) do
rewarding_context_message_markdown: Faker::Markdown.random,
)
end
##############################################################################
seeder.create_if_none(Page) do
2.times do
Page.create!(
slug: Faker::Lorem.word,
body_html: "<p>#{Faker::Hipster.paragraph(sentence_count: 2)}</p>",
title: "#{Faker::Lorem.word} #{rand(100)}",
description: "A test page",
is_top_level_path: true,
landing_page: false,
)
end
end

View file

@ -13,6 +13,7 @@ RSpec.describe "Admin creates new page", type: :system do
expect(find_field("page[title]").value).to eq("Code of Conduct")
expect(find_field("page[slug]").value).to eq("code-of-conduct")
expect(find_field("page[is_top_level_path]").value).to eq("1")
expect(find_field("page[landing_page]").value).to eq("1")
text = "All participants of #{community_name} are expected to abide by our Code of Conduct"
expect(find_field("page[body_html]").value).to include(text)

View file

@ -9,7 +9,8 @@ RSpec.describe "Admin manages pages", type: :system do
body_html: "<div>hello there</div>",
title: "Test Page",
description: "A test page",
is_top_level_path: true)
is_top_level_path: true,
landing_page: false)
sign_in admin
visit admin_pages_path
end
@ -111,4 +112,37 @@ RSpec.describe "Admin manages pages", type: :system do
end
end
end
describe "when there is a landing page" do
let(:current_landing_page) { create(:page, landing_page: true) }
let(:new_landing_page) { create(:page, landing_page: true) }
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")
uncheck "Use as 'Locked Screen'"
click_on("Update Page")
expect(page).to have_current_path(admin_pages_path)
end
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")
check "Use as 'Locked Screen'"
expect(page).to have_link("Current Locked Screen: #{new_landing_page.title}")
click_on("Current Locked Screen")
expect(page).to have_current_path(new_landing_page.path)
expect(page).to have_content(new_landing_page.title)
end
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")
check "Use as 'Locked Screen'"
expect(page).to have_link("Current Locked Screen: #{new_landing_page.title}")
click_on("Overwrite current locked screen")
click_on("Update Page")
expect(page).to have_current_path(admin_pages_path)
end
end
end