Merge pull request #3945 from alphagov/graphql-feature-fix

Make the GraphQL overrides work consistently

app/controllers/ministers_controller.rb

@@ -2,15 +2,25 @@ class MinistersController < ApplicationController
   around_action :switch_locale
 
   def index
-    if params.include?(:graphql)
-      ministers_index = Graphql::MinistersIndex.find!(request.path)
-      content_item_data = ministers_index.content_item
-    else
-      ministers_index = MinistersIndex.find!(request.path)
-      content_item_data = ministers_index.content_item.content_item_data
-    end
+    content_item_data = if params[:graphql] == "false"
+                          load_from_content_store
+                        elsif params[:graphql] == "true" || Features.graphql_feature_enabled?
+                          load_from_graphql
+                        else
+                          load_from_content_store
+                        end
 
     @presented_ministers = MinistersIndexPresenter.new(content_item_data)
-    setup_content_item_and_navigation_helpers(ministers_index)
+    setup_content_item_and_navigation_helpers(@ministers_index)
+  end
+
+  def load_from_graphql
+    @ministers_index = Graphql::MinistersIndex.find!(request.path)
+    @ministers_index.content_item
+  end
+
+  def load_from_content_store
+    @ministers_index = MinistersIndex.find!(request.path)
+    @ministers_index.content_item.content_item_data
   end
 end

app/controllers/roles_controller.rb

@@ -2,15 +2,25 @@ class RolesController < ApplicationController
   around_action :switch_locale
 
   def show
-    if Features.graphql_feature_enabled? || params.include?(:graphql)
-      @role = Graphql::Role.find!(request.path)
-      content_item_data = @role.content_item
-    else
-      @role = Role.find!(request.path)
-      content_item_data = @role.content_item.content_item_data
-    end
+    content_item_data = if params[:graphql] == "false"
+                          load_from_content_store
+                        elsif params[:graphql] == "true" || Features.graphql_feature_enabled?
+                          load_from_graphql
+                        else
+                          load_from_content_store
+                        end
 
     setup_content_item_and_navigation_helpers(@role)
     render :show, locals: { role: RolePresenter.new(content_item_data) }
   end
+
+  def load_from_graphql
+    @role = Graphql::Role.find!(request.path)
+    @role.content_item
+  end
+
+  def load_from_content_store
+    @role = Role.find!(request.path)
+    @role.content_item.content_item_data
+  end
 end

app/controllers/world_controller.rb

@@ -1,14 +1,24 @@
 class WorldController < ApplicationController
   def index
-    if Features.graphql_feature_enabled? || params.include?(:graphql)
-      world_index = Graphql::WorldIndex.find!(request.path)
-      content_item_data = world_index.content_item
-    else
-      world_index = WorldIndex.find!(request.path)
-      content_item_data = world_index.content_item.content_item_data
-    end
+    content_item_data = if params[:graphql] == "false"
+                          load_from_content_store
+                        elsif params[:graphql] == "true" || Features.graphql_feature_enabled?
+                          load_from_graphql
+                        else
+                          load_from_content_store
+                        end
 
     @presented_index = WorldIndexPresenter.new(content_item_data)
-    setup_content_item_and_navigation_helpers(world_index)
+    setup_content_item_and_navigation_helpers(@world_index)
+  end
+
+  def load_from_graphql
+    @world_index = Graphql::WorldIndex.find!(request.path)
+    @world_index.content_item
+  end
+
+  def load_from_content_store
+    @world_index = WorldIndex.find!(request.path)
+    @world_index.content_item.content_item_data
   end
 end

spec/features/ministers_spec.rb

@@ -123,52 +123,124 @@
     end
   end
 
-  context "without the graphql feature flag" do
+  context "without the GraphQL feature flag" do
     let(:document) { GovukSchemas::Example.find("ministers_index", example_name: "ministers_index-reshuffle-mode-off") }
 
     before do
       stub_content_store_has_item("/government/ministers", document)
-      visit "/government/ministers"
     end
 
