Automatic merge of gitlab-org/gitlab master

GITALY_SERVER_VERSION

-a5a5d83630f13c3eb3e1650a24423fc5e9bc47d2
+3cbd24e3e2fd09eb526d04f8a419f6d103c440dc

Gemfile

@@ -115,7 +115,7 @@ gem 'carrierwave', '~> 1.3'
 gem 'mini_magick', '~> 4.10.1'
 
 # for backups
-gem 'fog-aws', '~> 3.6'
+gem 'fog-aws', '~> 3.7'
 # Locked until fog-google resolves https://github.com/fog/fog-google/issues/421.
 # Also see config/initializers/fog_core_patch.rb.
 gem 'fog-core', '= 2.1.0'

Gemfile.lock

@@ -363,7 +363,7 @@ GEM
       fog-json
       ipaddress (~> 0.8)
       xml-simple (~> 1.1)
-    fog-aws (3.6.7)
+    fog-aws (3.7.0)
       fog-core (~> 2.1)
       fog-json (~> 1.1)
       fog-xml (~> 0.1)
@@ -1335,7 +1335,7 @@ DEPENDENCIES
   flipper-active_support_cache_store (~> 0.17.1)
   flowdock (~> 0.7)
   fog-aliyun (~> 0.3)
-  fog-aws (~> 3.6)
+  fog-aws (~> 3.7)
   fog-core (= 2.1.0)
   fog-google (~> 1.11)
   fog-local (~> 0.6)

app/assets/javascripts/commit/image_file.js

