diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-06-11 21:09:58 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-06-11 21:09:58 +0000 |
commit | 68b6fd7fb2edaaae86a4ee8df02b7f9783672050 (patch) | |
tree | 732a7b3924f19755871e9a300db5acaaff8434f8 | |
parent | e58ce90f147742c314b9cc08c2d1c0b585e39cf9 (diff) | |
download | gitlab-ce-68b6fd7fb2edaaae86a4ee8df02b7f9783672050.tar.gz |
Add latest changes from gitlab-org/gitlab@master
-rw-r--r-- | app/graphql/types/project_type.rb | 4 | ||||
-rw-r--r-- | app/helpers/tags_helper.rb | 10 | ||||
-rw-r--r-- | app/models/project.rb | 4 | ||||
-rw-r--r-- | doc/ci/pipelines/settings.md | 2 | ||||
-rw-r--r-- | doc/development/routing.md | 12 | ||||
-rw-r--r-- | lib/api/internal/base.rb | 11 | ||||
-rw-r--r-- | spec/features/projects/show/schema_markup_spec.rb | 4 | ||||
-rw-r--r-- | spec/graphql/resolvers/projects_resolver_spec.rb | 2 | ||||
-rw-r--r-- | spec/models/project_spec.rb | 8 | ||||
-rw-r--r-- | spec/requests/api/graphql/project_query_spec.rb | 2 | ||||
-rw-r--r-- | spec/requests/api/internal/base_spec.rb | 53 | ||||
-rw-r--r-- | spec/requests/api/projects_spec.rb | 12 |
12 files changed, 41 insertions, 83 deletions
diff --git a/app/graphql/types/project_type.rb b/app/graphql/types/project_type.rb index a2852588e89..55dc73d898d 100644 --- a/app/graphql/types/project_type.rb +++ b/app/graphql/types/project_type.rb @@ -387,6 +387,10 @@ module Types ::Security::CiConfiguration::SastParserService.new(object).configuration end + def tag_list + object.topic_list + end + private def project diff --git a/app/helpers/tags_helper.rb b/app/helpers/tags_helper.rb index bfc8803f514..86289ec8ed2 100644 --- a/app/helpers/tags_helper.rb +++ b/app/helpers/tags_helper.rb @@ -15,16 +15,6 @@ module TagsHelper project_tags_path(@project, @id, options) end - def tag_list(project) - html = [] - - project.tag_list.each do |tag| - html << link_to(tag, tag_path(tag)) - end - - html.join.html_safe - end - def protected_tag?(project, tag) ProtectedTag.protected?(project, tag.name) end diff --git a/app/models/project.rb b/app/models/project.rb index 6895bba7cf7..0a16a056271 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -127,10 +127,6 @@ class Project < ApplicationRecord after_create :check_repository_absence! acts_as_ordered_taggable_on :topics - # The 'tag_list' alias is required during the 'tags -> topics' migration - # TODO: eliminate 'tag_list' in the further process of the migration - # https://gitlab.com/gitlab-org/gitlab/-/issues/328226 - alias_attribute :tag_list, :topic_list attr_accessor :old_path_with_namespace attr_accessor :template_name diff --git a/doc/ci/pipelines/settings.md b/doc/ci/pipelines/settings.md index 6fefe3e7d39..ab3f4169c18 100644 --- a/doc/ci/pipelines/settings.md +++ b/doc/ci/pipelines/settings.md @@ -149,7 +149,7 @@ averaged. - JaCoCo (Java/Kotlin). Example: `Total.*?([0-9]{1,3})%`. - `go test -cover` (Go). Example: `coverage: \d+.\d+% of statements`. - .Net (OpenCover). Example: `(Visited Points).*\((.*)\)`. -- .Net (`dotnet test` line coverage). Example: `Total\s*\|\s*(\d+\.?\d+)`. +- .Net (`dotnet test` line coverage). Example: `Total\s*\|\s*(\d+(?:\.\d+)?)`. <!-- vale gitlab.Spelling = YES --> diff --git a/doc/development/routing.md b/doc/development/routing.md index 2c2f6b2a558..8fca9b00157 100644 --- a/doc/development/routing.md +++ b/doc/development/routing.md @@ -69,6 +69,18 @@ gitlab-org/gitlab/-/settings/repository gitlab-org/serverless/runtimes/-/settings/repository ``` +## Changing existing routes + +Don't change a URL to an existing page, unless it's necessary. If you must make a change, +make it unnoticeable for users, because we don't want them to receive `404 Not Found` +if we can avoid it. This table should help: + +| URL description | Example | What to do | +|---|---|---| +| Can be used in scripts and automation | `snippet#raw` | Support both an old and new URL for one major release. Then, support a redirect from an old URL to a new URL for another major release. | +| Likely to be saved or shared | `issue#show` | Add a redirect from an old URL to a new URL until the next major release. | +| Limited use, unlikely to be shared | `admin#labels` | No extra steps required. | + ## Migrating unscoped routes Currently, the majority of routes are placed under the `/-/` scope. However, diff --git a/lib/api/internal/base.rb b/lib/api/internal/base.rb index 96d3674077d..ee0ddccc8d4 100644 --- a/lib/api/internal/base.rb +++ b/lib/api/internal/base.rb @@ -167,18 +167,15 @@ module API end # - # Get a ssh key using the fingerprint + # Check whether an SSH key is known to GitLab # - # rubocop: disable CodeReuse/ActiveRecord get '/authorized_keys', feature_category: :source_code_management do - fingerprint = params.fetch(:fingerprint) do - Gitlab::InsecureKeyFingerprint.new(params.fetch(:key)).fingerprint - end - key = Key.find_by(fingerprint: fingerprint) + fingerprint = Gitlab::InsecureKeyFingerprint.new(params.fetch(:key)).fingerprint + + key = Key.find_by_fingerprint(fingerprint) not_found!('Key') if key.nil? present key, with: Entities::SSHKey end - # rubocop: enable CodeReuse/ActiveRecord # # Discover user by ssh key, user id or username diff --git a/spec/features/projects/show/schema_markup_spec.rb b/spec/features/projects/show/schema_markup_spec.rb index 1777b72cbf5..28803db924a 100644 --- a/spec/features/projects/show/schema_markup_spec.rb +++ b/spec/features/projects/show/schema_markup_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe 'Projects > Show > Schema Markup' do - let_it_be(:project) { create(:project, :repository, :public, :with_avatar, description: 'foobar', tag_list: 'tag1, tag2') } + let_it_be(:project) { create(:project, :repository, :public, :with_avatar, description: 'foobar', topic_list: 'topic1, topic2') } it 'shows SoftwareSourceCode structured markup', :js do visit project_path(project) @@ -16,7 +16,7 @@ RSpec.describe 'Projects > Show > Schema Markup' do expect(page).to have_selector('[itemprop="identifier"]', text: "Project ID: #{project.id}") expect(page).to have_selector('[itemprop="description"]', text: project.description) expect(page).to have_selector('[itemprop="license"]', text: project.repository.license.name) - expect(find_all('[itemprop="keywords"]').map(&:text)).to match_array(project.tag_list.map(&:capitalize)) + expect(find_all('[itemprop="keywords"]').map(&:text)).to match_array(project.topic_list.map(&:capitalize)) expect(page).to have_selector('[itemprop="about"]') end end diff --git a/spec/graphql/resolvers/projects_resolver_spec.rb b/spec/graphql/resolvers/projects_resolver_spec.rb index ed798fb0351..2f2aacb9ad5 100644 --- a/spec/graphql/resolvers/projects_resolver_spec.rb +++ b/spec/graphql/resolvers/projects_resolver_spec.rb @@ -10,7 +10,7 @@ RSpec.describe Resolvers::ProjectsResolver do let_it_be(:group) { create(:group, name: 'public-group') } let_it_be(:private_group) { create(:group, name: 'private-group') } - let_it_be(:project) { create(:project, :public, tag_list: %w(ruby javascript)) } + let_it_be(:project) { create(:project, :public, topic_list: %w(ruby javascript)) } let_it_be(:other_project) { create(:project, :public) } let_it_be(:group_project) { create(:project, :public, group: group) } let_it_be(:private_project) { create(:project, :private) } diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 0e6f662da50..242d5947d26 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -6945,7 +6945,7 @@ RSpec.describe Project, factory_default: :keep do end describe 'topics' do - let_it_be(:project) { create(:project, tag_list: 'topic1, topic2, topic3') } + let_it_be(:project) { create(:project, topic_list: 'topic1, topic2, topic3') } it 'topic_list returns correct string array' do expect(project.topic_list).to match_array(%w[topic1 topic2 topic3]) @@ -6955,12 +6955,6 @@ RSpec.describe Project, factory_default: :keep do expect(project.topics.first.class.name).to eq('ActsAsTaggableOn::Tag') expect(project.topics.map(&:name)).to match_array(%w[topic1 topic2 topic3]) end - - context 'aliases' do - it 'tag_list returns correct string array' do - expect(project.tag_list).to match_array(%w[topic1 topic2 topic3]) - end - end end shared_examples 'all_runners' do diff --git a/spec/requests/api/graphql/project_query_spec.rb b/spec/requests/api/graphql/project_query_spec.rb index 4160538b243..54375d4de1d 100644 --- a/spec/requests/api/graphql/project_query_spec.rb +++ b/spec/requests/api/graphql/project_query_spec.rb @@ -65,7 +65,7 @@ RSpec.describe 'getting project information' do end it 'includes topics array' do - project.update!(tag_list: 'topic1, topic2, topic3') + project.update!(topic_list: 'topic1, topic2, topic3') post_graphql(query, current_user: current_user) diff --git a/spec/requests/api/internal/base_spec.rb b/spec/requests/api/internal/base_spec.rb index 6bedd43e5c4..631698554f9 100644 --- a/spec/requests/api/internal/base_spec.rb +++ b/spec/requests/api/internal/base_spec.rb @@ -341,9 +341,9 @@ RSpec.describe API::Internal::Base do end describe "GET /internal/authorized_keys" do - context "using an existing key's fingerprint" do + context "using an existing key" do it "finds the key" do - get(api('/internal/authorized_keys'), params: { fingerprint: key.fingerprint, secret_token: secret_token }) + get(api('/internal/authorized_keys'), params: { key: key.key.split[1], secret_token: secret_token }) expect(response).to have_gitlab_http_status(:ok) expect(json_response['id']).to eq(key.id) @@ -351,58 +351,23 @@ RSpec.describe API::Internal::Base do end it 'exposes the comment of the key as a simple identifier of username + hostname' do - get(api('/internal/authorized_keys'), params: { fingerprint: key.fingerprint, secret_token: secret_token }) + get(api('/internal/authorized_keys'), params: { key: key.key.split[1], secret_token: secret_token }) expect(response).to have_gitlab_http_status(:ok) expect(json_response['key']).to include("#{key.user_name} (#{Gitlab.config.gitlab.host})") end end - context "non existing key's fingerprint" do - it "returns 404" do - get(api('/internal/authorized_keys'), params: { fingerprint: "no:t-:va:li:d0", secret_token: secret_token }) + it "returns 404 with a partial key" do + get(api('/internal/authorized_keys'), params: { key: key.key.split[1][0...-3], secret_token: secret_token }) - expect(response).to have_gitlab_http_status(:not_found) - end + expect(response).to have_gitlab_http_status(:not_found) end - context "using a partial fingerprint" do - it "returns 404" do - get(api('/internal/authorized_keys'), params: { fingerprint: "#{key.fingerprint[0..5]}%", secret_token: secret_token }) - - expect(response).to have_gitlab_http_status(:not_found) - end - end - - context "sending the key" do - context "using an existing key" do - it "finds the key" do - get(api('/internal/authorized_keys'), params: { key: key.key.split[1], secret_token: secret_token }) - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response['id']).to eq(key.id) - expect(json_response['key'].split[1]).to eq(key.key.split[1]) - end - - it 'exposes the comment of the key as a simple identifier of username + hostname' do - get(api('/internal/authorized_keys'), params: { fingerprint: key.fingerprint, secret_token: secret_token }) + it "returns 404 with an not valid base64 string" do + get(api('/internal/authorized_keys'), params: { key: "whatever!", secret_token: secret_token }) - expect(response).to have_gitlab_http_status(:ok) - expect(json_response['key']).to include("#{key.user_name} (#{Gitlab.config.gitlab.host})") - end - end - - it "returns 404 with a partial key" do - get(api('/internal/authorized_keys'), params: { key: key.key.split[1][0...-3], secret_token: secret_token }) - - expect(response).to have_gitlab_http_status(:not_found) - end - - it "returns 404 with an not valid base64 string" do - get(api('/internal/authorized_keys'), params: { key: "whatever!", secret_token: secret_token }) - - expect(response).to have_gitlab_http_status(:not_found) - end + expect(response).to have_gitlab_http_status(:not_found) end end diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index b62f8e6b259..87e4a416f9f 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -287,9 +287,9 @@ RSpec.describe API::Projects do expect(json_response.find { |hash| hash['id'] == project.id }.keys).not_to include('open_issues_count') end - context 'filter by topic (column tag_list)' do + context 'filter by topic (column topic_list)' do before do - project.update!(tag_list: %w(ruby javascript)) + project.update!(topic_list: %w(ruby javascript)) end it 'returns no projects' do @@ -1105,7 +1105,7 @@ RSpec.describe API::Projects do post api('/projects', user), params: project - expect(json_response['tag_list']).to eq(%w[tagFirst tagSecond]) + expect(json_response['topics']).to eq(%w[tagFirst tagSecond]) end it 'sets topics to a project' do @@ -1113,7 +1113,7 @@ RSpec.describe API::Projects do post api('/projects', user), params: project - expect(json_response['tag_list']).to eq(%w[topic1 topics2]) + expect(json_response['topics']).to eq(%w[topic1 topics2]) end it 'uploads avatar for project a project' do @@ -3111,7 +3111,7 @@ RSpec.describe API::Projects do expect(response).to have_gitlab_http_status(:ok) - expect(json_response['tag_list']).to eq(%w[topic1]) + expect(json_response['topics']).to eq(%w[topic1]) end it 'updates topics' do @@ -3121,7 +3121,7 @@ RSpec.describe API::Projects do expect(response).to have_gitlab_http_status(:ok) - expect(json_response['tag_list']).to eq(%w[topic2]) + expect(json_response['topics']).to eq(%w[topic2]) end end |