diff options
author | Robert Speicher <robert@gitlab.com> | 2017-02-07 19:14:19 +0000 |
---|---|---|
committer | Robert Speicher <robert@gitlab.com> | 2017-02-07 19:14:19 +0000 |
commit | 58eae0961ea6e2147a288cda8a56a51303bbf13b (patch) | |
tree | 0bbd873650dfc889af8df7e197397959d774d411 | |
parent | d3aaa1a2a0313b7132479d4211e406fde5bc9324 (diff) | |
parent | 0fdf54ddf5894ec1057b7351140fdce4f0df0ed1 (diff) | |
download | gitlab-ce-58eae0961ea6e2147a288cda8a56a51303bbf13b.tar.gz |
Merge branch 'route-map' into 'master'
Add 'View on [env]' link to blobs and individual files in diffs
See merge request !8867
38 files changed, 815 insertions, 157 deletions
diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb index 9940263ae24..4c39fe98028 100644 --- a/app/controllers/projects/blob_controller.rb +++ b/app/controllers/projects/blob_controller.rb @@ -30,6 +30,8 @@ class Projects::BlobController < Projects::ApplicationController end def show + environment_params = @repository.branch_exists?(@ref) ? { ref: @ref } : { commit: @commit } + @environment = EnvironmentsFinder.new(@project, current_user, environment_params).execute.last end def edit diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb index f880a9862c6..e10d7992db7 100644 --- a/app/controllers/projects/commit_controller.rb +++ b/app/controllers/projects/commit_controller.rb @@ -94,6 +94,8 @@ class Projects::CommitController < Projects::ApplicationController @diffs = commit.diffs(opts) @notes_count = commit.notes.count + + @environment = EnvironmentsFinder.new(@project, current_user, commit: @commit).execute.last end def define_note_vars diff --git a/app/controllers/projects/compare_controller.rb b/app/controllers/projects/compare_controller.rb index 321cde255c3..c6651254d70 100644 --- a/app/controllers/projects/compare_controller.rb +++ b/app/controllers/projects/compare_controller.rb @@ -57,6 +57,9 @@ class Projects::CompareController < Projects::ApplicationController @diffs = @compare.diffs(diff_options) + environment_params = @repository.branch_exists?(@head_ref) ? { ref: @head_ref } : { commit: @commit } + @environment = EnvironmentsFinder.new(@project, current_user, environment_params).execute.last + @diff_notes_disabled = true @grouped_diff_discussions = {} end diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index 77877cd262d..0ec8f5bd64a 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -10,7 +10,7 @@ class Projects::EnvironmentsController < Projects::ApplicationController def index @scope = params[:scope] - @environments = project.environments + @environments = project.environments.includes(:last_deployment) respond_to do |format| format.html diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 368bd27c91a..3be6e8e1772 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -103,6 +103,8 @@ class Projects::MergeRequestsController < Projects::ApplicationController end end + @environment = @merge_request.environments_for(current_user).last + respond_to do |format| format.html { define_discussion_vars } format.json do @@ -248,7 +250,9 @@ class Projects::MergeRequestsController < Projects::ApplicationController end @diff_notes_disabled = true - render json: { html: view_to_html_string('projects/merge_requests/_new_diffs', diffs: @diffs) } + @environment = @merge_request.environments_for(current_user).last + + render json: { html: view_to_html_string('projects/merge_requests/_new_diffs', diffs: @diffs, environment: @environment) } end end end @@ -447,9 +451,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController def ci_environments_status environments = begin - @merge_request.environments.map do |environment| - next unless can?(current_user, :read_environment, environment) - + @merge_request.environments_for(current_user).map do |environment| project = environment.project deployment = environment.first_deployment_for(@merge_request.diff_head_commit) diff --git a/app/finders/environments_finder.rb b/app/finders/environments_finder.rb new file mode 100644 index 00000000000..a59f8c1efa3 --- /dev/null +++ b/app/finders/environments_finder.rb @@ -0,0 +1,55 @@ +class EnvironmentsFinder + attr_reader :project, :current_user, :params + + def initialize(project, current_user, params = {}) + @project, @current_user, @params = project, current_user, params + end + + def execute + deployments = project.deployments + deployments = + if ref + deployments_query = params[:with_tags] ? 'ref = :ref OR tag IS TRUE' : 'ref = :ref' + deployments.where(deployments_query, ref: ref.to_s) + elsif commit + deployments.where(sha: commit.sha) + else + deployments.none + end + + environment_ids = deployments + .group(:environment_id) + .select(:environment_id) + + environments = project.environments.available + .where(id: environment_ids).order_by_last_deployed_at.to_a + + environments.select! do |environment| + Ability.allowed?(current_user, :read_environment, environment) + end + + if ref && commit + environments.select! do |environment| + environment.includes_commit?(commit) + end + end + + if ref && params[:recently_updated] + environments.select! do |environment| + environment.recently_updated_on_branch?(ref) + end + end + + environments + end + + private + + def ref + params[:ref].try(:to_s) + end + + def commit + params[:commit] + end +end diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb index 6dcb624c4da..8aad39e148b 100644 --- a/app/helpers/commits_helper.rb +++ b/app/helpers/commits_helper.rb @@ -194,7 +194,7 @@ module CommitsHelper end end - def view_file_btn(commit_sha, diff_new_path, project) + def view_file_button(commit_sha, diff_new_path, project) link_to( namespace_project_blob_path(project.namespace, project, tree_join(commit_sha, diff_new_path)), @@ -205,6 +205,17 @@ module CommitsHelper end end + def view_on_environment_button(commit_sha, diff_new_path, environment) + return unless environment && commit_sha + + external_url = environment.external_url_for(diff_new_path, commit_sha) + return unless external_url + + link_to(external_url, class: 'btn btn-file-option has-tooltip', target: '_blank', title: "View on #{environment.formatted_external_url}", data: { container: 'body' }) do + icon('external-link') + end + end + def truncate_sha(sha) Commit.truncate_sha(sha) end diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 3c1a1ae5933..5213ea9d02b 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -9,6 +9,7 @@ module Ci belongs_to :erased_by, class_name: 'User' has_many :deployments, as: :deployable + has_one :last_deployment, -> { order('deployments.id DESC') }, as: :deployable, class_name: 'Deployment' # The "environment" field for builds is a String, and is the unexpanded name def persisted_environment @@ -183,10 +184,6 @@ module Ci success? && !last_deployment.try(:last?) end - def last_deployment - deployments.last - end - def depends_on_builds # Get builds of the same type latest_builds = self.pipeline.builds.latest diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index fab8497ec7d..bbc358adb83 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -283,13 +283,7 @@ module Ci def ci_yaml_file return @ci_yaml_file if defined?(@ci_yaml_file) - @ci_yaml_file ||= begin - blob = project.repository.blob_at(sha, '.gitlab-ci.yml') - blob.load_all_data!(project.repository) - blob.data - rescue - nil - end + @ci_yaml_file = project.repository.gitlab_ci_yml_for(sha) rescue nil end def has_yaml_errors? diff --git a/app/models/environment.rb b/app/models/environment.rb index 13c4630c565..803060b3979 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -6,7 +6,8 @@ class Environment < ActiveRecord::Base belongs_to :project, required: true, validate: true - has_many :deployments + has_many :deployments, dependent: :destroy + has_one :last_deployment, -> { order('deployments.id DESC') }, class_name: 'Deployment' before_validation :nullify_external_url before_validation :generate_slug, if: ->(env) { env.slug.blank? } @@ -37,6 +38,13 @@ class Environment < ActiveRecord::Base scope :available, -> { with_state(:available) } scope :stopped, -> { with_state(:stopped) } + scope :order_by_last_deployed_at, -> do + max_deployment_id_sql = + Deployment.select(Deployment.arel_table[:id].maximum). + where(Deployment.arel_table[:environment_id].eq(arel_table[:id])). + to_sql + order(Gitlab::Database.nulls_first_order("(#{max_deployment_id_sql})", 'ASC')) + end state_machine :state, initial: :available do event :start do @@ -62,10 +70,6 @@ class Environment < ActiveRecord::Base ref.to_s == last_deployment.try(:ref) end - def last_deployment - deployments.last - end - def nullify_external_url self.external_url = nil if self.external_url.blank? end @@ -87,6 +91,10 @@ class Environment < ActiveRecord::Base last_deployment.includes_commit?(commit) end + def last_deployed_at + last_deployment.try(:created_at) + end + def update_merge_request_metrics? (environment_type || name) == "production" end @@ -171,6 +179,15 @@ class Environment < ActiveRecord::Base self.slug = slugified end + def external_url_for(path, commit_sha) + return unless self.external_url + + public_path = project.public_path_for_source_path(path, commit_sha) + return unless public_path + + [external_url, public_path].join('/') + end + private # Slugifying a name may remove the uniqueness guarantee afforded by it being diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 082adcafcc8..43085f69105 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -715,18 +715,22 @@ class MergeRequest < ActiveRecord::Base !head_pipeline || head_pipeline.success? || head_pipeline.skipped? end - def environments + def environments_for(current_user) return [] unless diff_head_commit - @environments ||= begin - target_envs = target_project.environments_for( - target_branch, commit: diff_head_commit, with_tags: true) + @environments ||= Hash.new do |h, current_user| + envs = EnvironmentsFinder.new(target_project, current_user, + ref: target_branch, commit: diff_head_commit, with_tags: true).execute - source_envs = source_project.environments_for( - source_branch, commit: diff_head_commit) if source_project + if source_project + envs.concat EnvironmentsFinder.new(source_project, current_user, + ref: source_branch, commit: diff_head_commit).execute + end - (target_envs.to_a + source_envs.to_a).uniq + h[current_user] = envs.uniq end + + @environments[current_user] end def state_human_name diff --git a/app/models/project.rb b/app/models/project.rb index 7c5fdad5122..b45f22d94d9 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1306,28 +1306,26 @@ class Project < ActiveRecord::Base Gitlab::Redis.with { |redis| redis.del(pushes_since_gc_redis_key) } end - def environments_for(ref, commit: nil, with_tags: false) - deployments_query = with_tags ? 'ref = ? OR tag IS TRUE' : 'ref = ?' - - environment_ids = deployments - .where(deployments_query, ref.to_s) - .group(:environment_id) - .select(:environment_id) - - environments_found = environments.available - .where(id: environment_ids).to_a - - return environments_found unless commit - - environments_found.select do |environment| - environment.includes_commit?(commit) + def route_map_for(commit_sha) + @route_maps_by_commit ||= Hash.new do |h, sha| + h[sha] = begin + data = repository.route_map_for(sha) + next unless data + + Gitlab::RouteMap.new(data) + rescue Gitlab::RouteMap::FormatError + nil + end end + + @route_maps_by_commit[commit_sha] end - def environments_recently_updated_on_branch(branch) - environments_for(branch).select do |environment| - environment.recently_updated_on_branch?(branch) - end + def public_path_for_source_path(path, commit_sha) + map = route_map_for(commit_sha) + return unless map + + map.public_path_for_source_path(path) end private diff --git a/app/models/repository.rb b/app/models/repository.rb index 7cf09c52bf4..d2d92a064a4 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -464,6 +464,8 @@ class Repository unless Gitlab::Git.blank_ref?(sha) Blob.decorate(Gitlab::Git::Blob.find(self, sha, path)) end + rescue Gitlab::Git::Repository::NoRepository + nil end def blob_by_oid(oid) @@ -1160,6 +1162,14 @@ class Repository end end + def route_map_for(sha) + blob_data_at(sha, '.gitlab/route-map.yml') + end + + def gitlab_ci_yml_for(sha) + blob_data_at(sha, '.gitlab-ci.yml') + end + protected def tree_entry_at(branch_name, path) @@ -1186,6 +1196,14 @@ class Repository private + def blob_data_at(sha, path) + blob = blob_at(sha, path) + return unless blob + + blob.load_all_data!(self) + blob.data + end + def git_action(index, action) path = normalize_path(action[:file_path]) diff --git a/app/services/ci/stop_environments_service.rb b/app/services/ci/stop_environments_service.rb index a51310c3967..42c72aba7dd 100644 --- a/app/services/ci/stop_environments_service.rb +++ b/app/services/ci/stop_environments_service.rb @@ -21,8 +21,8 @@ module Ci end def environments - @environments ||= project - .environments_recently_updated_on_branch(@ref) + @environments ||= + EnvironmentsFinder.new(project, current_user, ref: @ref, recently_updated: true).execute end end end diff --git a/app/views/projects/blob/_actions.html.haml b/app/views/projects/blob/_actions.html.haml index 26531f0b1a6..7b9cfbbd067 100644 --- a/app/views/projects/blob/_actions.html.haml +++ b/app/views/projects/blob/_actions.html.haml @@ -1,3 +1,6 @@ +.btn-group + = view_on_environment_button(@commit.sha, @path, @environment) if @environment + .btn-group.tree-btn-group = link_to 'Raw', namespace_project_raw_path(@project.namespace, @project, @id), class: 'btn btn-sm', target: '_blank' diff --git a/app/views/projects/commit/show.html.haml b/app/views/projects/commit/show.html.haml index 7afd3d80ef5..d5fc283aa8d 100644 --- a/app/views/projects/commit/show.html.haml +++ b/app/views/projects/commit/show.html.haml @@ -9,7 +9,7 @@ = render "ci_menu" - else .block-connector - = render "projects/diffs/diffs", diffs: @diffs + = render "projects/diffs/diffs", diffs: @diffs, environment: @environment = render "projects/notes/notes_with_form" - if can_collaborate_with_project? - %w(revert cherry-pick).each do |type| diff --git a/app/views/projects/compare/show.html.haml b/app/views/projects/compare/show.html.haml index 9c8f58d4aea..0dfc9fe20ed 100644 --- a/app/views/projects/compare/show.html.haml +++ b/app/views/projects/compare/show.html.haml @@ -8,7 +8,7 @@ - if @commits.present? = render "projects/commits/commit_list" - = render "projects/diffs/diffs", diffs: @diffs + = render "projects/diffs/diffs", diffs: @diffs, environment: @environment - else .light-well .center diff --git a/app/views/projects/diffs/_diffs.html.haml b/app/views/projects/diffs/_diffs.html.haml index 58c20e225c6..4b49bed835f 100644 --- a/app/views/projects/diffs/_diffs.html.haml +++ b/app/views/projects/diffs/_diffs.html.haml @@ -1,3 +1,4 @@ +- environment = local_assigns.fetch(:environment, nil) - show_whitespace_toggle = local_assigns.fetch(:show_whitespace_toggle, true) - can_create_note = !@diff_notes_disabled && can?(current_user, :create_note, diffs.project) - diff_files = diffs.diff_files @@ -30,4 +31,4 @@ - file_hash = hexdigest(diff_file.file_path) = render 'projects/diffs/file', file_hash: file_hash, project: diffs.project, - diff_file: diff_file, diff_commit: diff_commit, blob: blob + diff_file: diff_file, diff_commit: diff_commit, blob: blob, environment: environment diff --git a/app/views/projects/diffs/_file.html.haml b/app/views/projects/diffs/_file.html.haml index fc478ccc995..75885badac9 100644 --- a/app/views/projects/diffs/_file.html.haml +++ b/app/views/projects/diffs/_file.html.haml @@ -1,3 +1,4 @@ +- environment = local_assigns.fetch(:environment, nil) .diff-file.file-holder{ id: file_hash, data: diff_file_html_data(project, diff_file.file_path, diff_commit.id) } .file-title = render "projects/diffs/file_header", diff_file: diff_file, blob: blob, diff_commit: diff_commit, project: project, url: "##{file_hash}" @@ -13,6 +14,7 @@ = edit_blob_link(@merge_request.source_project, @merge_request.source_branch, diff_file.new_path, blob: blob, link_opts: link_opts) - = view_file_btn(diff_commit.id, diff_file.new_path, project) + = view_file_button(diff_commit.id, diff_file.new_path, project) + = view_on_environment_button(diff_commit.id, diff_file.new_path, environment) if environment = render 'projects/diffs/content', diff_file: diff_file, diff_commit: diff_commit, blob: blob, project: project diff --git a/app/views/projects/merge_requests/_new_diffs.html.haml b/app/views/projects/merge_requests/_new_diffs.html.haml index 74367ab9b7b..627fc4e9671 100644 --- a/app/views/projects/merge_requests/_new_diffs.html.haml +++ b/app/views/projects/merge_requests/_new_diffs.html.haml @@ -1 +1 @@ -= render "projects/diffs/diffs", diffs: @diffs, show_whitespace_toggle: false += render "projects/diffs/diffs", diffs: @diffs, environment: @environment, show_whitespace_toggle: false diff --git a/app/views/projects/merge_requests/show/_diffs.html.haml b/app/views/projects/merge_requests/show/_diffs.html.haml index 5f048d04b27..7f0913ea516 100644 --- a/app/views/projects/merge_requests/show/_diffs.html.haml +++ b/app/views/projects/merge_requests/show/_diffs.html.haml @@ -1,5 +1,5 @@ - if @merge_request_diff.collected? || @merge_request_diff.overflow? = render 'projects/merge_requests/show/versions' - = render "projects/diffs/diffs", diffs: @diffs + = render "projects/diffs/diffs", diffs: @diffs, environment: @environment - elsif @merge_request_diff.empty? .nothing-here-block Nothing to merge from #{@merge_request.source_branch} into #{@merge_request.target_branch} diff --git a/changelogs/unreleased/route-map.yml b/changelogs/unreleased/route-map.yml new file mode 100644 index 00000000000..9b6df0c54af --- /dev/null +++ b/changelogs/unreleased/route-map.yml @@ -0,0 +1,4 @@ +--- +title: Add 'View on [env]' link to blobs and individual files in diffs +merge_request: 8867 +author: diff --git a/doc/ci/environments.md b/doc/ci/environments.md index 579135c2052..f00cc854b05 100644 --- a/doc/ci/environments.md +++ b/doc/ci/environments.md @@ -442,6 +442,45 @@ and/or `production`) you can see this information in the merge request itself. ![Environment URLs in merge request](img/environments_link_url_mr.png) +### Go directly from source files to public pages on the environment + +> Introduced in GitLab 8.17. + +To go one step further, we can specify a Route Map to get GitLab to show us "View on [environment URL]" buttons to go directly from a file to that file's representation on the deployed website. It will be exposed in a few places: + +| In the diff for a merge request, comparison or commit | In the file view | +| ------ | ------ | +| !["View on env" button in merge request diff](img/view_on_env_mr.png) | !["View on env" button in file view](img/view_on_env_blob.png) | + +To get this to work, you need to tell GitLab how the paths of files in your repository map to paths of pages on your website, using a Route Map. + +A Route Map is a file inside the repository at `.gitlab/route-map.yml`, which contains a YAML array that maps `source` paths (in the repository) to `public` paths (on the website). + +This is an example of a route map for [Middleman](https://middlemanapp.com) static websites like [http://about.gitlab.com](https://gitlab.com/gitlab-com/www-gitlab-com): + +```yaml +# Blogposts +- source: /source\/posts\/([0-9]{4})-([0-9]{2})-([0-9]{2})-(.+?)\..*/ # source/posts/2017-01-30-around-the-world-in-6-releases.html.md.erb + public: '\1/\2/\3/\4/' # 2017/01/30/around-the-world-in-6-releases/ + +# HTML files +- source: /source\/(.+?\.html).*/ # source/index.html.haml + public: '\1' # index.html + +# Other files +- source: /source\/(.*)/ # source/images/blogimages/around-the-world-in-6-releases-cover.png + public: '\1' # images/blogimages/around-the-world-in-6-releases-cover.png +``` + +Mappings are defined as entries in the root YAML array, and are identified by a `-` prefix. Within an entry, we have a hash map with two keys: + +- `source`: a regular expression, starting and ending with `/`. Can include capture groups denoted by `()` that can be referred to in the `public` path. Slashes (`/`) can, but don't have to be, escaped as `\/`. +- `public`: a string, starting and ending with `'`. Can include `\N` expressions to refer to capture groups in the `source` regular expression in order of their occurence, starting with `\1`. + +The public path for a source path is determined by finding the first `source` expression that matches it, and returning the corresponding `public` path, replacing the `\N` expressions with the values of the `()` capture groups. + +In the example above, the fact that mappings are evaluated in order of their definition is used to ensure that `source/index.html.haml` will match `/source\/(.+?\.html).*/` instead of `/source\/(.*)/`, and will result in a public path of `index.html`, instead of `index.html.haml`. + --- We now have a full development cycle, where our app is tested, built, deployed diff --git a/doc/ci/img/view_on_env_blob.png b/doc/ci/img/view_on_env_blob.png Binary files differnew file mode 100644 index 00000000000..f4fe99046f0 --- /dev/null +++ b/doc/ci/img/view_on_env_blob.png diff --git a/doc/ci/img/view_on_env_mr.png b/doc/ci/img/view_on_env_mr.png Binary files differnew file mode 100644 index 00000000000..47ddb40bdc1 --- /dev/null +++ b/doc/ci/img/view_on_env_mr.png diff --git a/lib/gitlab/database.rb b/lib/gitlab/database.rb index 55b8f888d53..dc2537d36aa 100644 --- a/lib/gitlab/database.rb +++ b/lib/gitlab/database.rb @@ -35,6 +35,20 @@ module Gitlab order end + def self.nulls_first_order(field, direction = 'ASC') + order = "#{field} #{direction}" + + if Gitlab::Database.postgresql? + order << ' NULLS FIRST' + else + # `field IS NULL` will be `0` for non-NULL columns and `1` for NULL + # columns. In the (default) ascending order, `0` comes first. + order.prepend("#{field} IS NULL, ") if direction == 'DESC' + end + + order + end + def self.random Gitlab::Database.postgresql? ? "RANDOM()" : "RAND()" end diff --git a/lib/gitlab/route_map.rb b/lib/gitlab/route_map.rb new file mode 100644 index 00000000000..89985d90c10 --- /dev/null +++ b/lib/gitlab/route_map.rb @@ -0,0 +1,52 @@ +module Gitlab + class RouteMap + class FormatError < StandardError; end + + def initialize(data) + begin + entries = YAML.safe_load(data) + rescue + raise FormatError, 'Route map needs to be valid YAML' + end + + raise FormatError, 'Route map needs to be an array' unless entries.is_a?(Array) + + @map = entries.map { |entry| parse_entry(entry) } + end + + def public_path_for_source_path(path) + mapping = @map.find { |mapping| path =~ mapping[:source] } + return unless mapping + + path.sub(mapping[:source], mapping[:public]) + end + + private + + def parse_entry(entry) + raise FormatError, 'Route map entry needs to be a hash' unless entry.is_a?(Hash) + raise FormatError, 'Route map entry requires a source key' unless entry.has_key?('source') + raise FormatError, 'Route map entry requires a public key' unless entry.has_key?('public') + + source_regexp = entry['source'] + public_path = entry['public'] + + unless source_regexp.start_with?('/') && source_regexp.end_with?('/') + raise FormatError, 'Route map entry source needs to start and end in a slash (/)' + end + + source_regexp = source_regexp[1...-1].gsub('\/', '/') + + begin + source_regexp = Regexp.new("^#{source_regexp}$") + rescue RegexpError => e + raise FormatError, "Route map entry source needs to be a valid regular expression: #{e}" + end + + { + source: source_regexp, + public: public_path + } + end + end +end diff --git a/spec/features/projects/commits/cherry_pick_spec.rb b/spec/features/projects/commit/cherry_pick_spec.rb index 7baf7913424..7baf7913424 100644 --- a/spec/features/projects/commits/cherry_pick_spec.rb +++ b/spec/features/projects/commit/cherry_pick_spec.rb diff --git a/spec/features/compare_spec.rb b/spec/features/projects/compare_spec.rb index 43eb4000e58..43eb4000e58 100644 --- a/spec/features/compare_spec.rb +++ b/spec/features/projects/compare_spec.rb diff --git a/spec/features/projects/view_on_env_spec.rb b/spec/features/projects/view_on_env_spec.rb new file mode 100644 index 00000000000..ce5c5f21167 --- /dev/null +++ b/spec/features/projects/view_on_env_spec.rb @@ -0,0 +1,140 @@ +require 'spec_helper' + +describe 'View on environment', js: true do + include WaitForAjax + + let(:branch_name) { 'feature' } + let(:file_path) { 'files/ruby/feature.rb' } + let(:project) { create(:project, :repository) } + let(:user) { project.creator } + + before do + project.add_master(user) + end + + context 'when the branch has a route map' do + let(:route_map) do + <<-MAP.strip_heredoc + - source: /files/(.*)\\..*/ + public: '\\1' + MAP + end + + before do + Files::CreateService.new( + project, + user, + start_branch: branch_name, + target_branch: branch_name, + commit_message: "Add .gitlab/route-map.yml", + file_path: '.gitlab/route-map.yml', + file_content: route_map + ).execute + + # Update the file so that we still have a commit that will have a file on the environment + Files::UpdateService.new( + project, + user, + start_branch: branch_name, + target_branch: branch_name, + commit_message: "Update feature", + file_path: file_path, + file_content: "# Noop" + ).execute + end + + context 'and an active deployment' do + let(:sha) { project.commit(branch_name).sha } + let(:environment) { create(:environment, project: project, name: 'review/feature', external_url: 'http://feature.review.example.com') } + let!(:deployment) { create(:deployment, environment: environment, ref: branch_name, sha: sha) } + + context 'when visiting the diff of a merge request for the branch' do + let(:merge_request) { create(:merge_request, :simple, source_project: project, source_branch: branch_name) } + + before do + login_as(user) + + visit diffs_namespace_project_merge_request_path(project.namespace, project, merge_request) + + wait_for_ajax + end + + it 'has a "View on env" button' do + within '.diffs' do + expect(page).to have_link('View on feature.review.example.com', href: 'http://feature.review.example.com/ruby/feature') + end + end + end + + context 'when visiting a comparison for the branch' do + before do + login_as(user) + + visit namespace_project_compare_path(project.namespace, project, from: 'master', to: branch_name) + + wait_for_ajax + end + + it 'has a "View on env" button' do + expect(page).to have_link('View on feature.review.example.com', href: 'http://feature.review.example.com/ruby/feature') + end + end + + context 'when visiting a comparison for the commit' do + before do + login_as(user) + + visit namespace_project_compare_path(project.namespace, project, from: 'master', to: sha) + + wait_for_ajax + end + + it 'has a "View on env" button' do + expect(page).to have_link('View on feature.review.example.com', href: 'http://feature.review.example.com/ruby/feature') + end + end + + context 'when visiting a blob on the branch' do + before do + login_as(user) + + visit namespace_project_blob_path(project.namespace, project, File.join(branch_name, file_path)) + + wait_for_ajax + end + + it 'has a "View on env" button' do + expect(page).to have_link('View on feature.review.example.com', href: 'http://feature.review.example.com/ruby/feature') + end + end + + context 'when visiting a blob on the commit' do + before do + login_as(user) + + visit namespace_project_blob_path(project.namespace, project, File.join(sha, file_path)) + + wait_for_ajax + end + + it 'has a "View on env" button' do + expect(page).to have_link('View on feature.review.example.com', href: 'http://feature.review.example.com/ruby/feature') + end + end + + context 'when visiting the commit' do + before do + login_as(user) + + visit namespace_project_commit_path(project.namespace, project, sha) + + wait_for_ajax + end + + it 'has a "View on env" button' do + expect(page).to have_link('View on feature.review.example.com', href: 'http://feature.review.example.com/ruby/feature') + end + end + end + end +end diff --git a/spec/finders/environments_finder_spec.rb b/spec/finders/environments_finder_spec.rb new file mode 100644 index 00000000000..0c063f6d5ee --- /dev/null +++ b/spec/finders/environments_finder_spec.rb @@ -0,0 +1,110 @@ +require 'spec_helper' + +describe EnvironmentsFinder do + describe '#execute' do + let(:project) { create(:project, :repository) } + let(:user) { project.creator } + let(:environment) { create(:environment, project: project) } + + before do + project.add_master(user) + end + + context 'tagged deployment' do + before do + create(:deployment, environment: environment, ref: '1.0', tag: true, sha: project.commit.id) + end + + it 'returns environment when with_tags is set' do + expect(described_class.new(project, user, ref: 'master', commit: project.commit, with_tags: true).execute) + .to contain_exactly(environment) + end + + it 'does not return environment when no with_tags is set' do + expect(described_class.new(project, user, ref: 'master', commit: project.commit).execute) + .to be_empty + end + + it 'does not return environment when commit is not part of deployment' do + expect(described_class.new(project, user, ref: 'master', commit: project.commit('feature')).execute) + .to be_empty + end + end + + context 'branch deployment' do + before do + create(:deployment, environment: environment, ref: 'master', sha: project.commit.id) + end + + it 'returns environment when ref is set' do + expect(described_class.new(project, user, ref: 'master', commit: project.commit).execute) + .to contain_exactly(environment) + end + + it 'does not environment when ref is different' do + expect(described_class.new(project, user, ref: 'feature', commit: project.commit).execute) + .to be_empty + end + + it 'does not return environment when commit is not part of deployment' do + expect(described_class.new(project, user, ref: 'master', commit: project.commit('feature')).execute) + .to be_empty + end + + it 'returns environment when commit constraint is not set' do + expect(described_class.new(project, user, ref: 'master').execute) + .to contain_exactly(environment) + end + end + + context 'commit deployment' do + before do + create(:deployment, environment: environment, ref: 'master', sha: project.commit.id) + end + + it 'returns environment' do + expect(described_class.new(project, user, commit: project.commit).execute) + .to contain_exactly(environment) + end + end + + context 'recently updated' do + context 'when last deployment to environment is the most recent one' do + before do + create(:deployment, environment: environment, ref: 'feature') + end + + it 'finds recently updated environment' do + expect(described_class.new(project, user, ref: 'feature', recently_updated: true).execute) + .to contain_exactly(environment) + end + end + + context 'when last deployment to environment is not the most recent' do + before do + create(:deployment, environment: environment, ref: 'feature') + create(:deployment, environment: environment, ref: 'master') + end + + it 'does not find environment' do + expect(described_class.new(project, user, ref: 'feature', recently_updated: true).execute) + .to be_empty + end + end + + context 'when there are two environments that deploy to the same branch' do + let(:second_environment) { create(:environment, project: project) } + + before do + create(:deployment, environment: environment, ref: 'feature') + create(:deployment, environment: second_environment, ref: 'feature') + end + + it 'finds both environments' do + expect(described_class.new(project, user, ref: 'feature', recently_updated: true).execute) + .to contain_exactly(environment, second_environment) + end + end + end + end +end diff --git a/spec/helpers/commits_helper_spec.rb b/spec/helpers/commits_helper_spec.rb index 727c25ff529..a2c008790f9 100644 --- a/spec/helpers/commits_helper_spec.rb +++ b/spec/helpers/commits_helper_spec.rb @@ -26,4 +26,23 @@ describe CommitsHelper do not_to include('onmouseover="alert(1)"') end end + + describe '#view_on_environment_button' do + let(:project) { create(:empty_project) } + let(:environment) { create(:environment, external_url: 'http://example.com') } + let(:path) { 'source/file.html' } + let(:sha) { RepoHelpers.sample_commit.id } + + before do + allow(environment).to receive(:external_url_for).with(path, sha).and_return('http://example.com/file.html') + end + + it 'returns a link tag linking to the file in the environment' do + html = helper.view_on_environment_button(sha, path, environment) + node = Nokogiri::HTML.parse(html).at_css('a') + + expect(node[:title]).to eq('View on example.com') + expect(node[:href]).to eq('http://example.com/file.html') + end + end end diff --git a/spec/lib/gitlab/database_spec.rb b/spec/lib/gitlab/database_spec.rb index 3031559c613..b142b3a2781 100644 --- a/spec/lib/gitlab/database_spec.rb +++ b/spec/lib/gitlab/database_spec.rb @@ -55,6 +55,22 @@ describe Gitlab::Database, lib: true do end end + describe '.nulls_first_order' do + context 'when using PostgreSQL' do + before { expect(described_class).to receive(:postgresql?).and_return(true) } + + it { expect(described_class.nulls_first_order('column', 'ASC')).to eq 'column ASC NULLS FIRST'} + it { expect(described_class.nulls_first_order('column', 'DESC')).to eq 'column DESC NULLS FIRST'} + end + + context 'when using MySQL' do + before { expect(described_class).to receive(:postgresql?).and_return(false) } + + it { expect(described_class.nulls_first_order('column', 'ASC')).to eq 'column ASC'} + it { expect(described_class.nulls_first_order('column', 'DESC')).to eq 'column IS NULL, column DESC'} + end + end + describe '#true_value' do it 'returns correct value for PostgreSQL' do expect(described_class).to receive(:postgresql?).and_return(true) diff --git a/spec/lib/gitlab/route_map_spec.rb b/spec/lib/gitlab/route_map_spec.rb new file mode 100644 index 00000000000..459fa8a63a9 --- /dev/null +++ b/spec/lib/gitlab/route_map_spec.rb @@ -0,0 +1,90 @@ +require 'spec_helper' + +describe Gitlab::RouteMap, lib: true do + describe '#initialize' do + context 'when the data is not YAML' do + it 'raises an error' do + expect { described_class.new('"') }. + to raise_error(Gitlab::RouteMap::FormatError, /valid YAML/) + end + end + + context 'when the data is not a YAML array' do + it 'raises an error' do + expect { described_class.new(YAML.dump('foo')) }. + to raise_error(Gitlab::RouteMap::FormatError, /an array/) + end + end + + context 'when an entry is not a hash' do + it 'raises an error' do + expect { described_class.new(YAML.dump(['foo'])) }. + to raise_error(Gitlab::RouteMap::FormatError, /a hash/) + end + end + + context 'when an entry does not have a source key' do + it 'raises an error' do + expect { described_class.new(YAML.dump([{ 'public' => 'index.html' }])) }. + to raise_error(Gitlab::RouteMap::FormatError, /source key/) + end + end + + context 'when an entry does not have a public key' do + it 'raises an error' do + expect { described_class.new(YAML.dump([{ 'source' => '/index\.html/' }])) }. + to raise_error(Gitlab::RouteMap::FormatError, /public key/) + end + end + + context 'when an entry source does not start and end with a slash' do + it 'raises an error' do + expect { described_class.new(YAML.dump([{ 'source' => 'index.html', 'public' => 'index.html' }])) }. + to raise_error(Gitlab::RouteMap::FormatError, /a slash/) + end + end + + context 'when an entry source is not a valid regex' do + it 'raises an error' do + expect { described_class.new(YAML.dump([{ 'source' => '/[/', 'public' => 'index.html' }])) }. + to raise_error(Gitlab::RouteMap::FormatError, /regular expression/) + end + end + + context 'when all is good' do + it 'returns a route map' do + route_map = described_class.new(YAML.dump([{ 'source' => '/index\.html/', 'public' => 'index.html' }])) + + expect(route_map.public_path_for_source_path('index.html')).to eq('index.html') + end + end + end + + describe '#public_path_for_source_path' do + subject do + described_class.new(<<-'MAP'.strip_heredoc) + # Blogposts + - source: /source/posts/([0-9]{4})-([0-9]{2})-([0-9]{2})-(.+?)\..*/ # source/posts/2017-01-30-around-the-world-in-6-releases.html.md.erb + public: '\1/\2/\3/\4/' # 2017/01/30/around-the-world-in-6-releases/ + + # HTML files + - source: /source/(.+?\.html).*/ # source/index.html.haml + public: '\1' # index.html + + # Other files + - source: /source/(.*)/ # source/images/blogimages/around-the-world-in-6-releases-cover.png + public: '\1' # images/blogimages/around-the-world-in-6-releases-cover.png + MAP + end + + it 'returns the public path for a provided source path' do + expect(subject.public_path_for_source_path('source/posts/2017-01-30-around-the-world-in-6-releases.html.md.erb')).to eq('2017/01/30/around-the-world-in-6-releases/') + + expect(subject.public_path_for_source_path('source/index.html.haml')).to eq('index.html') + + expect(subject.public_path_for_source_path('source/images/blogimages/around-the-world-in-6-releases-cover.png')).to eq('images/blogimages/around-the-world-in-6-releases-cover.png') + + expect(subject.public_path_for_source_path('.gitlab/route-map.yml')).to be_nil + end + end +end diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 8b57d8600fe..960f29f3805 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -7,8 +7,6 @@ describe Environment, models: true do it { is_expected.to belong_to(:project) } it { is_expected.to have_many(:deployments) } - it { is_expected.to delegate_method(:last_deployment).to(:deployments).as(:last) } - it { is_expected.to delegate_method(:stop_action).to(:last_deployment) } it { is_expected.to delegate_method(:manual_actions).to(:last_deployment) } @@ -22,6 +20,20 @@ describe Environment, models: true do it { is_expected.to validate_length_of(:external_url).is_at_most(255) } it { is_expected.to validate_uniqueness_of(:external_url).scoped_to(:project_id) } + describe '.order_by_last_deployed_at' do + let(:project) { create(:project) } + let!(:environment1) { create(:environment, project: project) } + let!(:environment2) { create(:environment, project: project) } + let!(:environment3) { create(:environment, project: project) } + let!(:deployment1) { create(:deployment, environment: environment1) } + let!(:deployment2) { create(:deployment, environment: environment2) } + let!(:deployment3) { create(:deployment, environment: environment1) } + + it 'returns the environments in order of having been last deployed' do + expect(project.environments.order_by_last_deployed_at.to_a).to eq([environment3, environment2, environment1]) + end + end + describe '#nullify_external_url' do it 'replaces a blank url with nil' do env = build(:environment, external_url: "") @@ -323,4 +335,33 @@ describe Environment, models: true do end end end + + describe '#external_url_for' do + let(:source_path) { 'source/file.html' } + let(:sha) { RepoHelpers.sample_commit.id } + + before do + environment.external_url = 'http://example.com' + end + + context 'when the public path is not known' do + before do + allow(project).to receive(:public_path_for_source_path).with(source_path, sha).and_return(nil) + end + + it 'returns nil' do + expect(environment.external_url_for(source_path, sha)).to be_nil + end + end + + context 'when the public path is known' do + before do + allow(project).to receive(:public_path_for_source_path).with(source_path, sha).and_return('file.html') + end + + it 'returns the full external URL' do + expect(environment.external_url_for(source_path, sha)).to eq('http://example.com/file.html') + end + end + end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 32ed1e96749..e1e99300489 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1005,10 +1005,16 @@ describe MergeRequest, models: true do end end - describe "#environments" do + describe "#environments_for" do let(:project) { create(:project, :repository) } + let(:user) { project.creator } let(:merge_request) { create(:merge_request, source_project: project) } + before do + merge_request.source_project.add_master(user) + merge_request.target_project.add_master(user) + end + context 'with multiple environments' do let(:environments) { create_list(:environment, 3, project: project) } @@ -1018,7 +1024,7 @@ describe MergeRequest, models: true do end it 'selects deployed environments' do - expect(merge_request.environments).to contain_exactly(environments.first) + expect(merge_request.environments_for(user)).to contain_exactly(environments.first) end end @@ -1042,7 +1048,7 @@ describe MergeRequest, models: true do end it 'selects deployed environments' do - expect(merge_request.environments).to contain_exactly(source_environment) + expect(merge_request.environments_for(user)).to contain_exactly(source_environment) end context 'with environments on target project' do @@ -1053,7 +1059,7 @@ describe MergeRequest, models: true do end it 'selects deployed environments' do - expect(merge_request.environments).to contain_exactly(source_environment, target_environment) + expect(merge_request.environments_for(user)).to contain_exactly(source_environment, target_environment) end end end @@ -1064,7 +1070,7 @@ describe MergeRequest, models: true do end it 'returns an empty array' do - expect(merge_request.environments).to be_empty + expect(merge_request.environments_for(user)).to be_empty end end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index d7e6da02261..2129bcbd74d 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1716,100 +1716,6 @@ describe Project, models: true do end end - describe '#environments_for' do - let(:project) { create(:project, :repository) } - let(:environment) { create(:environment, project: project) } - - context 'tagged deployment' do - before do - create(:deployment, environment: environment, ref: '1.0', tag: true, sha: project.commit.id) - end - - it 'returns environment when with_tags is set' do - expect(project.environments_for('master', commit: project.commit, with_tags: true)) - .to contain_exactly(environment) - end - - it 'does not return environment when no with_tags is set' do - expect(project.environments_for('master', commit: project.commit)) - .to be_empty - end - - it 'does not return environment when commit is not part of deployment' do - expect(project.environments_for('master', commit: project.commit('feature'))) - .to be_empty - end - end - - context 'branch deployment' do - before do - create(:deployment, environment: environment, ref: 'master', sha: project.commit.id) - end - - it 'returns environment when ref is set' do - expect(project.environments_for('master', commit: project.commit)) - .to contain_exactly(environment) - end - - it 'does not environment when ref is different' do - expect(project.environments_for('feature', commit: project.commit)) - .to be_empty - end - - it 'does not return environment when commit is not part of deployment' do - expect(project.environments_for('master', commit: project.commit('feature'))) - .to be_empty - end - - it 'returns environment when commit constraint is not set' do - expect(project.environments_for('master')) - .to contain_exactly(environment) - end - end - end - - describe '#environments_recently_updated_on_branch' do - let(:project) { create(:project, :repository) } - let(:environment) { create(:environment, project: project) } - - context 'when last deployment to environment is the most recent one' do - before do - create(:deployment, environment: environment, ref: 'feature') - end - - it 'finds recently updated environment' do - expect(project.environments_recently_updated_on_branch('feature')) - .to contain_exactly(environment) - end - end - - context 'when last deployment to environment is not the most recent' do - before do - create(:deployment, environment: environment, ref: 'feature') - create(:deployment, environment: environment, ref: 'master') - end - - it 'does not find environment' do - expect(project.environments_recently_updated_on_branch('feature')) - .to be_empty - end - end - - context 'when there are two environments that deploy to the same branch' do - let(:second_environment) { create(:environment, project: project) } - - before do - create(:deployment, environment: environment, ref: 'feature') - create(:deployment, environment: second_environment, ref: 'feature') - end - - it 'finds both environments' do - expect(project.environments_recently_updated_on_branch('feature')) - .to contain_exactly(environment, second_environment) - end - end - end - describe '#deployment_variables' do context 'when project has no deployment service' do let(:project) { create(:empty_project) } @@ -1858,6 +1764,82 @@ describe Project, models: true do it { expect(Project.inside_path(path)).to eq([project1]) } end + describe '#route_map_for' do + let(:project) { create(:project) } + let(:route_map) do + <<-MAP.strip_heredoc + - source: /source/(.*)/ + public: '\\1' + MAP + end + + before do + project.repository.commit_file(User.last, '.gitlab/route-map.yml', route_map, message: 'Add .gitlab/route-map.yml', branch_name: 'master', update: false) + end + + context 'when there is a .gitlab/route-map.yml at the commit' do + context 'when the route map is valid' do + it 'returns a route map' do + map = project.route_map_for(project.commit.sha) + expect(map).to be_a_kind_of(Gitlab::RouteMap) + end + end + + context 'when the route map is invalid' do + let(:route_map) { 'INVALID' } + + it 'returns nil' do + expect(project.route_map_for(project.commit.sha)).to be_nil + end + end + end + + context 'when there is no .gitlab/route-map.yml at the commit' do + it 'returns nil' do + expect(project.route_map_for(project.commit.parent.sha)).to be_nil + end + end + end + + describe '#public_path_for_source_path' do + let(:project) { create(:project) } + let(:route_map) do + Gitlab::RouteMap.new(<<-MAP.strip_heredoc) + - source: /source/(.*)/ + public: '\\1' + MAP + end + let(:sha) { project.commit.id } + + context 'when there is a route map' do + before do + allow(project).to receive(:route_map_for).with(sha).and_return(route_map) + end + + context 'when the source path is mapped' do + it 'returns the public path' do + expect(project.public_path_for_source_path('source/file.html', sha)).to eq('file.html') + end + end + + context 'when the source path is not mapped' do + it 'returns nil' do + expect(project.public_path_for_source_path('file.html', sha)).to be_nil + end + end + end + + context 'when there is no route map' do + before do + allow(project).to receive(:route_map_for).with(sha).and_return(nil) + end + + it 'returns nil' do + expect(project.public_path_for_source_path('source/file.html', sha)).to be_nil + end + end + end + def enable_lfs allow(Gitlab.config.lfs).to receive(:enabled).and_return(true) end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 53b98ba05f8..9bfa6409607 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1782,4 +1782,40 @@ describe Repository, models: true do repository.refresh_method_caches(%i(readme license)) end end + + describe '#gitlab_ci_yml_for' do + before do + repository.commit_file(User.last, '.gitlab-ci.yml', 'CONTENT', message: 'Add .gitlab-ci.yml', branch_name: 'master', update: false) + end + + context 'when there is a .gitlab-ci.yml at the commit' do + it 'returns the content' do + expect(repository.gitlab_ci_yml_for(repository.commit.sha)).to eq('CONTENT') + end + end + + context 'when there is no .gitlab-ci.yml at the commit' do + it 'returns nil' do + expect(repository.gitlab_ci_yml_for(repository.commit.parent.sha)).to be_nil + end + end + end + + describe '#route_map_for' do + before do + repository.commit_file(User.last, '.gitlab/route-map.yml', 'CONTENT', message: 'Add .gitlab/route-map.yml', branch_name: 'master', update: false) + end + + context 'when there is a .gitlab/route-map.yml at the commit' do + it 'returns the content' do + expect(repository.route_map_for(repository.commit.sha)).to eq('CONTENT') + end + end + + context 'when there is no .gitlab/route-map.yml at the commit' do + it 'returns nil' do + expect(repository.route_map_for(repository.commit.parent.sha)).to be_nil + end + end + end end |