Merge pull request #251 from alphagov/remove-early-access-users-from-questions

Remove Early Access Users from Questions/Conversations

Author: Sunny-xyz

app/controllers/admin/questions_controller.rb

@@ -5,7 +5,7 @@ def index
   end
 
   def show
-    @question = Question.includes(conversation: %i[user signon_user], answer: %i[feedback sources])
+    @question = Question.includes(conversation: :signon_user, answer: %i[feedback sources])
                          .find(params[:id])
     @answer = @question.answer
     @question_number = Question.where(conversation: @question.conversation)
@@ -26,7 +26,6 @@ def filter_params
       :question_routing_label,
       :page,
       :sort,
-      :user_id,
       :signon_user_id,
       :conversation_id,
     )

app/helpers/admin/questions_helper.rb

@@ -61,29 +61,6 @@ def question_show_summary_list_rows(question, answer, question_number, total_que
         },
       ].compact
 
-      user = conversation.user
-      if user.present?
-        rows << {
-          field: "Early access user",
-          value: safe_join([
-            link_to(user.email, admin_early_access_user_path(user), class: "govuk-link"),
-            " (",
-            link_to("View all questions", admin_questions_path(user_id: user.id), class: "govuk-link"),
-            ")",
-          ]),
-        }
-      elsif conversation.early_access_user_id
-        rows << {
-          field: "Early access user",
-          value: safe_join([
-            "Deleted user",
-            " (",
-            link_to("View all questions", admin_questions_path(user_id: conversation.early_access_user_id), class: "govuk-link"),
-            ")",
-          ]),
-        }
-      end
-
       signon_user = conversation.signon_user
       if signon_user.present? && conversation.source_api?
         rows << {

app/models/admin/filters/questions_filter.rb

@@ -7,7 +7,6 @@ class Admin::Filters::QuestionsFilter < Admin::Filters::BaseFilter
   attribute :conversation_id
   attribute :answer_feedback_useful, :boolean
   attribute :question_routing_label
-  attribute :user_id
   attribute :signon_user_id
 
   validate :validate_dates
@@ -38,19 +37,12 @@ def results
       scope = conversation_scope(scope)
       scope = question_routing_label_scope(scope)
       scope = ordering_scope(scope)
-      scope = user_scope(scope)
       scope = signon_user_scope(scope)
       scope.page(page)
            .per(25)
     end
   end
 
-  def user
-    return @user if defined?(@user)
-
-    @user = EarlyAccessUser.includes(:conversations).find_by_id(user_id)
-  end
-
   def signon_user
     return @signon_user if defined?(@signon_user)
 
@@ -73,7 +65,6 @@ def pagination_query_params
     filters[:end_date_params] = end_date_params if end_date_params.values.any?(&:present?)
     filters[:sort] = sort if sort != self.class.default_sort
     filters[:answer_feedback_useful] = answer_feedback_useful unless answer_feedback_useful.nil?
-    filters[:user_id] = user_id if user_id.present?
     filters[:conversation_id] = conversation.id if conversation.present?
 
     filters
@@ -125,14 +116,8 @@ def conversation_scope(scope)
     scope.where(conversation_id: conversation.id)
   end
 
-  def user_scope(scope)
-    return scope if user_id.blank?
-
-    scope.joins(:conversation).where("conversations.early_access_user_id = ?", user_id)
-  end
-
   def signon_user_scope(scope)
-    return scope if signon_user_id.blank? || user_id.present?
+    return scope if signon_user_id.blank?
 
     scope.joins(:conversation).where(conversation: { signon_user_id: signon_user_id, source: :api })
   end

app/views/admin/questions/_question_filters.html.erb

@@ -1,44 +1,17 @@
 <%
-  user = filter.user
   conversation = filter.conversation
   signon_user = filter.signon_user
 %>
 
 <%= form_with(url: admin_questions_path, method: :get) do %>
-  <% if params[:user_id].present? %>
-    <p class="govuk-body">
-      Filtering by user:
-      <% if user.present? %>
-        <%= link_to user.email, admin_early_access_user_path(user), class: "govuk-link" %>
-      <% else %>
-        <%= params[:user_id] %> (Deleted user)
-      <% end %>
-    </p>
-    <%= hidden_field_tag(:user_id, params[:user_id]) %>
-  <% elsif signon_user.present? %>
+  <% if signon_user.present? %>
     <p class="govuk-body">
       Filtering by API user: <%= signon_user.name %>
     </p>
     <%= hidden_field_tag(:signon_user_id, params[:signon_user_id]) %>
   <% end %>
 
-  <% if user.present? %>
-    <%= render "govuk_publishing_components/components/select", {
-      id: "conversation_id",
-      name: "conversation_id",
-      label: "Conversation",
-      heading_size: "s",
-      full_width: true,
-      options: [{ text: "", value: "" }] +
-        user.conversations.map.with_index(1) do |conversation, index|
-          {
-            text: index.ordinalize,
-            value: conversation.id,
-            selected: params[:conversation_id] == conversation.id,
-          }
-        end,
-    } %>
-  <% elsif conversation.present? %>
+  <% if conversation.present? %>
     <p class="govuk-body govuk-!-font-weight-bold govuk-!-margin-bottom-2">
       Filtering by conversation ID:
     </p>

spec/helpers/admin/questions_helper_spec.rb

@@ -48,7 +48,6 @@
       result = helper.question_show_summary_list_rows(question, nil, 1, 1)
       expected_keys = [
         "Conversation id",
-        "Early access user",
         "Question number",
         "Question id",
         "Question created at",
@@ -72,7 +71,6 @@
         "Question created at",
         "Question",
         "Show search results",
-        "Early access user",
         "Source",
         "Rephrased question",
         "Status",
@@ -143,32 +141,6 @@
       expect(row[:value]).to eq("Advice, opinions, predictions")
     end
 
-    it "returns a row with a link to the user's details" do
-      result = helper.question_show_summary_list_rows(question, nil, 1, 1)
-
-      row = result.find { |r| r[:field] == "Early access user" }
-      value = row[:value]
-
-      expect(value)
-        .to have_link(conversation.user.email, href: admin_early_access_user_path(conversation.user))
-        .and have_link("View all questions", href: admin_questions_path(user_id: conversation.user.id))
-    end
-
-    it "returns a row with a link to the deleted user's details" do
-      user_id = conversation.user.id
-      conversation.user.destroy!
-      conversation.reload
-
-      result = helper.question_show_summary_list_rows(question, nil, 1, 1)
-
-      row = result.find { |r| r[:field] == "Early access user" }
-      value = row[:value]
-
-      expect(value).to include("Deleted user")
-
-      expect(value).to have_link("View all questions", href: admin_questions_path(user_id:))
-    end
-
     it "doesn't return a 'Early access user' field when the conversation is not associated with one" do
       conversation.update!(user: nil)
 

spec/models/admin/filters/questions_filter_spec.rb

@@ -218,19 +218,6 @@
       expect(filter.results).to eq([useless_question])
     end
 
-    it "filters the results by user" do
-      alice = create(:early_access_user, email: "alice@example.com")
-      bob = create(:early_access_user, email: "bob@example.com")
-      alice_question = create(:question, conversation: create(:conversation, user: alice))
-      bob_question = create(:question, conversation: create(:conversation, user: bob))
-
-      filter = described_class.new(user_id: alice.id)
-      expect(filter.results).to eq([alice_question])
-
-      filter = described_class.new(user_id: bob.id)
-      expect(filter.results).to eq([bob_question])
-    end
-
     it "filters the results by signon user" do
       alice = create(:signon_user, email: "alice@example.com")
       bob = create(:signon_user, email: "bob@example.com")
@@ -253,16 +240,6 @@
       expect(filter.results).to eq([])
     end
 
-    it "doesn't filter the results by signon user if signon_user_id and user_id are passed in" do
-      alice = create(:signon_user, email: "alice@example.com")
-      bob = create(:early_access_user, email: "bob@example.com")
-      create(:question, conversation: create(:conversation, signon_user: alice))
-      bob_question = create(:question, conversation: create(:conversation, user: bob))
-
-      filter = described_class.new(signon_user_id: alice.id, user_id: bob.id)
-      expect(filter.results).to eq([bob_question])
-    end
-
     it "filters the results by question routing label" do
       create(:question, answer: build(:answer, question_routing_label: "genuine_rag"))
       non_english_question = create(:question, answer: build(:answer, question_routing_label: "non_english"))
@@ -293,25 +270,6 @@
     end
   end
 
-  describe "#user" do
-    it "returns the user if user_id is passed in" do
-      user = create(:early_access_user)
-      filter = described_class.new(user_id: user.id)
-
-      expect(filter.user).to eq(user)
-    end
-
-    it "returns nil if user_id is not passed in" do
-      filter = described_class.new
-      expect(filter.user).to be_nil
-    end
-
-    it "returns nil if user_id is passed in but the user does not exist" do
-      filter = described_class.new(user_id: "invalid_id")
-      expect(filter.user).to be_nil
-    end
-  end
-
   describe "#signon_user" do
     it "returns the signon_user if signon_user_id is passed in" do
       signon_user = create(:signon_user)
@@ -354,8 +312,7 @@
 
   describe "#previous_page_params" do
     it "retains all other query params when constructing the params" do
-      user = create(:early_access_user)
-      conversation = create(:conversation, user:)
+      conversation = create(:conversation)
       26.times do
         question = create(:question, conversation:)
         create(:answer, :with_feedback, question:)
@@ -371,7 +328,6 @@
         start_date_params:,
         end_date_params:,
         answer_feedback_useful: "true",
-        user_id: user.id,
         conversation_id: conversation.id,
       )
 
@@ -383,7 +339,6 @@
             answer_feedback_useful: true,
             start_date_params:,
             end_date_params:,
-            user_id: user.id,
             conversation_id: conversation.id,
           },
         )
