Unpublish post service class + V1 API endpoint (#18031)

* Unpublish post service + API

* Apply PR feedback

* Use module_function on service class

app/assets/stylesheets/views/profile.scss

@@ -76,7 +76,7 @@
       max-width: 100%;
       overflow: hidden;
       text-overflow: ellipsis;
-      
+
       & > span {
         overflow: hidden;
         text-overflow: ellipsis;

app/controllers/api/v1/api_controller.rb

@@ -35,10 +35,6 @@ module Api
       # @note This method is performing both authentication and authorization.  The user suspended
       #       should be something added to the corresponding pundit policy.
       def authenticate!
-        # FeatureFlag endpoints don't require authentication because they're
-        # only used in the test environment (Cypress test FeatureFlag toggle)
-        return true if params[:controller]&.match?(%r{^api/v1/(feature_flags|instances|profile_images)$})
-
         @user = authenticate_with_api_key
         return error_unauthorized unless @user
         return error_unauthorized if @user.suspended?

app/controllers/api/v1/instances_controller.rb

@@ -3,7 +3,6 @@ module Api
     class InstancesController < ApiController
       include Api::InstancesController
 
-      before_action :authenticate!
       before_action :set_no_cache_header
     end
   end

app/controllers/articles_controller.rb

@@ -211,15 +211,8 @@ class ArticlesController < ApplicationController
 
   def admin_unpublish
     authorize @article
-    attributes = {}
-    if @article.has_frontmatter?
-      body_markdown = @article.body_markdown.sub(/\npublished:\s*true\s*\n/, "\npublished: false\n")
-      attributes[:body_markdown] = body_markdown
-    else
-      attributes[:published] = false
-    end
 
-    result = Articles::Updater.call(current_user, @article, attributes)
+    result = Articles::Unpublish.call(current_user, @article)
 
     if result.success
       render json: { message: "success", path: @article.current_state_path }, status: :ok

app/controllers/concerns/api/articles_controller.rb

@@ -103,6 +103,18 @@ module Api
         .decorate
     end
 
+    def unpublish
+      @article = Article.find(params[:id])
+
+      authorize @article, :revoke_publication?
+
+      if Articles::Unpublish.call(@user, @article)
+        render head: :ok
+      else
+        render json: { message: @article.errors.full_messages }, status: :unprocessable_entity
+      end
+    end
+
     private
 
     def article_params

app/services/articles/unpublish.rb

@@ -0,0 +1,17 @@
+module Articles
+  module Unpublish
+    module_function
+
+    def call(user, article)
+      attributes = {}
+      if article.has_frontmatter?
+        body_markdown = article.body_markdown.sub(/\npublished:\s*true\s*\n/, "\npublished: false\n")
+        attributes[:body_markdown] = body_markdown
+      else
+        attributes[:published] = false
+      end
+
+      Articles::Updater.call(user, article, attributes)
+    end
+  end
+end

app/views/api/v1/articles/unpublish.json.jbuilder

@@ -0,0 +1 @@
+# This endpoint returns an empty 200 response

config/routes.rb

@@ -45,6 +45,9 @@ Rails.application.routes.draw do
       # API V1 is in pre-release: Available iff api_v1 FeatureFlag is enabled
       constraints(->(_req) { FeatureFlag.enabled?(:api_v1) }) do
         scope module: :v1, constraints: ApiConstraints.new(version: 1, default: false) do
+          # V1 only endpoints
+          put "/articles/:id/unpublish", to: "articles#unpublish", as: :article_unpublish
+
           draw :api
         end
       end

spec/requests/api/v1/articles_spec.rb

@@ -6,7 +6,13 @@ RSpec.describe "Api::V1::Articles", type: :request do
   let(:article) { create(:article, featured: true, tags: "discuss") }
   let(:new_article) { create(:article) }
   let(:api_secret) { create(:api_secret) }
-  let(:v1_headers) { { "Accept" => "application/vnd.forem.api-v1+json", "api-key" => api_secret.secret } }
+  let(:v1_headers) do
+    {
+      "content-type" => "application/json",
+      "Accept" => "application/vnd.forem.api-v1+json",
+      "api-key" => api_secret.secret
+    }
+  end
 
   before do
     stub_const("FlareTag::FLARE_TAG_IDS_HASH", { "discuss" => tag.id })
@@ -883,7 +889,7 @@ RSpec.describe "Api::V1::Articles", type: :request do
         expect do
           post api_articles_path, params: {}.to_json, headers: post_headers
         end.not_to raise_error
-        expect(response.status).to eq(422)
+        expect(response).to have_http_status(:unprocessable_entity)
       end
 
       it "fails with a nil body markdown" do
@@ -1177,4 +1183,51 @@ RSpec.describe "Api::V1::Articles", type: :request do
       end
     end
   end
+
+  describe "PUT /api/articles/:id/unpublish" do
+    let(:user) { api_secret.user }
+    let!(:published_article) { create(:article, published: true) }
+    let(:path) { api_article_unpublish_path(published_article.id) }
+
+    context "when unauthorized" do
+      it "fails with no api key" do
+        put path, headers: { "content-type" => "application/json", "Accept" => "application/vnd.forem.api-v1+json" }
+        expect(response).to have_http_status(:unauthorized)
+      end
+
+      it "fails with the wrong api key" do
+        put path,
+            headers: { "api-key" => "foobar", "content-type" => "application/json",
+                       "Accept" => "application/vnd.forem.api-v1+json" }
+        expect(response).to have_http_status(:unauthorized)
+      end
+
+      it "fails without elevated_user?" do
+        put path, headers: v1_headers
+        expect(response).to have_http_status(:unauthorized)
+      end
+    end
+
+    context "when authorized as moderator" do
+      before { user.add_role(:moderator) }
+
+      it "unpublishes an article" do
+        expect(published_article.published).to be true
+        put path, headers: v1_headers
+        expect(response).to have_http_status(:ok)
+        expect(published_article.reload.published).to be false
+      end
+    end
+
+    context "when authorized as super_admin" do
+      before { user.add_role(:super_admin) }
+
+      it "unpublishes an article" do
+        expect(published_article.published).to be true
+        put path, headers: v1_headers
+        expect(response).to have_http_status(:ok)
+        expect(published_article.reload.published).to be false
+      end
+    end
+  end
 end

spec/services/articles/unpublish_spec.rb

@@ -0,0 +1,24 @@
+require "rails_helper"
+
+RSpec.describe Articles::Unpublish, type: :service do
+  let(:user) { create(:user) }
+  let(:article) { create(:article, published: true) }
+  let(:frontmatter) { "---\ntitle: Hellohnnnn#{rand(1000)}\npublished: true\ntags: hiring\n---\n\nHello" }
+  let(:frontmatter_article) { create(:article, body_markdown: frontmatter) }
+
+  before { article.update(body_markdown: Faker::Hipster.paragraph(sentence_count: 2)) }
+
+  it "unpublishes article without frontmatter" do
+    expect(article.has_frontmatter?).to be(false)
+    expect do
+      described_class.call(user, article)
+    end.to change(article, :published?).from(true).to(false)
+  end
+
+  it "unpublishes article with frontmatter" do
+    expect(frontmatter_article.has_frontmatter?).to be(true)
+    expect do
+      described_class.call(user, frontmatter_article)
+    end.to change(frontmatter_article, :published?).from(true).to(false)
+  end
+end