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

Adjusting conditional create post rendering (#17132)

Browse source 
* Revert "conditionally render create post button for admins #16490 (#16606)"

This reverts commit 1cb45995cb.

* Adding conditional create post rendering

This pull request does two things:

- Reverts the forem/forem#16606
- Replaces the conditional rendering with the approach from forem/forem#17076

What to consider:

- How does this impact Cumulative Layout Shift (CLS).  Prior to this
  commit, we didn't "flicker" the "Create Post" button into view if the
  site didn't have the conditional.
- In reverting forem/forem#16606 we remove a dependency on Turbo.
- We reduce one network call, relying instead on the async_info to carry
  the visible/hidden logic.

If the "flicker" is a problem we could add a conditional in the page for
if the site has enabled that feature.  However, that's a place holder as
we're looking at possible other reasons for allowing/disabling the
create a post.

This is a non-blocking PR, as in we can merge or not merge this and
proceed with what we have.  Instead the main goal is to "unify" our approach.

* Update cypress/integration/seededFlows/policyFlows/limitPostCreationToAdmins.spec.js

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

* Leveraging change from forem/forem#17143

Co-authored-by: Suzanne Aitchison <suzanne@forem.com>
This commit is contained in:
Jeremy Friesen 2022-04-07 09:11:37 -04:00 • committed by GitHub
parent 674a971887
commit 07983e38be
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 5 additions and 122 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
app/assets/javascripts/base.js.erb
View file

@ -4,11 +4,9 @@
//= require initializePage
//= require utilities/getImageForLink
//= require @honeybadger-io/js/dist/browser/honeybadger.js
//= require @hotwired/turbo
//= require i18n
//= require ahoy.js/dist/ahoy.js
Turbo.session.drive = false
I18n.defaultLocale = 'en';
I18n.locale = document.body.dataset.locale;
I18n.translations = JSON.parse(document.getElementById('i18n-translations').dataset.translations);

16
app/controllers/authorization/articles_controller.rb
View file

@ -1,16 +0,0 @@
module Authorization
class ArticlesController < ApplicationController
include ApplicationHelper
layout false
before_action :authenticate_user!
after_action :verify_authorized
def create_post_button
authorize Article, :create?
unless FeatureFlag.enabled?(:limit_post_creation_to_admins)
render file: "public/404.html", status: :not_found
end
end
end
end

3
app/views/authorization/articles/create_post_button.html.erb
View file

@ -1,3 +0,0 @@
<turbo-frame id="authorization-articles-create-post-button">
<a href="<%= new_path %>" class="c-cta c-cta--branded mr-2 whitespace-nowrap"><%= t("views.main.create_post") %></a>
</turbo-frame>

7
app/views/layouts/_top_bar.html.erb
View file

@ -29,11 +29,8 @@
</a>
<% if user_signed_in? %>
<span class="hidden m:flex ml-auto">
<% if FeatureFlag.enabled?(:limit_post_creation_to_admins) %>
<turbo-frame id="authorization-articles-create-post-button" src="<%= authorization_create_post_button_path %>">
</turbo-frame>
<% else %>
<a href="<%= new_path %>" class="c-cta c-cta--branded mr-2 whitespace-nowrap"><%= t("views.main.create_post") %></a>
<%= application_policy_content_tag("a", record: Article, query: :create?, feature_flag: :limit_post_creation_to_admins, href: new_path, class: "c-cta c-cta--branded mr-2 whitespace-nowrap") do %>
<%= t("views.main.create_post") %>
<% end %>
<span class="trusted-visible-block">

6
config/routes.rb
View file

@ -33,12 +33,6 @@ Rails.application.routes.draw do
draw :listing
end
namespace :authorization do
scope :articles do
get "create_post_button", controller: "articles"
end
end
namespace :stories, defaults: { format: "json" } do
resource :feed, only: [:show] do
resource :pinned_article, only: %w[show update destroy]

9
cypress/integration/seededFlows/policyFlows/limitPostCreationToAdmins.spec.js
View file

@ -1,7 +1,7 @@
/**
This is to verify visibility of buttons. The authorization tests are
asserted in their corresponding policy tests.
*/
*/
describe('Limit Post Creation to Admins', () => {
beforeEach(() => {
cy.testSetup();
@ -13,11 +13,9 @@ describe('Limit Post Creation to Admins', () => {
});
it('clicking on User Avatar should open User Dropdown menu and no Create Post is visible', () => {
// TODO: If we go with the approach in this commit, then we should test the
// "Create Button" that's on the nav bar but not in the drop down.
cy.findByRole('button', { name: 'Navigation menu' }).as('menuButton');
cy.get('@menuButton')
cy.get('@menuButton')
.should('have.attr', 'aria-expanded', 'false')
.click()
.should('have.attr', 'aria-expanded', 'true');
@ -26,8 +24,7 @@ describe('Limit Post Creation to Admins', () => {
name: 'Article Editor v1 User @article_editor_v1_user',
}).should('exist');
cy.findByRole('link', { name: 'Create Post' }).should('not.exist');
// The "Create Post" button either outside of the drop-down nor inside
cy.findByRole('link', { name: 'Create Post' }).should('not.exist');
});
});

1
package.json
View file

@ -109,7 +109,6 @@
"@honeybadger-io/webpack": "^1.5.1",
"@hotwired/stimulus": "3.0.1",
"@hotwired/stimulus-webpack-helpers": "1.0.1",
"@hotwired/turbo": "^7.1.0",
"@rails/ujs": "7.0.2",
"@rails/webpacker": "5.4.3",
"@reach/combobox": "^0.16.5",

47
spec/requests/authorization/articles_spec.rb
View file

@ -1,47 +0,0 @@
require "rails_helper"
RSpec.describe "Articles", type: :request do
let(:user) { create(:user) }
let(:super_admin) { create(:user, :super_admin) }
describe "GET /create_post_button" do
context "when limit post creation to admins is enabled" do
before do
FeatureFlag.add("limit_post_creation_to_admins")
FeatureFlag.enable("limit_post_creation_to_admins")
end
after { FeatureFlag.remove("limit_post_creation_to_admins") }
it "returns the Create Post anchor tag for admins" do
sign_in super_admin
get authorization_create_post_button_path
expect(response.body).to include("Create Post")
end
it "raises Pundit::NotAuthorizedError for non admins" do
sign_in user
expect { get authorization_create_post_button_path }.to raise_error(Pundit::NotAuthorizedError)
end
end
context "when limit post creation to admins is disabled" do
before do
FeatureFlag.add("limit_post_creation_to_admins")
FeatureFlag.disable("limit_post_creation_to_admins")
end
after { FeatureFlag.remove("limit_post_creation_to_admins") }
it "returns a 404", :aggregate_failures do
sign_in user
get authorization_create_post_button_path
expect(response).to have_http_status(:not_found)
sign_in super_admin
get authorization_create_post_button_path
expect(response).to have_http_status(:not_found)
end
end
end
end

31
spec/requests/root_page_spec.rb
View file

@ -1,31 +0,0 @@
require "rails_helper"
RSpec.describe "Homepage", type: :request do
let(:user) { create(:user) }
describe "GET /" do
context "when limit post creation to admins" do
before { FeatureFlag.add("limit_post_creation_to_admins") }
after { FeatureFlag.remove("limit_post_creation_to_admins") }
it "does not render turbo frame when disabled" do
FeatureFlag.disable("limit_post_creation_to_admins")
sign_in user
get root_path
expect(response.body).not_to include("turbo-frame")
end
it "renders turbo frame when enabled" do
FeatureFlag.enable("limit_post_creation_to_admins")
sign_in user
get root_path
expect(response.body).to include("turbo-frame")
end
end
end
end

5
yarn.lock
View file

@ -1527,11 +1527,6 @@
resolved "https://registry.yarnpkg.com/@hotwired/stimulus/-/stimulus-3.0.1.tgz#141f15645acaa3b133b7c247cad58ae252ffae85"
integrity sha512-oHsJhgY2cip+K2ED7vKUNd2P+BEswVhrCYcJ802DSsblJFv7mPFVk3cQKvm2vHgHeDVdnj7oOKrBbzp1u8D+KA==
"@hotwired/turbo@^7.1.0":
version "7.1.0"
resolved "https://registry.yarnpkg.com/@hotwired/turbo/-/turbo-7.1.0.tgz#27e44e0e3dc5bd1d4bda0766d579cf5a14091cd7"
integrity sha512-Q8kGjqwPqER+CtpQudbH+3Zgs2X4zb6pBAlr6NsKTXadg45pAOvxI9i4QpuHbwSzR2+x87HUm+rot9F/Pe8rxA==
"@humanwhocodes/config-array@^0.9.2":
version "0.9.2"
resolved "https://registry.yarnpkg.com/@humanwhocodes/config-array/-/config-array-0.9.2.tgz#68be55c737023009dfc5fe245d51181bb6476914"