@@ -27,7 +27,7 @@ export default class ImageFile {
 
   initViewModes() {
     const viewMode = viewModes[0];
-    $('.view-modes', this.file).removeClass('hide');
+    $('.view-modes', this.file).removeClass('gl-display-none');
     $('.view-modes-menu', this.file).on('click', 'li', event => {
       if (!$(event.currentTarget).hasClass('active')) {
         return this.activateViewMode(event.currentTarget.className);
@@ -42,12 +42,10 @@ export default class ImageFile {
       .filter(`.${viewMode}`)
       .addClass('active');
 
-    // eslint-disable-next-line no-jquery/no-fade
-    return $(`.view:visible:not(.${viewMode})`, this.file).fadeOut(200, () => {
-      // eslint-disable-next-line no-jquery/no-fade
-      $(`.view.${viewMode}`, this.file).fadeIn(200);
-      return this.initView(viewMode);
-    });
+    $(`.view:visible:not(.${viewMode})`, this.file).addClass('gl-display-none');
+    $(`.view.${viewMode}`, this.file).removeClass('gl-display-none');
+
+    return this.initView(viewMode);
   }
 
   initView(viewMode) {
@@ -120,7 +118,7 @@ export default class ImageFile {
         return this.requestImageInfo($('img', wrap), (width, height) => {
           $('.image-info .meta-width', wrap).text(`${width}px`);
           $('.image-info .meta-height', wrap).text(`${height}px`);
-          return $('.image-info', wrap).removeClass('hide');
+          return $('.image-info', wrap).removeClass('gl-display-none');
         });
       });
     },

app/assets/javascripts/due_date_select.js

@@ -119,20 +119,18 @@ class DueDateSelect {
   }
 
   updateIssueBoardIssue() {
-    // eslint-disable-next-line no-jquery/no-fade
-    this.$loading.fadeIn();
+    this.$loading.removeClass('gl-display-none');
     this.$dropdown.trigger('loading.gl.dropdown');
     this.$selectbox.hide();
     this.$value.css('display', '');
-    const fadeOutLoader = () => {
-      // eslint-disable-next-line no-jquery/no-fade
-      this.$loading.fadeOut();
+    const hideLoader = () => {
+      this.$loading.addClass('gl-display-none');
     };
 
     boardsStore.detail.issue
       .update(this.$dropdown.attr('data-issue-update'))
-      .then(fadeOutLoader)
-      .catch(fadeOutLoader);
+      .then(hideLoader)
+      .catch(hideLoader);
   }
 
   submitSelectedDate(isDropdown) {
@@ -140,8 +138,7 @@ class DueDateSelect {
     const hasDueDate = this.displayedDate !== __('None');
     const displayedDateStyle = hasDueDate ? 'bold' : 'no-value';
 
-    // eslint-disable-next-line no-jquery/no-fade
-    this.$loading.removeClass('hidden').fadeIn();
+    this.$loading.removeClass('gl-display-none');
 
     if (isDropdown) {
       this.$dropdown.trigger('loading.gl.dropdown');
@@ -164,8 +161,7 @@ class DueDateSelect {
       }
       this.$sidebarCollapsedValue.attr('data-original-title', tooltipText);
 
-      // eslint-disable-next-line no-jquery/no-fade
-      return this.$loading.fadeOut();
+      return this.$loading.addClass('gl-display-none');
     });
   }
 }
@@ -211,7 +207,8 @@ export default class DueDateSelectors {
   initIssuableSelect() {
     const $loading = $('.js-issuable-update .due_date')
       .find('.block-loading')
-      .hide();
+      .removeClass('hidden')
+      .addClass('gl-display-none');
 
     $('.js-due-date-select').each((i, dropdown) => {
       const $dropdown = $(dropdown);

app/assets/javascripts/filterable_list.js

@@ -64,8 +64,7 @@ export default class FilterableList {
       return false;
     }
 
-    // eslint-disable-next-line no-jquery/no-fade
-    $(this.listHolderElement).fadeTo(250, 0.5);
+    $(this.listHolderElement).addClass('gl-opacity-5');
 
     this.isBusy = true;
 
@@ -99,7 +98,6 @@ export default class FilterableList {
 
   onFilterComplete() {
     this.isBusy = false;
-    // eslint-disable-next-line no-jquery/no-fade
-    $(this.listHolderElement).fadeTo(250, 1);
+    $(this.listHolderElement).removeClass('gl-opacity-5');
   }
 }

app/assets/javascripts/labels_select.js

@@ -45,8 +45,7 @@ export default class LabelsSelect {
       const $sidebarCollapsedValue = $block.find('.sidebar-collapsed-icon span');
       const $value = $block.find('.value');
       const $dropdownMenu = $dropdown.parent().find('.dropdown-menu');
-      // eslint-disable-next-line no-jquery/no-fade
-      const $loading = $block.find('.block-loading').fadeOut();
+      const $loading = $block.find('.block-loading').addClass('gl-display-none');
       const fieldName = $dropdown.data('fieldName');
       let initialSelected = $selectbox
         .find(`input[name="${$dropdown.data('fieldName')}"]`)
@@ -83,15 +82,13 @@ export default class LabelsSelect {
         if (!selected.length) {
           data[abilityName].label_ids = [''];
         }
-        // eslint-disable-next-line no-jquery/no-fade
-        $loading.removeClass('hidden').fadeIn();
+        $loading.removeClass('gl-display-none');
         $dropdown.trigger('loading.gl.dropdown');
         axios
           .put(issueUpdateURL, data)
           .then(({ data }) => {
             let template;
-            // eslint-disable-next-line no-jquery/no-fade
-            $loading.fadeOut();
+            $loading.addClass('gl-display-none');
             $dropdown.trigger('loaded.gl.dropdown');
             $selectbox.hide();
             data.issueUpdateURL = issueUpdateURL;
@@ -340,9 +337,8 @@ export default class LabelsSelect {
           const { $el, e, isMarking } = clickEvent;
           const label = clickEvent.selectedObj;
 
-          const fadeOutLoader = () => {
-            // eslint-disable-next-line no-jquery/no-fade
-            $loading.fadeOut();
+          const hideLoader = () => {
+            $loading.addClass('gl-display-none');
           };
 
           const page = $('body').attr('data-page');
@@ -403,8 +399,7 @@ export default class LabelsSelect {
               boardsStore.detail.issue.labels = labels;
             }
 
-            // eslint-disable-next-line no-jquery/no-fade
-            $loading.fadeIn();
+            $loading.removeClass('gl-display-none');
             const oldLabels = boardsStore.detail.issue.labels;
 
             boardsStore.detail.issue
@@ -420,8 +415,8 @@ export default class LabelsSelect {
                     .removeClass('is-active');
                 }
               })
-              .then(fadeOutLoader)
-              .catch(fadeOutLoader);
+              .then(hideLoader)
+              .catch(hideLoader);
           } else if (handleClick) {
             e.preventDefault();
             handleClick(label);

app/assets/javascripts/main.js

@@ -136,10 +136,9 @@ function deferredInitialisation() {
   $('.remove-row').on('ajax:success', function removeRowAjaxSuccessCallback() {
     tooltips.dispose(this);
 
-    // eslint-disable-next-line no-jquery/no-fade
     $(this)
       .closest('li')
-      .fadeOut();
+      .addClass('gl-display-none!');
   });
 
   $('.js-remove-tr').on('ajax:before', function removeTRAjaxBeforeCallback() {
@@ -147,10 +146,9 @@ function deferredInitialisation() {
   });
 
   $('.js-remove-tr').on('ajax:success', function removeTRAjaxSuccessCallback() {
-    // eslint-disable-next-line no-jquery/no-fade
     $(this)
       .closest('tr')
-      .fadeOut();
+      .addClass('gl-display-none!');
   });
 
   const glTooltipDelay = localStorage.getItem('gl-tooltip-delay');

app/assets/javascripts/milestone_select.js

@@ -53,8 +53,7 @@ export default class MilestoneSelect {
       const $block = $selectBox.closest('.block');
       const $sidebarCollapsedValue = $block.find('.sidebar-collapsed-icon');
       const $value = $block.find('.value');
-      // eslint-disable-next-line no-jquery/no-fade
-      const $loading = $block.find('.block-loading').fadeOut();
+      const $loading = $block.find('.block-loading').addClass('gl-display-none');
       selectedMilestoneDefault = showAny ? '' : null;
       selectedMilestoneDefault =
         showNo && defaultNo ? __('No milestone') : selectedMilestoneDefault;
@@ -255,34 +254,29 @@ export default class MilestoneSelect {
             }
 
             $dropdown.trigger('loading.gl.dropdown');
-            // eslint-disable-next-line no-jquery/no-fade
-            $loading.removeClass('hidden').fadeIn();
+            $loading.removeClass('gl-display-none');
 
             boardsStore.detail.issue
               .update($dropdown.attr('data-issue-update'))
               .then(() => {
                 $dropdown.trigger('loaded.gl.dropdown');
-                // eslint-disable-next-line no-jquery/no-fade
-                $loading.fadeOut();
+                $loading.addClass('gl-display-none');
               })
               .catch(() => {
-                // eslint-disable-next-line no-jquery/no-fade
-                $loading.fadeOut();
+                $loading.addClass('gl-display-none');
               });
           } else {
             selected = $selectBox.find('input[type="hidden"]').val();
             data = {};
             data[abilityName] = {};
             data[abilityName].milestone_id = selected != null ? selected : null;
-            // eslint-disable-next-line no-jquery/no-fade
-            $loading.removeClass('hidden').fadeIn();
+            $loading.removeClass('gl-display-none');
             $dropdown.trigger('loading.gl.dropdown');
             return axios
               .put(issueUpdateURL, data)
               .then(({ data }) => {
                 $dropdown.trigger('loaded.gl.dropdown');
-                // eslint-disable-next-line no-jquery/no-fade
-                $loading.fadeOut();
+                $loading.addClass('gl-display-none');
                 $selectBox.hide();
                 $value.css('display', '');
                 if (data.milestone != null) {
@@ -313,8 +307,7 @@ export default class MilestoneSelect {
                   .text(__('None'));
               })
               .catch(() => {
-                // eslint-disable-next-line no-jquery/no-fade
-                $loading.fadeOut();
+                $loading.addClass('gl-display-none');
               });
           }
         },

app/assets/javascripts/users_select/index.js

@@ -63,8 +63,7 @@ function UsersSelect(currentUser, els, options = {}) {
     const abilityName = $dropdown.data('abilityName');
     let $value = $block.find('.value');
     const $collapsedSidebar = $block.find('.sidebar-collapsed-user');
-    // eslint-disable-next-line no-jquery/no-fade
-    const $loading = $block.find('.block-loading').fadeOut();
+    const $loading = $block.find('.block-loading').addClass('gl-display-none');
     const selectedIdDefault = defaultNullUser && showNullUser ? 0 : null;
     let selectedId = $dropdown.data('selected');
     let assignTo;
@@ -205,16 +204,14 @@ function UsersSelect(currentUser, els, options = {}) {
       const data = {};
       data[abilityName] = {};
       data[abilityName].assignee_id = selected != null ? selected : null;
-      // eslint-disable-next-line no-jquery/no-fade
-      $loading.removeClass('hidden').fadeIn();
+      $loading.removeClass('gl-display-none');
       $dropdown.trigger('loading.gl.dropdown');
 
       return axios.put(issueURL, data).then(({ data }) => {
         let user = {};
         let tooltipTitle = user.name;
         $dropdown.trigger('loaded.gl.dropdown');
-        // eslint-disable-next-line no-jquery/no-fade
-        $loading.fadeOut();
+        $loading.addClass('gl-display-none');
         if (data.assignee) {
           user = {
             name: data.assignee.name,

app/services/system_hooks_service.rb

 # frozen_string_literal: true
 
 class SystemHooksService
+  BUILDER_DRIVEN_EVENT_DATA_AVAILABLE_FOR_CLASSES = [GroupMember].freeze
+
   def execute_hooks_for(model, event)
     data = build_event_data(model, event)
 
@@ -20,6 +22,9 @@ class SystemHooksService
   private
 
   def build_event_data(model, event)
+    # return entire event data from its builder class, if available.
+    return builder_driven_event_data(model, event) if builder_driven_event_data_available?(model)
+
     data = {
       event_name: build_event_name(model, event),
       created_at: model.created_at&.xmlschema,
@@ -62,8 +67,6 @@ class SystemHooksService
           old_full_path: model.full_path_before_last_save
         )
       end
-    when GroupMember
-      data.merge!(group_member_data(model))
     end
 
     data
@@ -75,10 +78,6 @@ class SystemHooksService
       return "user_add_to_team"      if event == :create
       return "user_remove_from_team" if event == :destroy
       return "user_update_for_team"  if event == :update
-    when GroupMember
-      return 'user_add_to_group'      if event == :create
-      return 'user_remove_from_group' if event == :destroy
-      return 'user_update_for_group'  if event == :update
     else
       "#{model.class.name.downcase}_#{event}"
     end
@@ -128,19 +127,6 @@ class SystemHooksService
     }
   end
 
-  def group_member_data(model)
-    {
-      group_name: model.group.name,
-      group_path: model.group.path,
-      group_id: model.group.id,
-      user_username: model.user.username,
-      user_name: model.user.name,
-      user_email: model.user.email,
-      user_id: model.user.id,
-      group_access: model.human_access
-    }
-  end
-
   def user_data(model)
     {
       name: model.name,
@@ -149,6 +135,17 @@ class SystemHooksService
       username: model.username
     }
   end
+
+  def builder_driven_event_data_available?(model)
+    model.class.in?(BUILDER_DRIVEN_EVENT_DATA_AVAILABLE_FOR_CLASSES)
+  end
+
+  def builder_driven_event_data(model, event)
+    case model
+    when GroupMember
+      Gitlab::HookData::GroupMemberBuilder.new(model).build(event)
+    end
+  end
 end
 
 SystemHooksService.prepend_if_ee('EE::SystemHooksService')

app/views/projects/diffs/_replaced_image_diff.html.haml

@@ -14,7 +14,7 @@
     .wrap
       .frame.deleted
         = image_tag(old_blob_raw_url, alt: diff_file.old_path, lazy: false)
-      %p.image-info.hide
+      %p.image-info.gl-display-none
         %span.meta-filesize= number_to_human_size(old_blob.size)
         |
         %strong W:
@@ -24,7 +24,7 @@
         %span.meta-height
     .wrap
       = render partial: "projects/diffs/image_diff_frame", locals: { class_name: "added js-image-frame #{class_name}", position: position, note_type: DiffNote.name, image_path: blob_raw_url, alt: diff_file.new_path }
-      %p.image-info.hide
+      %p.image-info.gl-display-none
         %span.meta-filesize= number_to_human_size(blob.size)
         |
         %strong W:
@@ -33,7 +33,7 @@
         %strong H:
         %span.meta-height
 
-  .swipe.view.hide
+  .swipe.view.gl-display-none
     .swipe-frame
       .frame.deleted.old-diff
         = image_tag(old_blob_raw_url, alt: diff_file.old_path, lazy: false)
@@ -43,7 +43,7 @@
         %span.top-handle
         %span.bottom-handle
 
-  .onion-skin.view.hide
+  .onion-skin.view.gl-display-none
     .onion-skin-frame
       .frame.deleted
         = image_tag(old_blob_raw_url, alt: diff_file.old_path, lazy: false)
@@ -54,7 +54,7 @@
           .dragger{ :style => "left: 0px;" }
         .opaque
 
-.view-modes.hide
+.view-modes.gl-display-none
   %ul.view-modes-menu
     %li.two-up{ data: { mode: 'two-up' } } 2-up
     %li.swipe{ data: { mode: 'swipe' } } Swipe

changelogs/unreleased/sh-update-fog-3-7-0.yml

+---
+title: Upgrade fog-aws to v3.7.0
+merge_request: 48921
+author:
+type: changed

doc/administration/auth/ldap/index.md

@@ -175,7 +175,7 @@ production:
 | `password` | The password of the bind user. | no | `'your_great_password'` |
 | `encryption` | Encryption method. The `method` key is deprecated in favor of `encryption`. | yes | `'start_tls'` or `'simple_tls'` or `'plain'` |
 | `verify_certificates` | Enables SSL certificate verification if encryption method is `start_tls` or `simple_tls`. Defaults to true. | no | boolean |
-| `timeout` | Set a timeout, in seconds, for LDAP queries. This helps avoid blocking a request if the LDAP server becomes unresponsive. A value of 0 means there is no timeout. | no | `10` or `30` |
+| `timeout` | Set a timeout, in seconds, for LDAP queries. This helps avoid blocking a request if the LDAP server becomes unresponsive. A value of `0` means there is no timeout. (default: `10`) | no | `10` or `30` |
 | `active_directory` | This setting specifies if LDAP server is Active Directory LDAP server. For non-AD servers it skips the AD specific queries. If your LDAP server is not AD, set this to false. | no | boolean |
 | `allow_username_or_email_login` | If enabled, GitLab ignores everything after the first `@` in the LDAP username submitted by the user on sign-in. If you are using `uid: 'userPrincipalName'` on ActiveDirectory you need to disable this setting, because the userPrincipalName contains an `@`. | no | boolean |
 | `block_auto_created_users` | To maintain tight control over the number of billable users on your GitLab installation, enable this setting to keep new users blocked until they have been cleared by an administrator (default: false). | no | boolean |

doc/development/README.md

@@ -135,6 +135,7 @@ from:
 - [Wikis development guide](wikis.md)
 - [Newlines style guide](newlines_styleguide.md)
 - [Image scaling guide](image_scaling.md)
+- [Export to CSV](export_csv.md)
 
 ## Performance guides
 

doc/development/product_analytics/usage_ping.md

@@ -395,7 +395,9 @@ Implemented using Redis methods [PFADD](https://redis.io/commands/pfadd) and [PF
 
    API requests are protected by checking for a valid CSRF token.
 
-   In order to be able to increment the values the related feature `usage_data<event_name>` should be enabled.
+   In order to increment the values, the related feature `usage_data_<event_name>` should be
+   set to `default_enabled: true`. For more information, see
+   [Feature flags in development of GitLab](../feature_flags/index.md).
 
    ```plaintext
    POST /usage_data/increment_unique_users
@@ -418,7 +420,10 @@ Implemented using Redis methods [PFADD](https://redis.io/commands/pfadd) and [PF
 
    Example usage for an existing event already defined in [known events](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/usage_data_counters/known_events/):
 
-   Note that `usage_data_api` and `usage_data_#{event_name}` should be enabled in order to be able to track events
+   Usage Data API is behind  `usage_data_api` feature flag which, as of GitLab 13.7, is
+   now set to `default_enabled: true`.
+
+   Each event tracked using Usage Data API is behind a feature flag `usage_data_#{event_name}` which should be `default_enabled: true`
 
    ```javascript
    import api from '~/api';

ee/app/assets/javascripts/weight_select.js

@@ -11,8 +11,7 @@ function WeightSelect(els, options = {}) {
     const $selectbox = $dropdown.closest('.selectbox');
     const $block = $selectbox.closest('.block');
     const $value = $block.find('.value');
-    // eslint-disable-next-line no-jquery/no-fade
-    $block.find('.block-loading').fadeOut();
+    $block.find('.block-loading').addClass('gl-display-none');
     const fieldName = options.fieldName || $dropdown.data('fieldName');
     const inputField = $dropdown.closest('.selectbox').find(`input[name='${fieldName}']`);
 

ee/app/services/ee/system_hooks_service.rb

@@ -6,13 +6,6 @@ module EE
 
     private
 
-    override :group_member_data
-    def group_member_data(model)
-      super.tap do |data|
-        data[:group_plan] = model.group.gitlab_subscription&.plan_name
-      end
-    end
-
     override :user_data
     def user_data(model)
       super.tap do |data|

ee/lib/ee/gitlab/hook_data/group_member_builder.rb

+# frozen_string_literal: true
+module EE
+  module Gitlab
+    module HookData
+      module GroupMemberBuilder
+        extend ::Gitlab::Utils::Override
+
+        private
+
+        override :group_member_data
+        def group_member_data
+          super.tap do |data|
+            data[:group_plan] = group_member.group.gitlab_subscription&.plan_name
+          end
+        end
+      end
+    end
+  end
+end

ee/spec/lib/ee/gitlab/hook_data/group_member_builder_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+RSpec.describe Gitlab::HookData::GroupMemberBuilder do
+  let(:group_member) { create(:group_member, :developer, group: group) }
+
+  describe '#build' do
+    let(:event) { :create }
+    let(:data) { described_class.new(group_member).build(event) }
+
+    context 'data' do
+      context 'group_plan attribute' do
+        let(:group) { create(:group_with_plan, plan: :gold_plan) }
+
+        it 'returns correct group_plan' do
+          expect(data).to include(:group_plan)
+          expect(data[:group_plan]).to eq('gold')
+        end
+      end
+    end
+  end
+end

lib/gitlab/hook_data/base_builder.rb

@@ -21,6 +21,13 @@ module Gitlab
 
       private
 
+      def timestamps_data
+        {
+          created_at: object.created_at&.xmlschema,
+          updated_at: object.updated_at&.xmlschema
+        }
+      end
+
       def absolute_image_urls(markdown_text)
         return markdown_text unless markdown_text.present?
 

lib/gitlab/hook_data/group_member_builder.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module HookData
+    class GroupMemberBuilder < BaseBuilder
+      alias_method :group_member, :object
+
+      # Sample data
+
+      # {
+      #   :event_name=>"user_add_to_group",
+      #   :group_name=>"GitLab group",
+      #   :group_path=>"gitlab",
+      #   :group_id=>1,
+      #   :user_username=>"robert",
+      #   :user_name=>"Robert Mills",
+      #   :user_email=>"robert@example.com",
+      #   :user_id=>14,
+      #   :group_access=>"Guest",
+      #   :created_at=>"2020-11-04T10:12:10Z",
+      #   :updated_at=>"2020-11-04T10:12:10Z",
+      # }
+
+      def build(event)
+        [
+          timestamps_data,
+          group_member_data,
+          event_data(event)
+        ].reduce(:merge)
+      end
+
+      private
+
+      def group_member_data
+        {
+          group_name: group_member.group.name,
+          group_path: group_member.group.path,
+          group_id: group_member.group.id,
+          user_username: group_member.user.username,
+          user_name: group_member.user.name,
+          user_email: group_member.user.email,
+          user_id: group_member.user.id,
+          group_access: group_member.human_access
+        }
+      end
+
+      def event_data(event)
+        event_name =  case event
+                      when :create
+                        'user_add_to_group'
+                      when :destroy
+                        'user_remove_from_group'
+                      when :update
+                        'user_update_for_group'
+                      end
+
+        { event_name: event_name }
+      end
+    end
+  end
+end
+
+Gitlab::HookData::GroupMemberBuilder.prepend_if_ee('EE::Gitlab::HookData::GroupMemberBuilder')

spec/lib/gitlab/hook_data/group_member_builder_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::HookData::GroupMemberBuilder do
+  let_it_be(:group) { create(:group) }
+  let_it_be(:group_member) { create(:group_member, :developer, group: group) }
+
+  describe '#build' do
+    let(:data) { described_class.new(group_member).build(event) }
+    let(:event_name) { data[:event_name] }
+    let(:attributes) do
+      [
+        :event_name, :created_at, :updated_at, :group_name, :group_path,
+        :group_id, :user_id, :user_username, :user_name, :user_email, :group_access
+      ]
+    end
+
+    context 'data' do
+      shared_examples_for 'includes the required attributes' do
+        it 'includes the required attributes' do
+          expect(data).to include(*attributes)
+
+          expect(data[:group_name]).to eq(group.name)
+          expect(data[:group_path]).to eq(group.path)
+          expect(data[:group_id]).to eq(group.id)
+          expect(data[:user_username]).to eq(group_member.user.username)
+          expect(data[:user_name]).to eq(group_member.user.name)
+          expect(data[:user_email]).to eq(group_member.user.email)
+          expect(data[:user_id]).to eq(group_member.user.id)
+          expect(data[:group_access]).to eq('Developer')
+          expect(data[:created_at]).to eq(group_member.created_at&.xmlschema)
+          expect(data[:updated_at]).to eq(group_member.updated_at&.xmlschema)
+        end
+      end
+
+      context 'on create' do
+        let(:event) { :create }
+
+        it { expect(event_name).to eq('user_add_to_group') }
+        it_behaves_like 'includes the required attributes'
+      end
+
+      context 'on update' do
+        let(:event) { :update }
+
+        it { expect(event_name).to eq('user_update_for_group') }
+        it_behaves_like 'includes the required attributes'
+      end
+
+      context 'on destroy' do
+        let(:event) { :destroy }
+
+        it { expect(event_name).to eq('user_remove_from_group') }
+        it_behaves_like 'includes the required attributes'
+      end
+    end
+  end
+end

spec/services/system_hooks_service_spec.rb

@@ -159,9 +159,6 @@ RSpec.describe SystemHooksService do
     it { expect(event_name(group, :create)).to eq 'group_create' }
     it { expect(event_name(group, :destroy)).to eq 'group_destroy' }
     it { expect(event_name(group, :rename)).to eq 'group_rename' }
-    it { expect(event_name(group_member, :create)).to eq 'user_add_to_group' }
-    it { expect(event_name(group_member, :destroy)).to eq 'user_remove_from_group' }
-    it { expect(event_name(group_member, :update)).to eq 'user_update_for_group' }
   end
 
   def event_data(*args)