-    it_behaves_like "ministers index page"
+    context "when the GraphQL parameter is not set" do
+      before do
+        visit "/government/ministers"
+      end
+
+      it_behaves_like "ministers index page"
+
+      it "does not get the data from GraphQL" do
+        expect(a_request(:post, "#{Plek.find('publishing-api')}/graphql")).not_to have_been_made
+      end
+
+      context "during a reshuffle" do
+        let(:document) { GovukSchemas::Example.find("ministers_index", example_name: "ministers_index-reshuffle-mode-on") }
+
+        it_behaves_like "ministers index page during a reshuffle"
+      end
+
+      context "during a reshuffle preview" do
+        let(:document) { GovukSchemas::Example.find("ministers_index", example_name: "ministers_index-reshuffle-mode-on-preview") }
+
+        it_behaves_like "ministers index page during a reshuffle preview"
+      end
+    end
+
+    context "when the GraphQL parameter is true" do
+      before do
+        stub_publishing_api_graphql_query(
+          Graphql::MinistersIndexQuery.new("/government/ministers").query,
+          document,
+        )
+
+        visit "/government/ministers?graphql=true"
+      end
 
-    context "during a reshuffle" do
-      let(:document) { GovukSchemas::Example.find("ministers_index", example_name: "ministers_index-reshuffle-mode-on") }
+      let(:document) { fetch_graphql_fixture("ministers_index-reshuffle-mode-off") }
 
-      it_behaves_like "ministers index page during a reshuffle"
+      it "gets the data from GraphQL" do
+        expect(a_request(:post, "#{Plek.find('publishing-api')}/graphql")).to have_been_made
+      end
     end
 
-    context "during a reshuffle preview" do
-      let(:document) { GovukSchemas::Example.find("ministers_index", example_name: "ministers_index-reshuffle-mode-on-preview") }
+    context "when the GraphQL parameter is false" do
+      before do
+        visit "/government/ministers?graphql=false"
+      end
 
-      it_behaves_like "ministers index page during a reshuffle preview"
+      it "does not get the data from GraphQL" do
+        expect(a_request(:post, "#{Plek.find('publishing-api')}/graphql")).not_to have_been_made
+      end
     end
   end
 
-  context "with the GraphQL query param set" do
+  context "with the GraphQL feature flag" do
     let(:document) { fetch_graphql_fixture("ministers_index-reshuffle-mode-off") }
 
     before do
+      enable_graphql_feature_flag
+
       stub_publishing_api_graphql_query(
         Graphql::MinistersIndexQuery.new("/government/ministers").query,
         document,
       )
-      visit "/government/ministers?graphql=true"
     end
 
-    it_behaves_like "ministers index page"
+    context "when the GraphQL parameter is not set" do
+      before do
+        visit "/government/ministers"
+      end
+
+      it_behaves_like "ministers index page"
 
-    context "during a reshuffle" do
-      let(:document) { fetch_graphql_fixture("ministers_index-reshuffle-mode-on") }
+      it "gets the data from GraphQL" do
+        expect(a_request(:post, "#{Plek.find('publishing-api')}/graphql")).to have_been_made
+      end
 
-      it_behaves_like "ministers index page during a reshuffle"
+      context "during a reshuffle" do
+        let(:document) { fetch_graphql_fixture("ministers_index-reshuffle-mode-on") }
+
+        it_behaves_like "ministers index page during a reshuffle"
+      end
+
+      context "during a reshuffle preview" do
+        let(:document) { fetch_graphql_fixture("ministers_index-reshuffle-mode-on-preview") }
+
+        it_behaves_like "ministers index page during a reshuffle preview"
+      end
     end
 
-    context "during a reshuffle preview" do
-      let(:document) { fetch_graphql_fixture("ministers_index-reshuffle-mode-on-preview") }
+    context "when the GraphQL parameter is true" do
+      before do
+        visit "/government/ministers?graphql=true"
+      end
 
-      it_behaves_like "ministers index page during a reshuffle preview"
+      let(:document) { fetch_graphql_fixture("ministers_index-reshuffle-mode-off") }
+
+      it "gets the data from GraphQL" do
+        expect(a_request(:post, "#{Plek.find('publishing-api')}/graphql")).to have_been_made
+      end
+    end
+
+    context "when the GraphQL parameter is false" do
+      let(:document) { GovukSchemas::Example.find("ministers_index", example_name: "ministers_index-reshuffle-mode-off") }
+
+      before do
+        stub_content_store_has_item("/government/ministers", document)
+        visit "/government/ministers?graphql=false"
+      end
+
+      it "does not get the data from GraphQL" do
+        expect(a_request(:post, "#{Plek.find('publishing-api')}/graphql")).not_to have_been_made
+      end
     end
   end
 end