@@ -392,8 +347,7 @@
 
   describe "#next_page_params" do
     it "retains all other query params when constructing the params" do
-      user = create(:early_access_user)
-      conversation = create(:conversation, user:)
+      conversation = create(:conversation)
       26.times do
         question = create(:question, conversation:)
         create(:answer, :with_feedback, question:)
@@ -408,7 +362,6 @@
         start_date_params:,
         end_date_params:,
         answer_feedback_useful: "true",
-        user_id: user.id,
         conversation_id: conversation.id,
       )
 
@@ -421,7 +374,6 @@
             page: 2,
             start_date_params:,
             end_date_params:,
-            user_id: user.id,
             conversation_id: conversation.id,
           },
         )

spec/requests/admin/questions_spec.rb

@@ -79,31 +79,6 @@
         expect_unprocessible_entity_with_date_errors
       end
 
-      it "renders the user's details when filtering by a non-deleted user" do
-        user = create(:early_access_user)
-        get admin_questions_path(user_id: user.id)
-
-        expect(response.body.squish).to have_content("Filtering by user: #{user.email}")
-      end
-
-      it "renders the user's ID when filtering by a deleted user" do
-        user = create(:early_access_user)
-        user.destroy!
-        get admin_questions_path(user_id: user.id)
-
-        expect(response.body.squish).to have_content("Filtering by user: #{user.id} (Deleted user)")
-      end
-
-      it "renders a conversation_id select filter when filtering by a user" do
-        user = create(:early_access_user)
-        create(:conversation, user:)
-        create(:conversation, user:)
-
-        get admin_questions_path(user_id: user.id)
-
-        expect(response.body).to have_select("conversation_id", options: ["", "1st", "2nd"])
-      end
-
       it "renders a conversation_id when filtering by a conversation_id" do
         conversation = create(:conversation)
         get admin_questions_path(conversation_id: conversation.id)
@@ -119,21 +94,6 @@
         expect(response.body.squish)
           .to have_content("Filtering by API user: #{signon_user.name}")
       end
-
-      context "and filter params are present for user and signon user" do
-        it "doesn't render the signon user's details" do
-          signon_user = create(:signon_user)
-          create(:conversation, signon_user:)
-
-          get admin_questions_path(
-            signon_user_id: signon_user.id,
-            user_id: SecureRandom.uuid,
-          )
-
-          expect(response.body.squish)
-            .not_to have_content("Filtering by API user: #{signon_user.name}")
-        end
-      end
     end
 
     context "when the sort param is not the default value" do