From 07983e38be832a168e8e40e82acefa94dcfcdab0 Mon Sep 17 00:00:00 2001
From: Jeremy Friesen <jeremy.n.friesen@gmail.com>
Date: Thu, 7 Apr 2022 09:11:37 -0400
Subject: [PATCH] Adjusting conditional create post rendering (#17132)

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

This reverts commit 1cb45995cb5e9580cc30efcb6ebdc8667cbd6a59.

* 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>
---
 app/assets/javascripts/base.js.erb            |  2 -
 .../authorization/articles_controller.rb      | 16 -------
 .../articles/create_post_button.html.erb      |  3 --
 app/views/layouts/_top_bar.html.erb           |  7 +--
 config/routes.rb                              |  6 ---
 .../limitPostCreationToAdmins.spec.js         |  9 ++--
 package.json                                  |  1 -
 spec/requests/authorization/articles_spec.rb  | 47 -------------------
 spec/requests/root_page_spec.rb               | 31 ------------
 yarn.lock                                     |  5 --
 10 files changed, 5 insertions(+), 122 deletions(-)
 delete mode 100644 app/controllers/authorization/articles_controller.rb
 delete mode 100644 app/views/authorization/articles/create_post_button.html.erb
 delete mode 100644 spec/requests/authorization/articles_spec.rb
 delete mode 100644 spec/requests/root_page_spec.rb

diff --git a/app/assets/javascripts/base.js.erb b/app/assets/javascripts/base.js.erb
index 88c9eaf37..4c6fd0c24 100644
--- a/app/assets/javascripts/base.js.erb
+++ b/app/assets/javascripts/base.js.erb
@@ -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);
diff --git a/app/controllers/authorization/articles_controller.rb b/app/controllers/authorization/articles_controller.rb
deleted file mode 100644
index 6a4f6ab47..000000000
--- a/app/controllers/authorization/articles_controller.rb
+++ /dev/null
@@ -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
diff --git a/app/views/authorization/articles/create_post_button.html.erb b/app/views/authorization/articles/create_post_button.html.erb
deleted file mode 100644
index 1092a72d3..000000000
--- a/app/views/authorization/articles/create_post_button.html.erb
+++ /dev/null
@@ -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>
diff --git a/app/views/layouts/_top_bar.html.erb b/app/views/layouts/_top_bar.html.erb
index a5b5a48b2..b0bd64019 100644
--- a/app/views/layouts/_top_bar.html.erb
+++ b/app/views/layouts/_top_bar.html.erb
@@ -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">
diff --git a/config/routes.rb b/config/routes.rb
index 793be60af..a8c3eabe7 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -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]
diff --git a/cypress/integration/seededFlows/policyFlows/limitPostCreationToAdmins.spec.js b/cypress/integration/seededFlows/policyFlows/limitPostCreationToAdmins.spec.js
index 74ac1c235..918f03278 100644
--- a/cypress/integration/seededFlows/policyFlows/limitPostCreationToAdmins.spec.js
+++ b/cypress/integration/seededFlows/policyFlows/limitPostCreationToAdmins.spec.js
@@ -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');
   });
 });
diff --git a/package.json b/package.json
index 21a1d5bda..e5e1ba6a4 100644
--- a/package.json
+++ b/package.json
@@ -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",
diff --git a/spec/requests/authorization/articles_spec.rb b/spec/requests/authorization/articles_spec.rb
deleted file mode 100644
index 22cda8d48..000000000
--- a/spec/requests/authorization/articles_spec.rb
+++ /dev/null
@@ -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
diff --git a/spec/requests/root_page_spec.rb b/spec/requests/root_page_spec.rb
deleted file mode 100644
index 360563801..000000000
--- a/spec/requests/root_page_spec.rb
+++ /dev/null
@@ -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
diff --git a/yarn.lock b/yarn.lock
index 07d677aed..bd52bd780 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -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"