From a3a221fdaa85760596dad628ce6461e4e23b54f4 Mon Sep 17 00:00:00 2001
From: Christophe Bliard <c.bliard@openproject.com>
Date: Fri, 13 Sep 2024 08:35:26 +0200
Subject: [PATCH] Fixes from PR review

Co-authored-by: Aaron Contreras <aaronlcaq@gmail.com>
---
 app/components/op_primer/form_helpers.rb      |  4 +-
 lookbook/docs/patterns/14-forms.md.erb        | 55 ++++++++++++-------
 .../aggregation_settings_controller_spec.rb   |  2 +-
 3 files changed, 38 insertions(+), 23 deletions(-)

diff --git a/app/components/op_primer/form_helpers.rb b/app/components/op_primer/form_helpers.rb
index 9c6d465bfd26..fb742d2a5d4a 100644
--- a/app/components/op_primer/form_helpers.rb
+++ b/app/components/op_primer/form_helpers.rb
@@ -51,7 +51,7 @@ module FormHelpers
     #     end
     #   end
     #
-    # @param f [Object] The form object to be used for the form.
+    # @param form_builder [Object] The form builder object to be used for the form.
     # @param blk [Proc] A block that defines the form structure.
     def render_inline_form(form_builder, &blk)
       form_class = Class.new(ApplicationForm) do
@@ -85,7 +85,7 @@ def render_inline_form(form_builder, &blk)
     #     end
     #   end
     #
-    # @param f [Object] The form object to be used for the form.
+    # @param form_builder [Object] The form builder object to be used for the form.
     # @param blk [Proc] A block that defines the form structure.
     def render_inline_settings_form(form_builder, &blk)
       form_class = Class.new(ApplicationForm) do
diff --git a/lookbook/docs/patterns/14-forms.md.erb b/lookbook/docs/patterns/14-forms.md.erb
index 18199a8afacb..1c1598b39cba 100644
--- a/lookbook/docs/patterns/14-forms.md.erb
+++ b/lookbook/docs/patterns/14-forms.md.erb
@@ -60,6 +60,36 @@ This helper allows to render an anymous form instance, avoiding the need to
 create a dedicated form class. This can be useful for simple forms or when you
 don't want to pollute the form class namespace.
 
+The above example which was needing a dedicated `TextFieldAndCheckboxForm` form
+class can be rewritten like this:
+
+```erb
+<%%=
+primer_form_with(url: "/foo") do |f|
+  render_inline_form(f) do |my_form|
+    my_form.text_field(
+      name: :ultimate_answer,
+      label: "Ultimate answer",
+      required: true,
+      caption: "The answer to life, the universe, and everything"
+    )
+
+    my_form.check_box(
+      name: :enable_ipd,
+      label: "Enable the Infinite Improbability Drive",
+      caption: "Cross interstellar distances in a mere nothingth of a second."
+    )
+  end
+end
+%>
+```
+
+### `FormObject#html_content` to mix form fields and html content
+
+This helper allows to render non-form content in a form. For instance it can be
+used to render a description box inside a form, an image, or whatever makes
+sense for the form being built.
+
 ```ruby
 class TextFieldWithWarningForm < ApplicationForm
   attr_reader :warning
@@ -71,8 +101,8 @@ class TextFieldWithWarningForm < ApplicationForm
 
   form do |my_form|
     my_form.text_field(
-      name: :answer,
-      label: "answer",
+      name: :full_name,
+      label: "Full name",
       required: true
     )
 
@@ -87,21 +117,6 @@ class TextFieldWithWarningForm < ApplicationForm
 end
 ```
 
-### `FormObject#html_content` to mix form fields and html content
-
-This helper allows to render non-form content in a form. For instance it can be
-used to render a description box inside a form, an image, or whatever makes
-sense for the form being built.
-
-```erb
-<%%= primer_form_with(url: "/foo") do |f| %>
-  <%%= render(TextFieldAndCheckboxForm.new(f)) %>
-  <%%= f.html_content do %>
-    <%%= render(MessageComponent.new(icon: :info, message: "This will be fine!")) %>
-  <%% end %>
-<%% end %>
-```
-
 ## Forms for administration pages
 
 Administration pages forms are used to change the values of `Settings`. The name
@@ -166,8 +181,8 @@ or configuration files), the field name has to be translated and the value must
 be read from `Settings`. Entering all this information manually is tedious and
 error prone.
 
-In this case, `settings_form` can be usedinstead of `form` to get a form
-instance with knowledge about to render fields for settings.
+In this case, `settings_form` can be used instead of `form` to get a form
+instance with knowledge about how render fields for settings.
 
 The above example then becomes:
 
@@ -200,7 +215,7 @@ end
 It is easier to write and read.
 
 Under the hood, the form object is decorated with `SettingsFormDecorator`.
-That's where all the helper methods are defined. There are not much for now, but
+That's where all the helper methods are defined. There aren't many for now, but
 this is intended to grow to support more advanced form features for
 administration pages.
 
diff --git a/spec/controllers/admin/settings/aggregation_settings_controller_spec.rb b/spec/controllers/admin/settings/aggregation_settings_controller_spec.rb
index 9495179af8a4..93ed0c63a785 100644
--- a/spec/controllers/admin/settings/aggregation_settings_controller_spec.rb
+++ b/spec/controllers/admin/settings/aggregation_settings_controller_spec.rb
@@ -34,7 +34,7 @@
 RSpec.describe Admin::Settings::AggregationSettingsController do
   shared_let(:user) { create(:admin) }
 
-  shared_current_user { user }
+  current_user { user }
 
   include_examples "GET #show requires admin permission and renders template", path: "aggregation_settings"
 end