diff --git a/.gitlab/issue_templates/Technical Evaluation.md b/.gitlab/issue_templates/Technical Evaluation.md index f703f727113..f603d88a764 100644 --- a/.gitlab/issue_templates/Technical Evaluation.md +++ b/.gitlab/issue_templates/Technical Evaluation.md @@ -9,9 +9,12 @@ -- [ ] Add task -- [ ] Add task -- [ ] Add task +- [ ] Determine feasibility of the feature +- [ ] Create issue for implementation or update existing implementation issue description with implementation proposal +- [ ] Set weight on implementation issue +- [ ] If weight is greater than 5, break issue into smaller issues +- [ ] Add task +- [ ] Add task ### Risks and Implementation Considerations diff --git a/app/assets/javascripts/snippet/snippet_bundle.js b/app/assets/javascripts/snippet/snippet_bundle.js index 652531a1289..8e952fe9358 100644 --- a/app/assets/javascripts/snippet/snippet_bundle.js +++ b/app/assets/javascripts/snippet/snippet_bundle.js @@ -1,14 +1,50 @@ /* global ace */ - -import $ from 'jquery'; +import Editor from '~/editor/editor_lite'; import setupCollapsibleInputs from './collapsible_input'; -export default () => { - const editor = ace.edit('editor'); +let editor; - $('.snippet-form-holder form').on('submit', () => { - $('.snippet-file-content').val(editor.getValue()); +const initAce = () => { + editor = ace.edit('editor'); + + const form = document.querySelector('.snippet-form-holder form'); + const content = document.querySelector('.snippet-file-content'); + form.addEventListener('submit', () => { + content.value = editor.getValue(); + }); +}; + +const initMonaco = () => { + const editorEl = document.getElementById('editor'); + const contentEl = document.querySelector('.snippet-file-content'); + const fileNameEl = document.querySelector('.snippet-file-name'); + const form = document.querySelector('.snippet-form-holder form'); + + editor = new Editor(); + editor.createInstance({ + el: editorEl, + blobPath: fileNameEl.value, + blobContent: contentEl.value, }); + fileNameEl.addEventListener('change', () => { + editor.updateModelLanguage(fileNameEl.value); + }); + + form.addEventListener('submit', () => { + contentEl.value = editor.getValue(); + }); +}; + +export const initEditor = () => { + if (window?.gon?.features?.monacoSnippets) { + initMonaco(); + } else { + initAce(); + } setupCollapsibleInputs(); }; + +export default () => { + initEditor(); +}; diff --git a/app/models/deployment.rb b/app/models/deployment.rb index fe42fb93633..fbb59173a3c 100644 --- a/app/models/deployment.rb +++ b/app/models/deployment.rb @@ -229,7 +229,14 @@ class Deployment < ApplicationRecord end def link_merge_requests(relation) - select = relation.select(['merge_requests.id', id]).to_sql + # NOTE: relation.select will perform column deduplication, + # when id == environment_id it will outputs 2 columns instead of 3 + # i.e.: + # MergeRequest.select(1, 2).to_sql #=> SELECT 1, 2 FROM "merge_requests" + # MergeRequest.select(1, 1).to_sql #=> SELECT 1 FROM "merge_requests" + select = relation.select('merge_requests.id', + "#{id} as deployment_id", + "#{environment_id} as environment_id").to_sql # We don't use `Gitlab::Database.bulk_insert` here so that we don't need to # first pluck lots of IDs into memory. @@ -238,7 +245,7 @@ class Deployment < ApplicationRecord # for the same deployment, only inserting any missing merge requests. DeploymentMergeRequest.connection.execute(<<~SQL) INSERT INTO #{DeploymentMergeRequest.table_name} - (merge_request_id, deployment_id) + (merge_request_id, deployment_id, environment_id) #{select} ON CONFLICT DO NOTHING SQL diff --git a/app/views/shared/snippets/_form.html.haml b/app/views/shared/snippets/_form.html.haml index 3c2c751c579..b07992c6890 100644 --- a/app/views/shared/snippets/_form.html.haml +++ b/app/views/shared/snippets/_form.html.haml @@ -28,7 +28,7 @@ .js-file-title.file-title-flex-parent = f.text_field :file_name, placeholder: s_("Snippets|Give your file a name to add code highlighting, e.g. example.rb for Ruby"), class: 'form-control snippet-file-name qa-snippet-file-name' .file-content.code - %pre#editor= @snippet.content + %pre#editor{ data: { 'editor-loading': true } }= @snippet.content = f.hidden_field :content, class: 'snippet-file-content' .form-group diff --git a/changelogs/unreleased/198604-monaco-snippets.yml b/changelogs/unreleased/198604-monaco-snippets.yml new file mode 100644 index 00000000000..79f9584ec56 --- /dev/null +++ b/changelogs/unreleased/198604-monaco-snippets.yml @@ -0,0 +1,5 @@ +--- +title: Replaced ACE with Monaco editor for Snippets +merge_request: 25465 +author: +type: added diff --git a/changelogs/unreleased/25838-include-full-upload-url-in-api-response.yml b/changelogs/unreleased/25838-include-full-upload-url-in-api-response.yml new file mode 100644 index 00000000000..879273efefa --- /dev/null +++ b/changelogs/unreleased/25838-include-full-upload-url-in-api-response.yml @@ -0,0 +1,5 @@ +--- +title: Include full path to an upload in api response +merge_request: 23500 +author: briankabiro +type: other diff --git a/changelogs/unreleased/deploy-mr-once-take-2.yml b/changelogs/unreleased/deploy-mr-once-take-2.yml new file mode 100644 index 00000000000..59b612def4a --- /dev/null +++ b/changelogs/unreleased/deploy-mr-once-take-2.yml @@ -0,0 +1,5 @@ +--- +title: Don't track MR deployment multiple times +merge_request: 25537 +author: +type: fixed diff --git a/db/migrate/20200219133859_add_environment_id_to_deployment_merge_requests.rb b/db/migrate/20200219133859_add_environment_id_to_deployment_merge_requests.rb new file mode 100644 index 00000000000..a57d0b44c52 --- /dev/null +++ b/db/migrate/20200219133859_add_environment_id_to_deployment_merge_requests.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddEnvironmentIdToDeploymentMergeRequests < ActiveRecord::Migration[6.0] + DOWNTIME = false + + def change + add_column :deployment_merge_requests, :environment_id, :integer, null: true + end +end diff --git a/db/migrate/20200219141307_add_environment_id_fk_to_deployment_merge_requests.rb b/db/migrate/20200219141307_add_environment_id_fk_to_deployment_merge_requests.rb new file mode 100644 index 00000000000..76980b21feb --- /dev/null +++ b/db/migrate/20200219141307_add_environment_id_fk_to_deployment_merge_requests.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddEnvironmentIdFkToDeploymentMergeRequests < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_foreign_key :deployment_merge_requests, :environments, column: :environment_id, on_delete: :cascade + end + + def down + remove_foreign_key_if_exists :deployment_merge_requests, column: :environment_id + end +end diff --git a/db/migrate/20200219142522_add_environment_id_merge_request_id_uniq_idx_to_deployment_merge_requests.rb b/db/migrate/20200219142522_add_environment_id_merge_request_id_uniq_idx_to_deployment_merge_requests.rb new file mode 100644 index 00000000000..a557f3f88d7 --- /dev/null +++ b/db/migrate/20200219142522_add_environment_id_merge_request_id_uniq_idx_to_deployment_merge_requests.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddEnvironmentIdMergeRequestIdUniqIdxToDeploymentMergeRequests < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index :deployment_merge_requests, [:environment_id, :merge_request_id], unique: true, name: 'idx_environment_merge_requests_unique_index' + end + + def down + remove_concurrent_index_by_name :deployment_merge_requests, 'idx_environment_merge_requests_unique_index' + end +end diff --git a/db/schema.rb b/db/schema.rb index c6a5e2dd869..149582f94bf 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1372,7 +1372,9 @@ ActiveRecord::Schema.define(version: 2020_02_26_162723) do create_table "deployment_merge_requests", id: false, force: :cascade do |t| t.integer "deployment_id", null: false t.integer "merge_request_id", null: false + t.integer "environment_id" t.index ["deployment_id", "merge_request_id"], name: "idx_deployment_merge_requests_unique_index", unique: true + t.index ["environment_id", "merge_request_id"], name: "idx_environment_merge_requests_unique_index", unique: true t.index ["merge_request_id"], name: "index_deployment_merge_requests_on_merge_request_id" end @@ -4719,6 +4721,7 @@ ActiveRecord::Schema.define(version: 2020_02_26_162723) do add_foreign_key "deployment_clusters", "clusters", on_delete: :cascade add_foreign_key "deployment_clusters", "deployments", on_delete: :cascade add_foreign_key "deployment_merge_requests", "deployments", on_delete: :cascade + add_foreign_key "deployment_merge_requests", "environments", name: "fk_a064ff4453", on_delete: :cascade add_foreign_key "deployment_merge_requests", "merge_requests", on_delete: :cascade add_foreign_key "deployments", "clusters", name: "fk_289bba3222", on_delete: :nullify add_foreign_key "deployments", "projects", name: "fk_b9a3851b82", on_delete: :cascade diff --git a/doc/api/projects.md b/doc/api/projects.md index 905eb01b0ad..c2154233cc5 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -1836,11 +1836,12 @@ Returned object: { "alt": "dk", "url": "/uploads/66dbcd21ec5d24ed6ea225176098d52b/dk.png", + "full_path": "/namespace1/project1/uploads/66dbcd21ec5d24ed6ea225176098d52b/dk.png", "markdown": "![dk](/uploads/66dbcd21ec5d24ed6ea225176098d52b/dk.png)" } ``` ->**Note**: The returned `url` is relative to the project path. +>**Note**: The returned `url` is relative to the project path. The returned `full_path` is the absolute path to the file. In Markdown contexts, the link is automatically expanded when the format in `markdown` is used. diff --git a/doc/development/fe_guide/style/javascript.md b/doc/development/fe_guide/style/javascript.md index f40e8c7b5df..7951c702601 100644 --- a/doc/development/fe_guide/style/javascript.md +++ b/doc/development/fe_guide/style/javascript.md @@ -175,6 +175,21 @@ are loaded dynamically with webpack. Do not use `innerHTML`, `append()` or `html()` to set content. It opens up too many vulnerabilities. +## Avoid single-line conditional statements + +Indentation is important when scanning code as it gives a quick indication of the existence of branches, loops, and return points. +This can help to quickly understand the control flow. + +```javascript +// bad +if (isThingNull) return ''; + +// good +if (isThingNull) { + return ''; +} +``` + ## ESLint ESLint behaviour can be found in our [tooling guide](../tooling.md). diff --git a/doc/user/project/issues/design_management.md b/doc/user/project/issues/design_management.md index 58da77697d8..f7fa8c84d9e 100644 --- a/doc/user/project/issues/design_management.md +++ b/doc/user/project/issues/design_management.md @@ -37,6 +37,8 @@ Design Management requires that projects are using [hashed storage](../../../administration/repository_storage_types.md#hashed-storage) (the default storage type since v10.0). +If the requirements are not met, the **Designs** tab displays a message to the user. + ### Feature Flags - Reference Parsing diff --git a/lib/api/entities/project_upload.rb b/lib/api/entities/project_upload.rb new file mode 100644 index 00000000000..f38f8d74f7b --- /dev/null +++ b/lib/api/entities/project_upload.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module API + module Entities + class ProjectUpload < Grape::Entity + include Gitlab::Routing + + expose :markdown_name, as: :alt + expose :secure_url, as: :url + expose :full_path do |uploader| + show_project_uploads_path( + uploader.model, + uploader.secret, + uploader.filename + ) + end + + expose :markdown_link, as: :markdown + end + end +end diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 6f96ffde0a7..43d607bc0c1 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -494,7 +494,9 @@ module API requires :file, type: File, desc: 'The file to be uploaded' # rubocop:disable Scalability/FileUploads end post ":id/uploads" do - UploadService.new(user_project, params[:file]).execute.to_h + upload = UploadService.new(user_project, params[:file]).execute + + present upload, with: Entities::ProjectUpload end desc 'Get the users list of a project' do diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 32e1b704a15..1dfa9d0c95f 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -1293,6 +1293,15 @@ msgstr "" msgid "AdminArea|Stopping jobs failed" msgstr "" +msgid "AdminArea|Users statistics" +msgstr "" + +msgid "AdminArea|Users total" +msgstr "" + +msgid "AdminArea|Users with highest role" +msgstr "" + msgid "AdminArea|You’re about to stop all jobs.This will halt all current jobs that are running." msgstr "" @@ -6669,6 +6678,9 @@ msgstr "" msgid "DesignManagement|The one place for your designs" msgstr "" +msgid "DesignManagement|To enable design management, you'll need to %{requirements_link_start}meet the requirements%{requirements_link_end}. If you need help, reach out to our %{support_link_start}support team%{support_link_end} for assistance." +msgstr "" + msgid "DesignManagement|Upload and view the latest designs for this issue. Consistent and easy to find, so everyone is up to date." msgstr "" @@ -19358,6 +19370,9 @@ msgstr "" msgid "The number of times an upload record could not find its file" msgstr "" +msgid "The one place for your designs" +msgstr "" + msgid "The passphrase required to decrypt the private key. This is optional and the value is encrypted at rest." msgstr "" diff --git a/qa/qa/page/dashboard/snippet/new.rb b/qa/qa/page/dashboard/snippet/new.rb index df4c2902b31..da5013e787e 100644 --- a/qa/qa/page/dashboard/snippet/new.rb +++ b/qa/qa/page/dashboard/snippet/new.rb @@ -52,7 +52,7 @@ module QA private def text_area - find('#editor>textarea', visible: false) + find('#editor textarea', visible: false) end end end diff --git a/spec/features/projects/snippets/create_snippet_spec.rb b/spec/features/projects/snippets/create_snippet_spec.rb index 40c54b2f873..ee312a5811d 100644 --- a/spec/features/projects/snippets/create_snippet_spec.rb +++ b/spec/features/projects/snippets/create_snippet_spec.rb @@ -2,11 +2,10 @@ require 'spec_helper' -describe 'Projects > Snippets > Create Snippet', :js do - include DropzoneHelper - - let_it_be(:user) { create(:user) } - let_it_be(:project) { create(:project, :public) } +shared_examples_for 'snippet editor' do + before do + stub_feature_flags(monaco_snippets: flag) + end def description_field find('.js-description-input').find('input,textarea') @@ -20,7 +19,8 @@ describe 'Projects > Snippets > Create Snippet', :js do fill_in 'project_snippet_description', with: 'My Snippet **Description**' page.within('.file-editor') do - find('.ace_text-input', visible: false).send_keys('Hello World!') + el = flag == true ? find('.inputarea') : find('.ace_text-input', visible: false) + el.send_keys 'Hello World!' end end @@ -33,6 +33,7 @@ describe 'Projects > Snippets > Create Snippet', :js do visit project_snippets_path(project) click_on('New snippet') + wait_for_requests end it 'shows collapsible description input' do @@ -111,3 +112,22 @@ describe 'Projects > Snippets > Create Snippet', :js do end end end + +describe 'Projects > Snippets > Create Snippet', :js do + include DropzoneHelper + + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, :public) } + + context 'when using Monaco' do + it_behaves_like "snippet editor" do + let(:flag) { true } + end + end + + context 'when using ACE' do + it_behaves_like "snippet editor" do + let(:flag) { false } + end + end +end diff --git a/spec/features/snippets/spam_snippets_spec.rb b/spec/features/snippets/spam_snippets_spec.rb index 02b807ce071..efe1bdc963d 100644 --- a/spec/features/snippets/spam_snippets_spec.rb +++ b/spec/features/snippets/spam_snippets_spec.rb @@ -2,9 +2,7 @@ require 'spec_helper' -describe 'User creates snippet', :js do - let(:user) { create(:user) } - +shared_examples_for 'snippet editor' do def description_field find('.js-description-input').find('input,textarea') end @@ -12,6 +10,7 @@ describe 'User creates snippet', :js do before do stub_feature_flags(allow_possible_spam: false) stub_feature_flags(snippets_vue: false) + stub_feature_flags(monaco_snippets: flag) stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') Gitlab::CurrentSettings.update!( @@ -33,7 +32,8 @@ describe 'User creates snippet', :js do find('#personal_snippet_visibility_level_20').set(true) page.within('.file-editor') do - find('.ace_text-input', visible: false).send_keys 'Hello World!' + el = flag == true ? find('.inputarea') : find('.ace_text-input', visible: false) + el.send_keys 'Hello World!' end end @@ -80,3 +80,19 @@ describe 'User creates snippet', :js do end end end + +describe 'User creates snippet', :js do + let_it_be(:user) { create(:user) } + + context 'when using Monaco' do + it_behaves_like "snippet editor" do + let(:flag) { true } + end + end + + context 'when using ACE' do + it_behaves_like "snippet editor" do + let(:flag) { false } + end + end +end diff --git a/spec/features/snippets/user_creates_snippet_spec.rb b/spec/features/snippets/user_creates_snippet_spec.rb index 69a44513e7f..88f7896bfa6 100644 --- a/spec/features/snippets/user_creates_snippet_spec.rb +++ b/spec/features/snippets/user_creates_snippet_spec.rb @@ -2,13 +2,10 @@ require 'spec_helper' -describe 'User creates snippet', :js do - include DropzoneHelper - - let(:user) { create(:user) } - +shared_examples_for 'snippet editor' do before do stub_feature_flags(snippets_vue: false) + stub_feature_flags(monaco_snippets: flag) sign_in(user) visit new_snippet_path end @@ -25,7 +22,8 @@ describe 'User creates snippet', :js do fill_in 'personal_snippet_description', with: 'My Snippet **Description**' page.within('.file-editor') do - find('.ace_text-input', visible: false).send_keys 'Hello World!' + el = flag == true ? find('.inputarea') : find('.ace_text-input', visible: false) + el.send_keys 'Hello World!' end end @@ -109,7 +107,8 @@ describe 'User creates snippet', :js do fill_in 'personal_snippet_title', with: 'My Snippet Title' page.within('.file-editor') do find(:xpath, "//input[@id='personal_snippet_file_name']").set 'snippet+file+name' - find('.ace_text-input', visible: false).send_keys 'Hello World!' + el = flag == true ? find('.inputarea') : find('.ace_text-input', visible: false) + el.send_keys 'Hello World!' end click_button 'Create snippet' @@ -120,3 +119,21 @@ describe 'User creates snippet', :js do expect(page).to have_content('Hello World!') end end + +describe 'User creates snippet', :js do + include DropzoneHelper + + let_it_be(:user) { create(:user) } + + context 'when using Monaco' do + it_behaves_like "snippet editor" do + let(:flag) { true } + end + end + + context 'when using ACE' do + it_behaves_like "snippet editor" do + let(:flag) { false } + end + end +end diff --git a/spec/frontend/snippet/snippet_bundle_spec.js b/spec/frontend/snippet/snippet_bundle_spec.js new file mode 100644 index 00000000000..af98e8d665d --- /dev/null +++ b/spec/frontend/snippet/snippet_bundle_spec.js @@ -0,0 +1,94 @@ +import Editor from '~/editor/editor_lite'; +import { initEditor } from '~/snippet/snippet_bundle'; +import { setHTMLFixture } from 'helpers/fixtures'; + +jest.mock('~/editor/editor_lite', () => jest.fn()); + +describe('Snippet editor', () => { + describe('Monaco editor for Snippets', () => { + let oldGon; + let editorEl; + let contentEl; + let fileNameEl; + let form; + + const mockName = 'foo.bar'; + const mockContent = 'Foo Bar'; + const updatedMockContent = 'New Foo Bar'; + + const mockEditor = { + createInstance: jest.fn(), + updateModelLanguage: jest.fn(), + getValue: jest.fn().mockReturnValueOnce(updatedMockContent), + }; + Editor.mockImplementation(() => mockEditor); + + function setUpFixture(name, content) { + setHTMLFixture(` +