Automatic merge of gitlab-org/gitlab-ce master

app/assets/javascripts/lib/utils/grammar.js

+import { sprintf, s__ } from '~/locale';
+
+/**
+ * Combines each given item into a noun series sentence fragment. It does this
+ * in a way that supports i18n by giving context and punctuation to the locale
+ * functions.
+ *
+ * **Examples:**
+ *
+ * - `["A", "B"] => "A and B"`
+ * - `["A", "B", "C"] => "A, B, and C"`
+ *
+ * **Why only nouns?**
+ *
+ * Some languages need a bit more context to translate other series.
+ *
+ * @param {String[]} items
+ */
+export const toNounSeriesText = items => {
+  if (items.length === 0) {
+    return '';
+  } else if (items.length === 1) {
+    return items[0];
+  } else if (items.length === 2) {
+    return sprintf(s__('nounSeries|%{firstItem} and %{lastItem}'), {
+      firstItem: items[0],
+      lastItem: items[1],
+    });
+  }
+
+  return items.reduce((item, nextItem, idx) =>
+    idx === items.length - 1
+      ? sprintf(s__('nounSeries|%{item}, and %{lastItem}'), { item, lastItem: nextItem })
+      : sprintf(s__('nounSeries|%{item}, %{nextItem}'), { item, nextItem }),
+  );
+};
+
+export default {
+  toNounSeriesText,
+};

app/workers/mail_scheduler/notification_service_worker.rb

@@ -23,7 +23,7 @@ module MailScheduler
     end
 
     def self.perform_async(*args)
-      super(*ActiveJob::Arguments.serialize(args))
+      super(*Arguments.serialize(args))
     end
 
     private
@@ -38,5 +38,34 @@ module MailScheduler
         end
       end
     end
+
+    # Permit ActionController::Parameters for serializable Hash
+    #
+    # Port of
+    # https://github.com/rails/rails/commit/945fdd76925c9f615bf016717c4c8db2b2955357#diff-fc90ec41ef75be8b2259526fe1a8b663
+    module Arguments
+      include ActiveJob::Arguments
+      extend self
+
+      private
+
+      def serialize_argument(argument)
+        case argument
+        when -> (arg) { arg.respond_to?(:permitted?) }
+          serialize_hash(argument.to_h).tap do |result|
+            result[WITH_INDIFFERENT_ACCESS_KEY] = serialize_argument(true)
+          end
+        else
+          super
+        end
+      end
+    end
+
+    # Make sure we remove this patch starting with Rails 6.0.
+    if Rails.version.start_with?('6.0')
+      raise <<~MSG
+        Please remove the patch `Arguments` module and use `ActiveJob::Arguments` again.
+      MSG
+    end
   end
 end

changelogs/unreleased/pl-serialize-ac-parameters.yml

+---
+title: Make `ActionController::Parameters` serializable for sidekiq jobs
+merge_request: 24864
+author:
+type: fixed

locale/gitlab.pot

@@ -11398,6 +11398,15 @@ msgstr ""
 msgid "notification emails"
 msgstr ""
 
+msgid "nounSeries|%{firstItem} and %{lastItem}"
+msgstr ""
+
+msgid "nounSeries|%{item}, %{nextItem}"
+msgstr ""
+
+msgid "nounSeries|%{item}, and %{lastItem}"
+msgstr ""
+
 msgid "or"
 msgstr ""
 

spec/javascripts/lib/utils/grammar_spec.js

+import * as grammar from '~/lib/utils/grammar';
+
+describe('utils/grammar', () => {
+  describe('toNounSeriesText', () => {
+    it('with empty items returns empty string', () => {
+      expect(grammar.toNounSeriesText([])).toBe('');
+    });
+
+    it('with single item returns item', () => {
+      const items = ['Lorem Ipsum'];
+
+      expect(grammar.toNounSeriesText(items)).toBe(items[0]);
+    });
+
+    it('with 2 items returns item1 and item2', () => {
+      const items = ['Dolar', 'Sit Amit'];
+
+      expect(grammar.toNounSeriesText(items)).toBe(`${items[0]} and ${items[1]}`);
+    });
+
+    it('with 3 items returns comma separated series', () => {
+      const items = ['Lorem', 'Ipsum', 'dolar'];
+      const expected = 'Lorem, Ipsum, and dolar';
+
+      expect(grammar.toNounSeriesText(items)).toBe(expected);
+    });
+
+    it('with 6 items returns comma separated series', () => {
+      const items = ['Lorem', 'ipsum', 'dolar', 'sit', 'amit', 'consectetur'];
+      const expected = 'Lorem, ipsum, dolar, sit, amit, and consectetur';
+
+      expect(grammar.toNounSeriesText(items)).toBe(expected);
+    });
+  });
+});

spec/workers/mail_scheduler/notification_service_worker_spec.rb

@@ -9,6 +9,10 @@ describe MailScheduler::NotificationServiceWorker do
     ActiveJob::Arguments.serialize(args)
   end
 
+  def deserialize(args)
+    ActiveJob::Arguments.deserialize(args)
+  end
+
   describe '#perform' do
     it 'deserializes arguments from global IDs' do
       expect(worker.notification_service).to receive(method).with(key)
@@ -42,13 +46,48 @@ describe MailScheduler::NotificationServiceWorker do
     end
   end
 
-  describe '.perform_async' do
+  describe '.perform_async', :sidekiq do
+    around do |example|
+      Sidekiq::Testing.fake! { example.run }
+    end
+
     it 'serializes arguments as global IDs when scheduling' do
-      Sidekiq::Testing.fake! do
-        described_class.perform_async(method, key)
+      described_class.perform_async(method, key)
+
+      expect(described_class.jobs.count).to eq(1)
+      expect(described_class.jobs.first).to include('args' => [method, *serialize(key)])
+    end
+
+    context 'with ActiveController::Parameters' do
+      let(:parameters) { ActionController::Parameters.new(hash) }
+
+      let(:hash) do
+        {
+          "nested" => {
+            "hash" => true
+          }
+        }
+      end
+
+      context 'when permitted' do
+        before do
+          parameters.permit!
+        end
+
+        it 'serializes as a serializable Hash' do
+          described_class.perform_async(method, parameters)
 
-        expect(described_class.jobs.count).to eq(1)
-        expect(described_class.jobs.first).to include('args' => [method, *serialize(key)])
+          expect(described_class.jobs.count).to eq(1)
+          expect(deserialize(described_class.jobs.first['args']))
+            .to eq([method, hash])
+        end
+      end
+
+      context 'when not permitted' do
+        it 'fails to serialize' do
+          expect { described_class.perform_async(method, parameters) }
+            .to raise_error(ActionController::UnfilteredParameters)
+        end
       end
     end
   end