fix: normalize tag labels (#2888)

* drop `fetch_labels` in favor of `format_using`

* fix use label if item is a hash

* lint

* revert format_using extraction

* large whitelist items when suggestions are not enforced

* tweaks

* add test

* lint

* lint

* lint

* lint

* fix test

* lint

* try fix test

* lint

* fix test

* fix test

* fix test

* lint

app/components/avo/fields/concerns/item_labels.rb

@@ -13,6 +13,8 @@ def value_for_item(item)
         end
 
         def label_from_item(item)
+          return item[:label] if item.is_a?(Hash) && item[:label].present?
+
           value = value_for_item item
 
           if suggestions_are_a_hash?

app/components/avo/fields/tags_field/edit_component.html.erb

@@ -1,7 +1,7 @@
 <%= field_wrapper **field_wrapper_args, data: {
       controller: "tags-field",
       tags_field_mode_value: @field.mode,
-      tags_field_whitelist_items_value: @field.suggestions.to_json,
+      tags_field_whitelist_items_value: @field.whitelist_items,
       tags_field_disallowed_items_value: @field.disallowed.to_json,
       tags_field_enforce_suggestions_value: @field.enforce_suggestions,
       tags_field_suggestions_max_items_value: @field.suggestions_max_items,

lib/avo/fields/tags_field.rb

@@ -20,19 +20,22 @@ def initialize(id, **args, &block)
         add_string_prop args, :suggestions_max_items
         add_string_prop args, :mode, nil
         add_string_prop args, :fetch_values_from
-        add_string_prop args, :fetch_labels
-      end
-
-      def field_value
-        return fetched_labels if @fetch_labels.present?
 
-        return json_value if acts_as_taggable_on.present?
+        @format_using ||= args[:fetch_labels]
 
-        value || []
+        unless Rails.env.production?
+          if args[:fetch_labels].present?
+            puts "[Avo DEPRECATION WARNING]: The `fetch_labels` field configuration option is no longer supported and will be removed in future versions. Please discontinue its use and solely utilize the `format_using` instead."
+          end
+        end
       end
 
-      def json_value
-        acts_as_taggable_on_values.map { |value| {value:} }.as_json
+      def field_value
+        @field_value ||= if acts_as_taggable_on.present?
+          acts_as_taggable_on_values.map { |value| {value:} }.as_json
+        else
+          value || []
+        end
       end
 
       def acts_as_taggable_on_values
@@ -67,8 +70,14 @@ def fill_acts_as_taggable(record, key, value, params)
         record
       end
 
+      def whitelist_items
+        return suggestions.to_json if enforce_suggestions
+
+        (suggestions + field_value).to_json
+      end
+
       def suggestions
-        Avo::ExecutionContext.new(target: @suggestions, record: record).handle
+        @fetched_suggestions ||= Avo::ExecutionContext.new(target: @suggestions, record: record).handle
       end
 
       def disallowed
@@ -81,14 +90,6 @@ def fetch_values_from
 
       private
 
-      def fetched_labels
-        if @fetch_labels.respond_to?(:call)
-          Avo::ExecutionContext.new(target: @fetch_labels, resource: resource, record: record).handle
-        else
-          @fetch_labels
-        end
-      end
-
       def act_as_taggable_attribute(key)
         "#{key.singularize}_list="
       end

spec/dummy/app/avo/resources/course.rb

@@ -51,6 +51,18 @@ def fields_bag
         end
       end
 
+      # field :skills,
+      #   as: :tags,
+      #   fetch_values_from: "/admin/resources/users/get_users?hey=you&record_id=1", # {value: 1, label: "Jose"}
+      #   format_using: -> {
+      #     User.find(value).map do |user|
+      #       {
+      #         value: user.id,
+      #         label: user.name
+      #       }
+      #     end
+      #   }
+
       field :skills,
         as: :tags,
         disallowed: -> { record.skill_disallowed },

spec/dummy/app/models/course.rb

@@ -29,7 +29,7 @@ def has_skills=(value)
   end
 
   def skill_suggestions
-    ["example suggestion", "example tag", name]
+    ["example suggestion", "example tag", name].compact
   end
 
   def skill_disallowed

spec/system/avo/tags_spec.rb

@@ -170,6 +170,66 @@
       Avo::Resources::Course.restore_items_from_backup
     end
   end
+
+  describe "format_using (same as deprecated fetch_labels) with fetch_values_from" do
+    let!(:users) { create_list :user, 2, first_name: "Bob" }
+    let!(:courses) { create_list :course, 2, skills: users.pluck(:id) }
+    let(:field_value_slot) { tags_element(find_field_value_element("skills")) }
+    let(:tags_input) { field_value_slot.find("span[contenteditable]") }
+
+    it "fetches the labels" do
+      Avo::Resources::Course.with_temporary_items do
+        field :name
+        field :skills,
+          as: :tags,
+          fetch_values_from: "/admin/resources/users/get_users", # {value: 1, label: "Jose"}
+          format_using: -> {
+            User.find(value).map do |user|
+              {
+                value: user.id,
+                label: user.name
+              }
+            end
+          }
+      end
+
+      visit avo.resources_courses_path
+
+      expect(page).to have_text "#{users[0].first_name} #{users[0].last_name}"
+      expect(page).to have_text "#{users[1].first_name} #{users[1].last_name}"
+    end
+
+    it "keep correct tags on validations error and edit" do
+      visit avo.new_resources_course_path
+
+      tags_input.click
+      tags_input.set("Bob")
+      wait_for_tags_to_load(field_value_slot)
+      type(:down)
+      type(:down, :return)
+
+      sleep 0.3
+      save
+
+      expect(page).to have_text "Name can't be blank"
+      expect(page).to have_text "#{users[0].first_name} #{users[0].last_name}"
+
+      fill_in "course_name", with: "The course"
+
+      tags_input.click
+      tags_input.set("Bob")
+      wait_for_tags_to_load(field_value_slot)
+      type(:down)
+      type(:down, :return)
+
+      sleep 0.3
+      save
+
+      expect(Course.last.skills.map(&:to_i)).to eql(users.pluck(:id))
+
+      Avo::Resources::Course.restore_items_from_backup
+    end
+  end
 end
 
 def wait_for_tags_to_load(element, time = Capybara.default_max_wait_time)
@@ -182,3 +242,9 @@ def wait_for_tags_to_load(element, time = Capybara.default_max_wait_time)
 def tags_element(parent)
   parent.find "tags.tagify"
 end
+
+def wait_until
+  Timeout.timeout(Capybara.default_max_wait_time) do
+    loop until yield
+  end
+end