Merge branch 'ce-to-ee-2018-09-03' into 'master'

CE upstream - 2018-09-03 15:21 UTC

See merge request gitlab-org/gitlab-ee!7213

app/assets/javascripts/badges/components/badge_form.vue

@@ -23,6 +23,11 @@ export default {
       required: true,
     },
   },
+  data() {
+    return {
+      wasValidated: false,
+    };
+  },
   computed: {
     ...mapState([
       'badgeInAddForm',
@@ -39,16 +44,6 @@ export default {
 
       return this.badgeInAddForm;
     },
-    canSubmit() {
-      return (
-        this.badge !== null &&
-        this.badge.imageUrl &&
-        this.badge.imageUrl.trim() !== '' &&
-        this.badge.linkUrl &&
-        this.badge.linkUrl.trim() !== '' &&
-        !this.isSaving
-      );
-    },
     helpText() {
       const placeholders = ['project_path', 'project_id', 'default_branch', 'commit_sha']
         .map(placeholder => `<code>%{${placeholder}}</code>`)
@@ -93,11 +88,18 @@ export default {
         });
       },
     },
-    submitButtonLabel() {
-      if (this.isEditing) {
-        return s__('Badges|Save changes');
-      }
-      return s__('Badges|Add badge');
+    badgeImageUrlExample() {
+      const exampleUrl =
+        'https://example.gitlab.com/%{project_path}/badges/%{default_branch}/badge.svg';
+      return sprintf(s__('Badges|e.g. %{exampleUrl}'), {
+        exampleUrl,
+      });
+    },
+    badgeLinkUrlExample() {
+      const exampleUrl = 'https://example.gitlab.com/%{project_path}';
+      return sprintf(s__('Badges|e.g. %{exampleUrl}'), {
+        exampleUrl,
+      });
     },
   },
   methods: {
@@ -109,7 +111,9 @@ export default {
       this.stopEditing();
     },
     onSubmit() {
-      if (!this.canSubmit) {
+      const form = this.$el;
+      if (!form.checkValidity()) {
+        this.wasValidated = true;
         return Promise.resolve();
       }
 
@@ -117,6 +121,7 @@ export default {
         return this.saveBadge()
           .then(() => {
             createFlash(s__('Badges|The badge was saved.'), 'notice');
+            this.wasValidated = false;
           })
           .catch(error => {
             createFlash(
@@ -129,6 +134,7 @@ export default {
       return this.addBadge()
         .then(() => {
           createFlash(s__('Badges|A new badge was added.'), 'notice');
+          this.wasValidated = false;
         })
         .catch(error => {
           createFlash(
@@ -138,47 +144,58 @@ export default {
         });
     },
   },
-  badgeImageUrlPlaceholder:
-    'https://example.gitlab.com/%{project_path}/badges/%{default_branch}/<badge>.svg',
-  badgeLinkUrlPlaceholder: 'https://example.gitlab.com/%{project_path}',
 };
 </script>
 
 <template>
   <form
-    class="prepend-top-default append-bottom-default"
+    :class="{ 'was-validated': wasValidated }"
+    class="prepend-top-default append-bottom-default needs-validation"
+    novalidate
     @submit.prevent.stop="onSubmit"
   >
     <div class="form-group">
-      <label for="badge-link-url">{{ s__('Badges|Link') }}</label>
+      <label
+        for="badge-link-url"
+        class="label-bold"
+      >{{ s__('Badges|Link') }}</label>
+      <p v-html="helpText"></p>
       <input
         id="badge-link-url"
         v-model="linkUrl"
-        :placeholder="$options.badgeLinkUrlPlaceholder"
-        type="text"
+        type="URL"
         class="form-control"
+        required
         @input="debouncedPreview"
       />
-      <span
-        class="form-text text-muted"
-        v-html="helpText"
-      ></span>
+      <div class="invalid-feedback">
+        {{ s__('Badges|Please fill in a valid URL') }}
+      </div>
+      <span class="form-text text-muted">
+        {{ badgeLinkUrlExample }}
+      </span>
     </div>
 
     <div class="form-group">
-      <label for="badge-image-url">{{ s__('Badges|Badge image URL') }}</label>
+      <label
+        for="badge-image-url"
+        class="label-bold"
+      >{{ s__('Badges|Badge image URL') }}</label>
+      <p v-html="helpText"></p>
       <input
         id="badge-image-url"
         v-model="imageUrl"
-        :placeholder="$options.badgeImageUrlPlaceholder"
-        type="text"
+        type="URL"
         class="form-control"
+        required
         @input="debouncedPreview"
       />
-      <span
-        class="form-text text-muted"
-        v-html="helpText"
-      ></span>
+      <div class="invalid-feedback">
+        {{ s__('Badges|Please fill in a valid URL') }}
+      </div>
+      <span class="form-text text-muted">
+        {{ badgeImageUrlExample }}
+      </span>
     </div>
 
     <div class="form-group">
@@ -200,20 +217,32 @@ export default {
       >{{ s__('Badges|No image to preview') }}</p>
     </div>
 
-    <div class="row-content-block">
+    <div
+      v-if="isEditing"
+      class="row-content-block"
+    >
       <loading-button
-        :disabled="!canSubmit"
         :loading="isSaving"
-        :label="submitButtonLabel"
+        :label="s__('Badges|Save changes')"
         type="submit"
         container-class="btn btn-success"
       />
       <button
-        v-if="isEditing"
         class="btn btn-cancel"
         type="button"
         @click="onCancel"
       >{{ __('Cancel') }}</button>
     </div>
+    <div
+      v-else
+      class="form-group"
+    >
+      <loading-button
+        :loading="isSaving"
+        :label="s__('Badges|Add badge')"
+        type="submit"
+        container-class="btn btn-success"
+      />
+    </div>
   </form>
 </template>

app/assets/javascripts/badges/components/badge_list.vue

@@ -28,7 +28,7 @@ export default {
       {{ s__('Badges|Your badges') }}
       <span
         v-show="!isLoading"
-        class="badge"
+        class="badge badge-pill"
       >{{ badges.length }}</span>
     </div>
     <loading-icon

app/assets/javascripts/badges/components/badge_list_row.vue

@@ -43,13 +43,13 @@ export default {
     <badge
       :image-url="badge.renderedImageUrl"
       :link-url="badge.renderedLinkUrl"
-      class="table-section section-30"
+      class="table-section section-40"
     />
-    <span class="table-section section-50 str-truncated">{{ badge.linkUrl }}</span>
-    <div class="table-section section-10">
-      <span class="badge">{{ badgeKindText }}</span>
+    <span class="table-section section-30 str-truncated">{{ badge.linkUrl }}</span>
+    <div class="table-section section-15">
+      <span class="badge badge-pill">{{ badgeKindText }}</span>
     </div>
-    <div class="table-section section-10 table-button-footer">
+    <div class="table-section section-15 table-button-footer">
       <div
         v-if="canEditBadge"
         class="table-action-buttons">

app/assets/javascripts/pages/groups/edit/index.js

@@ -2,14 +2,13 @@ import groupAvatar from '~/group_avatar';
 import TransferDropdown from '~/groups/transfer_dropdown';
 import initConfirmDangerModal from '~/confirm_danger_modal';
 import initSettingsPanels from '~/settings_panels';
+import mountBadgeSettings from '~/pages/shared/mount_badge_settings';
+import { GROUP_BADGE } from '~/badges/constants';
 
 document.addEventListener('DOMContentLoaded', () => {
   groupAvatar();
   new TransferDropdown(); // eslint-disable-line no-new
   initConfirmDangerModal();
-});
-
-document.addEventListener('DOMContentLoaded', () => {
-  // Initialize expandable settings panels
   initSettingsPanels();
+  mountBadgeSettings(GROUP_BADGE);
 });

app/assets/javascripts/pages/projects/edit/index.js

+import { PROJECT_BADGE } from '~/badges/constants';
 import initSettingsPanels from '~/settings_panels';
 import setupProjectEdit from '~/project_edit';
 import initConfirmDangerModal from '~/confirm_danger_modal';
+import mountBadgeSettings from '~/pages/shared/mount_badge_settings';
 import initProjectLoadingSpinner from '../shared/save_project_loader';
 import projectAvatar from '../shared/project_avatar';
 import initProjectPermissionsSettings from '../shared/permissions';
@@ -13,4 +15,5 @@ document.addEventListener('DOMContentLoaded', () => {
   projectAvatar();
   initProjectPermissionsSettings();
   initConfirmDangerModal();
+  mountBadgeSettings(PROJECT_BADGE);
 });

app/assets/javascripts/pages/projects/settings/badges/index/index.js

-import Vue from 'vue';
-import Translate from '~/vue_shared/translate';
-import { PROJECT_BADGE } from '~/badges/constants';
-import mountBadgeSettings from '~/pages/shared/mount_badge_settings';
-
-Vue.use(Translate);
-
-document.addEventListener('DOMContentLoaded', () => {
-  mountBadgeSettings(PROJECT_BADGE);
-});

app/controllers/groups/settings/badges_controller.rb

-module Groups
-  module Settings
-    class BadgesController < Groups::ApplicationController
-      include API::Helpers::RelatedResourcesHelpers
-
-      before_action :authorize_admin_group!
-
-      def index
-        @badge_api_endpoint = expose_url(api_v4_groups_badges_path(id: @group.id))
-      end
-    end
-  end
-end

app/controllers/groups_controller.rb

 class GroupsController < Groups::ApplicationController
+  include API::Helpers::RelatedResourcesHelpers
   include IssuesAction
   include MergeRequestsAction
   include ParamsBackwardCompatibility
@@ -78,6 +79,7 @@ class GroupsController < Groups::ApplicationController
   end
 
   def edit
+    @badge_api_endpoint = expose_url(api_v4_groups_badges_path(id: @group.id))
   end
 
   def projects

app/controllers/projects/settings/badges_controller.rb

-module Projects
-  module Settings
-    class BadgesController < Projects::ApplicationController
-      include API::Helpers::RelatedResourcesHelpers
-
-      before_action :authorize_admin_project!
-
-      def index
-        @badge_api_endpoint = expose_url(api_v4_projects_badges_path(id: @project.id))
-      end
-    end
-  end
-end

app/controllers/projects_controller.rb

 class ProjectsController < Projects::ApplicationController
+  include API::Helpers::RelatedResourcesHelpers
   include IssuableCollections
   include ExtractsPath
   include PreviewMarkdown
@@ -34,6 +35,7 @@ class ProjectsController < Projects::ApplicationController
   end
 
   def edit
+    @badge_api_endpoint = expose_url(api_v4_projects_badges_path(id: @project.id))
     render 'edit'
   end
 

app/views/groups/edit.html.haml

@@ -25,6 +25,18 @@
   .settings-content
     = render 'groups/settings/permissions'
 
+%section.settings.no-animate{ class: ('expanded' if expanded) }
+  .settings-header
+    %h4
+      = s_('GroupSettings|Badges')
+    %button.btn.js-settings-toggle{ type: 'button' }
+      = expanded ? 'Collapse' : 'Expand'
+    %p
+      = s_('GroupSettings|Customize your group badges.')
+      = link_to s_('GroupSettings|Learn more about badges.'), help_page_path('user/project/badges')
+  .settings-content
+    = render 'shared/badges/badge_settings'
+
 %section.settings.gs-advanced.no-animate#js-advanced-settings{ class: ('expanded' if expanded) }
   .settings-header
     %h4

app/views/layouts/nav/sidebar/_group.html.haml

@@ -130,12 +130,6 @@
                 %span
                   = _('General')
 
-            = nav_link(controller: :badges) do
-              = link_to group_settings_badges_path(@group), title: _('Project Badges') do
-                %span
-                  = _('Project Badges')
-
-
             = nav_link(path: 'groups#projects') do
               = link_to projects_group_path(@group), title: _('Projects') do
                 %span

app/views/layouts/nav/sidebar/_project.html.haml

@@ -325,11 +325,6 @@
               = link_to project_project_members_path(@project), title: _('Members') do
                 %span
                   = _('Members')
-            - if can_edit
-              = nav_link(controller: :badges) do
-                = link_to project_settings_badges_path(@project), title: _('Badges') do
-                  %span
-                    = _('Badges')
             - if can_edit
               = nav_link(controller: [:integrations, :services, :hooks, :hook_logs]) do
                 = link_to project_settings_integrations_path(@project), title: _('Integrations') do

app/views/projects/edit.html.haml

@@ -102,6 +102,18 @@
 
   = render_if_exists 'projects/service_desk_settings'
 
+  %section.settings.no-animate{ class: ('expanded' if expanded) }
+    .settings-header
+      %h4
+        = s_('ProjectSettings|Badges')
+      %button.btn.js-settings-toggle{ type: 'button' }
+        = expanded ? 'Collapse' : 'Expand'
+      %p
+        = s_('ProjectSettings|Customize your project badges.')
+        = link_to s_('ProjectSettings|Learn more about badges.'), help_page_path('user/project/badges')
+    .settings-content
+      = render 'shared/badges/badge_settings'
+
   = render 'export', project: @project
 
   %section.qa-advanced-settings.settings.advanced-settings.no-animate#js-project-advanced-settings{ class: ('expanded' if expanded) }

changelogs/unreleased/sh-improve-bitbucket-server-logging.yml

+---
+title: Add JSON logging for Bitbucket Server importer
+merge_request: 21378
+author:
+type: other

changelogs/unreleased/winh-move-badge-settings.yml

+---
+title: Move badge settings to general settings
+merge_request: 21333
+author:
+type: changed

config/routes/group.rb

@@ -26,7 +26,6 @@ constraints(::Constraints::GroupUrlConstrainer.new) do
         constraints: { group_id: Gitlab::PathRegex.full_namespace_route_regex }) do
     namespace :settings do
       resource :ci_cd, only: [:show], controller: 'ci_cd'
-      resources :badges, only: [:index]
     end
 
     resource :variables, only: [:show, :update]

config/routes/project.rb

@@ -526,7 +526,6 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
         resource :repository, only: [:show], controller: :repository do
           post :create_deploy_token, path: 'deploy_token/create'
         end
-        resources :badges, only: [:index]
       end
 
       # Since both wiki and repository routing contains wildcard characters

doc/administration/logs.md

@@ -219,6 +219,15 @@ installations from source.
 
 It logs information whenever a [repository check is run][repocheck] on a project.
 
+## `importer.log`
+
+Introduced in GitLab 11.3. This file lives in `/var/log/gitlab/gitlab-rails/importer.log` for
+Omnibus GitLab packages or in `/home/git/gitlab/log/importer.log` for
+installations from source.
+
+Currently it logs the progress of project imports from the Bitbucket Server
+importer. Future importers may use this file.
+
 ## Reconfigure Logs
 
 Reconfigure log files live in `/var/log/gitlab/reconfigure` for Omnibus GitLab 

doc/api/settings.md

@@ -280,5 +280,5 @@ are listed in the descriptions of the relevant settings.
 | `usage_ping_enabled`                     | boolean          | no                                   | Every week GitLab will report license usage back to GitLab, Inc. |
 | `user_default_external`                  | boolean          | no                                   | Newly registered users will be external by default. |
 | `user_oauth_applications`                | boolean          | no                                   | Allow users to register any application to use GitLab as an OAuth provider. |
-| `user_show_add_ssh_key_message`          | boolean          | no                                   | When set to `false`, disable the "You won't be able to pull or push project code via SSH" warning shown to users with no uploaded SSH key. |
+| `user_show_add_ssh_key_message`          | boolean          | no                                   | When set to `false` disable the "You won't be able to pull or push project code via SSH" warning shown to users with no uploaded SSH key. |
 | `version_check_enabled`                  | boolean          | no                                   | Let GitLab inform you when an update is available. |

doc/user/project/badges.md

@@ -17,7 +17,7 @@ If you find that you have to add the same badges to several projects, you may wa
 
 To add a new badge to a project:
 
-1.  Navigate to your project's **Settings > Badges**.
+1.  Navigate to your project's **Settings > General > Badges**.
 1.  Under "Link", enter the URL that the badges should point to and under
     "Badge image URL" the URL of the image that should be displayed.
 1.  Submit the badge by clicking the **Add badge** button.
@@ -39,7 +39,7 @@ project, consider adding them on the [project level](#project-badges) or use
 
 To add a new badge to a group:
 
-1.  Navigate to your group's **Settings > Project Badges**.
+1.  Navigate to your group's **Settings > General > Badges**.
 1.  Under "Link", enter the URL that the badges should point to and under
     "Badge image URL" the URL of the image that should be displayed.
 1.  Submit the badge by clicking the **Add badge** button.

lib/gitlab/bitbucket_server_import/importer.rb

@@ -7,6 +7,7 @@ module Gitlab
 
       attr_reader :recover_missing_commits
       attr_reader :project, :project_key, :repository_slug, :client, :errors, :users
+      attr_accessor :logger
 
       REMOTE_NAME = 'bitbucket_server'.freeze
       BATCH_SIZE = 100
@@ -36,6 +37,7 @@ module Gitlab
         @errors = []
         @users = {}
         @temp_branches = []
+        @logger = Gitlab::Import::Logger.build
       end
 
       def execute
@@ -44,6 +46,8 @@ module Gitlab
         delete_temp_branches
         handle_errors
 
+        log_info(stage: "complete")
+
         true
       end
 
@@ -118,15 +122,21 @@ module Gitlab
             client.create_branch(project_key, repository_slug, branch_name, sha)
             branches_created << temp_branch
           rescue BitbucketServer::Connection::ConnectionError => e
-            Rails.logger.warn("BitbucketServerImporter: Unable to recreate branch for SHA #{sha}: #{e}")
+            log_warn(message: "Unable to recreate branch", sha: sha, error: e.message)
           end
         end
       end
 
       def import_repository
+        log_info(stage: 'import_repository', message: 'starting import')
+
         project.ensure_repository
         project.repository.fetch_as_mirror(project.import_url, refmap: self.class.refmap, remote_name: REMOTE_NAME)
+
+        log_info(stage: 'import_repository', message: 'finished import')
       rescue Gitlab::Shell::Error, Gitlab::Git::RepositoryMirroring::RemoteError => e
+        log_error(stage: 'import_repository', message: 'failed import', error: e.message)
+
         # Expire cache to prevent scenarios such as:
         # 1. First import failed, but the repo was imported successfully, so +exists?+ returns true
         # 2. Retried import, repo is broken or not imported but +exists?+ still returns true
@@ -157,7 +167,10 @@ module Gitlab
             begin
               import_bitbucket_pull_request(pull_request)
             rescue StandardError => e
-              errors << { type: :pull_request, iid: pull_request.iid, errors: e.message, trace: e.backtrace.join("\n"), raw_response: pull_request.raw }
+              backtrace = Gitlab::Profiler.clean_backtrace(e.backtrace)
+              log_error(stage: 'import_pull_requests', iid: pull_request.iid, error: e.message, backtrace: backtrace)
+
+              errors << { type: :pull_request, iid: pull_request.iid, errors: e.message, backtrace: backtrace.join("\n"), raw_response: pull_request.raw }
             end
           end
         end
@@ -169,12 +182,15 @@ module Gitlab
             client.delete_branch(project_key, repository_slug, branch.name, branch.sha)
             project.repository.delete_branch(branch.name)
           rescue BitbucketServer::Connection::ConnectionError => e
+            log_error(stage: 'delete_temp_branches', branch: branch.name, error: e.message)
             @errors << { type: :delete_temp_branches, branch_name: branch.name, errors: e.message }
           end
         end
       end
 
       def import_bitbucket_pull_request(pull_request)
+        log_info(stage: 'import_bitbucket_pull_requests', message: 'starting', iid: pull_request.iid)
+
         description = ''
         description += @formatter.author_line(pull_request.author) unless find_user_id(pull_request.author_email)
         description += pull_request.description if pull_request.description
@@ -201,9 +217,13 @@ module Gitlab
         merge_request = creator.execute(attributes)
 
         import_pull_request_comments(pull_request, merge_request) if merge_request.persisted?
+
+        log_info(stage: 'import_bitbucket_pull_requests', message: 'finished', iid: pull_request.iid)
       end
 
       def import_pull_request_comments(pull_request, merge_request)
+        log_info(stage: 'import_pull_request_comments', message: 'starting', iid: merge_request.iid)
+
         comments, other_activities = client.activities(project_key, repository_slug, pull_request.iid).partition(&:comment?)
 
         merge_event = other_activities.find(&:merge_event?)
@@ -213,9 +233,16 @@ module Gitlab
 
         import_inline_comments(inline_comments.map(&:comment), merge_request)
         import_standalone_pr_comments(pr_comments.map(&:comment), merge_request)
+
+        log_info(stage: 'import_pull_request_comments', message: 'finished', iid: merge_request.iid,
+                 merge_event_found: merge_event.present?,
+                 inline_comments_count: inline_comments.count,
+                 standalone_pr_comments: pr_comments.count)
       end
 
       def import_merge_event(merge_request, merge_event)
+        log_info(stage: 'import_merge_event', message: 'starting', iid: merge_request.iid)
+
         committer = merge_event.committer_email
 
         user_id = gitlab_user_id(committer)
@@ -223,9 +250,13 @@ module Gitlab
         merge_request.update({ merge_commit_sha: merge_event.merge_commit })
         metric = MergeRequest::Metrics.find_or_initialize_by(merge_request: merge_request)
         metric.update(merged_by_id: user_id, merged_at: timestamp)
+
+        log_info(stage: 'import_merge_event', message: 'finished', iid: merge_request.iid)
       end
 
       def import_inline_comments(inline_comments, merge_request)
+        log_info(stage: 'import_inline_comments', message: 'starting', iid: merge_request.iid)
+
         inline_comments.each do |comment|
           position = build_position(merge_request, comment)
           parent = create_diff_note(merge_request, comment, position)
@@ -238,6 +269,8 @@ module Gitlab
             create_diff_note(merge_request, reply, position, discussion_id)
           end
         end
+
+        log_info(stage: 'import_inline_comments', message: 'finished', iid: merge_request.iid)
       end
 
       def create_diff_note(merge_request, comment, position, discussion_id = nil)
@@ -252,11 +285,14 @@ module Gitlab
           return note
         end
 
+        log_info(stage: 'create_diff_note', message: 'creating fallback DiffNote', iid: merge_request.iid)
+
         # Bitbucket Server supports the ability to comment on any line, not just the
         # line in the diff. If we can't add the note as a DiffNote, fallback to creating
         # a regular note.
         create_fallback_diff_note(merge_request, comment, position)
       rescue StandardError => e
+        log_error(stage: 'create_diff_note', comment_id: comment.id, error: e.message)
         errors << { type: :pull_request, id: comment.id, errors: e.message }
         nil
       end
@@ -294,7 +330,8 @@ module Gitlab
               merge_request.notes.create!(pull_request_comment_attributes(replies))
             end
           rescue StandardError => e
-            errors << { type: :pull_request, iid: comment.id, errors: e.message }
+            log_error(stage: 'import_standalone_pr_comments', merge_request_id: merge_request.id, comment_id: comment.id, error: e.message)
+            errors << { type: :pull_request, comment_id: comment.id, errors: e.message }
           end
         end
       end
@@ -324,6 +361,26 @@ module Gitlab
           updated_at: comment.updated_at
         }
       end
+
+      def log_info(details)
+        logger.info(log_base_data.merge(details))
+      end
+
+      def log_error(details)
+        logger.error(log_base_data.merge(details))
+      end
+
+      def log_warn(details)
+        logger.warn(log_base_data.merge(details))
+      end
+
+      def log_base_data
+        {
+          class: self.class.name,
+          project_id: project.id,
+          project_path: project.full_path
+        }
+      end
     end
   end
 end

lib/gitlab/import/logger.rb

+module Gitlab
+  module Import
+    class Logger < ::Gitlab::JsonLogger
+      def self.file_name_noext
+        'importer'
+      end
+    end
+  end
+end

locale/gitlab.pot

@@ -957,6 +957,9 @@ msgstr ""
 msgid "Badges|No image to preview"
 msgstr ""
 
+msgid "Badges|Please fill in a valid URL"
+msgstr ""
+
 msgid "Badges|Project Badge"
 msgstr ""
 
@@ -990,6 +993,9 @@ msgstr ""
 msgid "Badges|Your badges"
 msgstr ""
 
+msgid "Badges|e.g. %{exampleUrl}"
+msgstr ""
+
 msgid "Begin with the selected commit"
 msgstr ""
 
@@ -3721,6 +3727,15 @@ msgstr ""
 msgid "GroupRoadmap|Until %{dateWord}"
 msgstr ""
 
+msgid "GroupSettings|Badges"
+msgstr ""
+
+msgid "GroupSettings|Customize your group badges."
+msgstr ""
+
+msgid "GroupSettings|Learn more about badges."
+msgstr ""
+
 msgid "GroupSettings|Prevent sharing a project within %{group} with other groups"
 msgstr ""
 
@@ -5689,15 +5704,24 @@ msgstr ""
 msgid "ProjectPage|Project ID: %{project_id}"
 msgstr ""
 
+msgid "ProjectSettings|Badges"
+msgstr ""
+
 msgid "ProjectSettings|Contact an admin to change this setting."
 msgstr ""
 
+msgid "ProjectSettings|Customize your project badges."
+msgstr ""
+
 msgid "ProjectSettings|Failed to protect the tag"
 msgstr ""
 
 msgid "ProjectSettings|Failed to update tag!"
 msgstr ""
 
+msgid "ProjectSettings|Learn more about badges."
+msgstr ""
+
 msgid "ProjectSettings|Only signed commits can be pushed to this repository."
 msgstr ""
 

spec/controllers/groups_controller_spec.rb

@@ -57,6 +57,16 @@ describe GroupsController do
     end
   end
 
+  describe 'GET edit' do
+    it 'sets the badge API endpoint' do
+      sign_in(owner)
+
+      get :edit, id: group.to_param
+
+      expect(assigns(:badge_api_endpoint)).not_to be_nil
+    end
+  end
+
   describe 'GET #new' do
     context 'when creating subgroups', :nested_groups do
       [true, false].each do |can_create_group_status|

spec/controllers/projects_controller_spec.rb

@@ -307,6 +307,19 @@ describe ProjectsController do
     end
   end
 
+  describe 'GET edit' do
+    it 'sets the badge API endpoint' do
+      sign_in(user)
+      project.add_maintainer(user)
+
+      get :edit,
+          namespace_id: project.namespace.path,
+          id: project.path
+
+      expect(assigns(:badge_api_endpoint)).not_to be_nil
+    end
+  end
+
   describe "#update" do
     render_views
 

spec/features/groups/settings/group_badges_spec.rb

@@ -14,7 +14,7 @@ describe 'Group Badges' do
     group.add_owner(user)
     sign_in(user)
 
-    visit(group_settings_badges_path(group))
+    visit(edit_group_path(group))
   end
 
   it 'shows a list of badges', :js do

spec/features/projects/settings/project_badges_spec.rb

@@ -15,7 +15,7 @@ describe 'Project Badges' do
     group.add_maintainer(user)
     sign_in(user)
 
-    visit(project_settings_badges_path(project))
+    visit(edit_project_path(project))
   end
 
   it 'shows a list of badges', :js do

spec/javascripts/badges/components/badge_form_spec.js

 import Vue from 'vue';
+import MockAdapter from 'axios-mock-adapter';
+import axios from '~/lib/utils/axios_utils';
 import store from '~/badges/store';
+import createEmptyBadge from '~/badges/empty_badge';
 import BadgeForm from '~/badges/components/badge_form.vue';
 import { mountComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
-import { createDummyBadge } from '../dummy_badge';
+import { DUMMY_IMAGE_URL, TEST_HOST } from '../../test_constants';
+
+// avoid preview background process
+BadgeForm.methods.debouncedPreview = () => {};
 
 describe('BadgeForm component', () => {
   const Component = Vue.extend(BadgeForm);
+  let axiosMock;
   let vm;
 
   beforeEach(() => {
     setFixtures(`
       <div id="dummy-element"></div>
     `);
+
+    axiosMock = new MockAdapter(axios);
   });
 
   afterEach(() => {
     vm.$destroy();
+    axiosMock.restore();
   });
 
   describe('methods', () => {
@@ -38,93 +48,86 @@ describe('BadgeForm component', () => {
         expect(vm.stopEditing).toHaveBeenCalled();
       });
     });
+  });
 
-    describe('onSubmit', () => {
-      describe('if isEditing is true', () => {
-        beforeEach(() => {
-          spyOn(vm, 'saveBadge').and.returnValue(Promise.resolve());
-          store.replaceState({
-            ...store.state,
-            isSaving: false,
-            badgeInEditForm: createDummyBadge(),
-          });
-          vm.isEditing = true;
-        });
-
-        it('returns immediately if imageUrl is empty', () => {
-          store.state.badgeInEditForm.imageUrl = '';
-
-          vm.onSubmit();
-
-          expect(vm.saveBadge).not.toHaveBeenCalled();
-        });
-
-        it('returns immediately if linkUrl is empty', () => {
-          store.state.badgeInEditForm.linkUrl = '';
-
-          vm.onSubmit();
-
-          expect(vm.saveBadge).not.toHaveBeenCalled();
-        });
-
-        it('returns immediately if isSaving is true', () => {
-          store.state.isSaving = true;
+  const sharedSubmitTests = submitAction => {
+    const imageUrlSelector = '#badge-image-url';
+    const findImageUrlElement = () => vm.$el.querySelector(imageUrlSelector);
+    const linkUrlSelector = '#badge-link-url';
+    const findLinkUrlElement = () => vm.$el.querySelector(linkUrlSelector);
+    const setValue = (inputElementSelector, url) => {
+      const inputElement = vm.$el.querySelector(inputElementSelector);
+      inputElement.value = url;
+      inputElement.dispatchEvent(new Event('input'));
+    };
+    const submitForm = () => {
+      const submitButton = vm.$el.querySelector('button[type="submit"]');
+      submitButton.click();
+    };
+    const expectInvalidInput = inputElementSelector => {
+      const inputElement = vm.$el.querySelector(inputElementSelector);
+      expect(inputElement).toBeMatchedBy(':invalid');
+      const feedbackElement = vm.$el.querySelector(`${inputElementSelector} + .invalid-feedback`);
+      expect(feedbackElement).toBeVisible();
+    };
 
-          vm.onSubmit();
+    beforeEach(() => {
+      spyOn(vm, submitAction).and.returnValue(Promise.resolve());
+      store.replaceState({
+        ...store.state,
+        badgeInAddForm: createEmptyBadge(),
+        badgeInEditForm: createEmptyBadge(),
+        isSaving: false,
+      });
 
-          expect(vm.saveBadge).not.toHaveBeenCalled();
-        });
+      setValue(linkUrlSelector, `${TEST_HOST}/link/url`);
+      setValue(imageUrlSelector, `${window.location.origin}${DUMMY_IMAGE_URL}`);
+    });
 
-        it('calls saveBadge', () => {
-          vm.onSubmit();
+    it('returns immediately if imageUrl is empty', () => {
+      setValue(imageUrlSelector, '');
 
-          expect(vm.saveBadge).toHaveBeenCalled();
-        });
-      });
+      submitForm();
 
-      describe('if isEditing is false', () => {
-        beforeEach(() => {
-          spyOn(vm, 'addBadge').and.returnValue(Promise.resolve());
-          store.replaceState({
-            ...store.state,
-            isSaving: false,
-            badgeInAddForm: createDummyBadge(),
-          });
-          vm.isEditing = false;
-        });
+      expectInvalidInput(imageUrlSelector);
+      expect(vm[submitAction]).not.toHaveBeenCalled();
+    });
 
-        it('returns immediately if imageUrl is empty', () => {
-          store.state.badgeInAddForm.imageUrl = '';
+    it('returns immediately if imageUrl is malformed', () => {
+      setValue(imageUrlSelector, 'not-a-url');
 
-          vm.onSubmit();
+      submitForm();
 
-          expect(vm.addBadge).not.toHaveBeenCalled();
-        });
+      expectInvalidInput(imageUrlSelector);
+      expect(vm[submitAction]).not.toHaveBeenCalled();
+    });
 
-        it('returns immediately if linkUrl is empty', () => {
-          store.state.badgeInAddForm.linkUrl = '';
+    it('returns immediately if linkUrl is empty', () => {
+      setValue(linkUrlSelector, '');
 
-          vm.onSubmit();
+      submitForm();
 
-          expect(vm.addBadge).not.toHaveBeenCalled();
-        });
+      expectInvalidInput(linkUrlSelector);
+      expect(vm[submitAction]).not.toHaveBeenCalled();
+    });
 
-        it('returns immediately if isSaving is true', () => {
-          store.state.isSaving = true;
+    it('returns immediately if linkUrl is malformed', () => {
+      setValue(linkUrlSelector, 'not-a-url');
 
-          vm.onSubmit();
+      submitForm();
 
-          expect(vm.addBadge).not.toHaveBeenCalled();
-        });
+      expectInvalidInput(linkUrlSelector);
+      expect(vm[submitAction]).not.toHaveBeenCalled();
+    });
 
-        it('calls addBadge', () => {
-          vm.onSubmit();
+    it(`calls ${submitAction}`, () => {
+      submitForm();
 
-          expect(vm.addBadge).toHaveBeenCalled();
-        });
-      });
+      expect(findImageUrlElement()).toBeMatchedBy(':valid');
+      expect(findLinkUrlElement()).toBeMatchedBy(':valid');
+      expect(vm[submitAction]).toHaveBeenCalled();
     });
-  });
+  };
 
   describe('if isEditing is false', () => {
     beforeEach(() => {
@@ -138,12 +141,15 @@ describe('BadgeForm component', () => {
     });
 
     it('renders one button', () => {
-      const buttons = vm.$el.querySelectorAll('.row-content-block button');
+      expect(vm.$el.querySelector('.row-content-block')).toBeNull();
+      const buttons = vm.$el.querySelectorAll('.form-group:last-of-type button');
       expect(buttons.length).toBe(1);
       const buttonAddElement = buttons[0];
       expect(buttonAddElement).toBeVisible();
       expect(buttonAddElement).toHaveText('Add badge');
     });
+
+    sharedSubmitTests('addBadge');
   });
 
   describe('if isEditing is true', () => {
@@ -167,5 +173,7 @@ describe('BadgeForm component', () => {
       expect(buttonCancelElement).toBeVisible();
       expect(buttonCancelElement).toHaveText('Cancel');
     });
+
+    sharedSubmitTests('saveBadge');
   });
 });