diff options
author | Heinrich Lee Yu <heinrich@gitlab.com> | 2018-11-30 12:03:35 +0800 |
---|---|---|
committer | Heinrich Lee Yu <hleeyu@gmail.com> | 2018-12-20 07:28:28 +0800 |
commit | 95aae95a1cbb55facd127c74d6c044b13533f3fe (patch) | |
tree | 6ecf27a219ee09f9e9d294d8304df2cb93c4da3d | |
parent | 48d2c02efe7697914591d7381ce1c72d68eed336 (diff) | |
download | gitlab-ce-95aae95a1cbb55facd127c74d6c044b13533f3fe.tar.gz |
Code style changes and refactor
27 files changed, 257 insertions, 291 deletions
diff --git a/app/assets/javascripts/right_sidebar.js b/app/assets/javascripts/right_sidebar.js index 93a37e17126..9a0cdc02952 100644 --- a/app/assets/javascripts/right_sidebar.js +++ b/app/assets/javascripts/right_sidebar.js @@ -120,14 +120,14 @@ Sidebar.prototype.todoUpdateDone = function(data) { .removeClass('is-loading') .enable() .attr('aria-label', $el.data(`${attrPrefix}Text`)) - .attr('data-delete-path', deletePath) - .attr('title', $el.data(`${attrPrefix}Text`)); + .attr('title', $el.data(`${attrPrefix}Text`)) + .data('deletePath', deletePath); if ($el.hasClass('has-tooltip')) { $el.tooltip('_fixTitle'); } - if ($el.data(`${attrPrefix}Icon`)) { + if (typeof $el.data('isCollapsed') !== 'undefined') { $elText.html($el.data(`${attrPrefix}Icon`)); } else { $elText.text($el.data(`${attrPrefix}Text`)); diff --git a/app/controllers/projects/merge_requests/conflicts_controller.rb b/app/controllers/projects/merge_requests/conflicts_controller.rb index 26219012121..045a4e974fe 100644 --- a/app/controllers/projects/merge_requests/conflicts_controller.rb +++ b/app/controllers/projects/merge_requests/conflicts_controller.rb @@ -69,6 +69,6 @@ class Projects::MergeRequests::ConflictsController < Projects::MergeRequests::Ap end def serializer - MergeRequestSerializer.new(current_user: current_user, project: merge_request.project) + MergeRequestSerializer.new(current_user: current_user, project: project) end end diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index 5e277a4c5ed..5f7147508c7 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -24,42 +24,40 @@ module IssuablesHelper end def sidebar_milestone_tooltip_label(milestone) - if milestone && milestone[:due_date] - "#{milestone[:title]}<br/>#{sidebar_milestone_remaining_days(milestone)}" - else - _('Milestone') + (milestone ? "<br/>#{milestone[:title]}" : "") - end + return _('Milestone') unless milestone.present? + + [milestone[:title], sidebar_milestone_remaining_days(milestone) || _('Milestone')].join('<br/>') end def sidebar_milestone_remaining_days(milestone) - due_date_remaining_days(due_date: milestone[:due_date], start_date: milestone[:start_date]) if milestone[:due_date] + due_date_with_remaining_days(milestone[:due_date], milestone[:start_date]) end def sidebar_due_date_tooltip_label(due_date) - _('Due date') + (due_date ? "<br/>#{due_date_remaining_days(due_date)}" : "") + [_('Due date'), due_date_with_remaining_days(due_date)].compact.join('<br/>') end - def due_date_remaining_days(due_date, start_date = nil) - "#{due_date.to_s(:medium)} (#{remaining_days_in_words(due_date: due_date, start_date: start_date)})" + def due_date_with_remaining_days(due_date, start_date = nil) + return unless due_date + + "#{due_date.to_s(:medium)} (#{remaining_days_in_words(due_date, start_date)})" end def sidebar_label_filter_path(base_path, label_name) - query_params = {label_name: [label_name]}.to_query + query_params = { label_name: [label_name] }.to_query "#{base_path}?#{query_params}" end def multi_label_name(current_labels, default_label) - if current_labels && current_labels.any? - title = current_labels.first.try(:title) || current_labels.first[:title] + return default_label if current_labels.blank? - if current_labels.size > 1 - "#{title} +#{current_labels.size - 1} more" - else - title - end + title = current_labels.first.try(:title) || current_labels.first[:title] + + if current_labels.size > 1 + "#{title} +#{current_labels.size - 1} more" else - default_label + title end end @@ -214,7 +212,7 @@ module IssuablesHelper first, last = labels.partition.with_index { |_, i| i < limit } if labels && labels.any? - label_names = first.collect { |l| l[:title] } + label_names = first.collect { |label| label.fetch(:title) } label_names << "and #{last.size} more" unless last.empty? label_names.join(', ') @@ -385,6 +383,23 @@ module IssuablesHelper params[:issuable_template] if issuable_templates(issuable).any? { |template| template[:name] == params[:issuable_template] } end + def issuable_todo_button_data(issuable, is_collapsed) + { + todo_text: _('Add todo'), + mark_text: _('Mark todo as done'), + todo_icon: sprite_icon('todo-add'), + mark_icon: sprite_icon('todo-done', css_class: 'todo-undone'), + issuable_id: issuable[:id], + issuable_type: issuable[:type], + create_path: issuable[:create_todo_path], + delete_path: issuable.dig(:current_user, :todo, :delete_path), + placement: is_collapsed ? 'left' : nil, + container: is_collapsed ? 'body' : nil, + boundary: 'viewport', + is_collapsed: is_collapsed + } + end + def close_reopen_params(issuable, action) { issuable.model_name.to_s.underscore => { state_event: action } @@ -401,16 +416,16 @@ module IssuablesHelper end end - def issuable_sidebar_options(sidebar_data) + def issuable_sidebar_options(issuable) { - endpoint: "#{sidebar_data[:issuable_json_path]}?serializer=sidebar_extras", - toggleSubscriptionEndpoint: sidebar_data[:toggle_subscription_path], - moveIssueEndpoint: sidebar_data[:move_issue_path], - projectsAutocompleteEndpoint: sidebar_data[:projects_autocomplete_path], - editable: sidebar_data[:can_edit], - currentUser: sidebar_data[:current_user], + endpoint: "#{issuable[:issuable_json_path]}?serializer=sidebar_extras", + toggleSubscriptionEndpoint: issuable[:toggle_subscription_path], + moveIssueEndpoint: issuable[:move_issue_path], + projectsAutocompleteEndpoint: issuable[:projects_autocomplete_path], + editable: issuable.dig(:current_user, :can_edit), + currentUser: issuable[:current_user], rootPath: root_path, - fullPath: sidebar_data[:project_full_path] + fullPath: issuable[:project_full_path] } end diff --git a/app/helpers/milestones_helper.rb b/app/helpers/milestones_helper.rb index e3dc598e98a..327b69e5110 100644 --- a/app/helpers/milestones_helper.rb +++ b/app/helpers/milestones_helper.rb @@ -167,7 +167,7 @@ module MilestonesHelper def milestone_tooltip_due_date(milestone) if milestone.due_date - "#{milestone.due_date.to_s(:medium)} (#{remaining_days_in_words(milestone)})" + "#{milestone.due_date.to_s(:medium)} (#{remaining_days_in_words(milestone.due_date, milestone.start_date)})" else _('Milestone') end diff --git a/app/serializers/entity_date_helper.rb b/app/serializers/entity_date_helper.rb index 87bba87a0ed..f515abe5917 100644 --- a/app/serializers/entity_date_helper.rb +++ b/app/serializers/entity_date_helper.rb @@ -44,13 +44,10 @@ module EntityDateHelper # It returns "Upcoming" for upcoming entities # If due date is provided, it returns "# days|weeks|months remaining|ago" # If start date is provided and elapsed, with no due date, it returns "# days elapsed" - def remaining_days_in_words(entity) - start_date = entity.try(:start_date) || entity.try(:[], :start_date) - due_date = entity.try(:due_date) || entity.try(:[], :due_date) - - if due_date && due_date.past? + def remaining_days_in_words(due_date, start_date = nil) + if due_date&.past? content_tag(:strong, 'Past due') - elsif start_date && start_date.future? + elsif start_date&.future? content_tag(:strong, 'Upcoming') elsif due_date is_upcoming = (due_date - Date.today).to_i > 0 @@ -66,7 +63,7 @@ module EntityDateHelper remaining_or_ago = is_upcoming ? _("remaining") : _("ago") "#{content} #{remaining_or_ago}".html_safe - elsif start_date && start_date.past? + elsif start_date&.past? days = (Date.today - start_date).to_i "#{content_tag(:strong, days)} #{'day'.pluralize(days)} elapsed".html_safe end diff --git a/app/serializers/issuable_sidebar_basic_entity.rb b/app/serializers/issuable_sidebar_basic_entity.rb new file mode 100644 index 00000000000..61de3c93337 --- /dev/null +++ b/app/serializers/issuable_sidebar_basic_entity.rb @@ -0,0 +1,106 @@ +# frozen_string_literal: true + +class IssuableSidebarBasicEntity < Grape::Entity + include RequestAwareEntity + + expose :id + expose :type do |issuable| + issuable.to_ability_name + end + expose :author_id + expose :project_id do |issuable| + issuable.project.id + end + expose :discussion_locked + expose :reference do |issuable| + issuable.to_reference(issuable.project, full: true) + end + + expose :milestone, using: ::API::Entities::Milestone + expose :labels, using: LabelEntity + + expose :current_user, if: lambda { |_issuable| current_user } do + expose :current_user, merge: true, using: API::Entities::UserBasic + + expose :todo, using: IssuableSidebarTodoEntity do |issuable| + current_user.pending_todo_for(issuable) + end + + expose :can_edit do |issuable| + can?(current_user, :"admin_#{issuable.to_ability_name}", issuable.project) + end + + expose :can_move do |issuable| + issuable.can_move?(current_user) + end + + expose :can_admin_label do |issuable| + can?(current_user, :admin_label, issuable.project) + end + end + + expose :issuable_json_path do |issuable| + if issuable.is_a?(MergeRequest) + project_merge_request_path(issuable.project, issuable.iid, :json) + else + project_issue_path(issuable.project, issuable.iid, :json) + end + end + + expose :namespace_path do |issuable| + issuable.project.namespace.full_path + end + + expose :project_path do |issuable| + issuable.project.path + end + + expose :project_full_path do |issuable| + issuable.project.full_path + end + + expose :project_issuables_path do |issuable| + project = issuable.project + namespace = project.namespace + + if issuable.is_a?(MergeRequest) + namespace_project_merge_requests_path(namespace, project) + else + namespace_project_issues_path(namespace, project) + end + end + + expose :create_todo_path do |issuable| + project_todos_path(issuable.project) + end + + expose :project_milestones_path do |issuable| + project_milestones_path(issuable.project, :json) + end + + expose :project_labels_path do |issuable| + project_labels_path(issuable.project, :json, include_ancestor_groups: true) + end + + expose :toggle_subscription_path do |issuable| + toggle_subscription_path(issuable) + end + + expose :move_issue_path do |issuable| + move_namespace_project_issue_path( + namespace_id: issuable.project.namespace.to_param, + project_id: issuable.project, + id: issuable + ) + end + + expose :projects_autocomplete_path do |issuable| + autocomplete_projects_path(project_id: issuable.project.id) + end + + private + + def current_user + request.current_user + end +end diff --git a/app/serializers/issuable_sidebar_entity.rb b/app/serializers/issuable_sidebar_entity.rb deleted file mode 100644 index 86210db4ef3..00000000000 --- a/app/serializers/issuable_sidebar_entity.rb +++ /dev/null @@ -1,120 +0,0 @@ -# frozen_string_literal: true - -class IssuableSidebarEntity < Grape::Entity - include RequestAwareEntity - - with_options if: { include_basic: true } do - expose :id - expose :type do |issuable| - issuable.to_ability_name - end - expose :author_id - expose :project_id do |issuable| - issuable.project.id - end - expose :discussion_locked - expose :reference do |issuable| - issuable.to_reference(issuable.project, full: true) - end - - expose :current_user, using: UserEntity do |issuable| - request.current_user - end - - # Relationships - expose :todo, using: IssuableSidebarTodoEntity do |issuable| - request.current_user.pending_todo_for(issuable) if request.current_user - end - expose :milestone, using: ::API::Entities::Milestone - expose :labels, using: LabelEntity - - # Permissions - expose :signed_in do |issuable| - request.current_user.present? - end - - expose :can_edit do |issuable| - can?(request.current_user, :"admin_#{issuable.to_ability_name}", issuable.project) - end - - expose :can_move do |issuable| - request.current_user && issuable.can_move?(request.current_user) - end - - expose :can_admin_label do |issuable| - can?(request.current_user, :admin_label, issuable.project) - end - - # Paths - expose :issuable_json_path do |issuable| - if issuable.is_a?(MergeRequest) - project_merge_request_path(issuable.project, issuable.iid, :json) - else - project_issue_path(issuable.project, issuable.iid, :json) - end - end - - expose :namespace_path do |issuable| - issuable.project.namespace.full_path - end - - expose :project_path do |issuable| - issuable.project.path - end - - expose :project_full_path do |issuable| - issuable.project.full_path - end - - expose :project_issuables_path do |issuable| - project = issuable.project - namespace = project.namespace - - if issuable.is_a?(MergeRequest) - namespace_project_merge_requests_path(namespace, project) - else - namespace_project_issues_path(namespace, project) - end - end - - expose :create_todo_path do |issuable| - project_todos_path(issuable.project) - end - - expose :project_milestones_path do |issuable| - project_milestones_path(issuable.project, :json) - end - - expose :project_labels_path do |issuable| - project_labels_path(issuable.project, :json, include_ancestor_groups: true) - end - - expose :toggle_subscription_path do |issuable| - toggle_subscription_path(issuable) - end - - expose :move_issue_path do |issuable| - move_namespace_project_issue_path( - namespace_id: issuable.project.namespace.to_param, - project_id: issuable.project, - id: issuable - ) - end - - expose :projects_autocomplete_path do |issuable| - autocomplete_projects_path(project_id: issuable.project.id) - end - end - - with_options if: { include_extras: true } do - include TimeTrackableEntity - - expose :participants, using: ::API::Entities::UserBasic do |issuable| - issuable.participants(request.current_user) - end - - expose :subscribed do |issuable| - issuable.subscribed?(request.current_user, issuable.project) - end - end -end diff --git a/app/serializers/issuable_sidebar_extras_entity.rb b/app/serializers/issuable_sidebar_extras_entity.rb new file mode 100644 index 00000000000..d60253564e1 --- /dev/null +++ b/app/serializers/issuable_sidebar_extras_entity.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class IssuableSidebarExtrasEntity < Grape::Entity + include RequestAwareEntity + include TimeTrackableEntity + + expose :participants, using: ::API::Entities::UserBasic do |issuable| + issuable.participants(request.current_user) + end + + expose :subscribed do |issuable| + issuable.subscribed?(request.current_user, issuable.project) + end +end diff --git a/app/serializers/issue_serializer.rb b/app/serializers/issue_serializer.rb index 93625ff3afc..0fa76f098cd 100644 --- a/app/serializers/issue_serializer.rb +++ b/app/serializers/issue_serializer.rb @@ -7,10 +7,10 @@ class IssueSerializer < BaseSerializer def represent(issue, opts = {}) entity = case opts[:serializer] - when 'sidebar', 'sidebar_extras' - opts[:include_basic] = (opts[:serializer] == 'sidebar') - opts[:include_extras] = (opts[:serializer] == 'sidebar_extras') - IssueSidebarEntity + when 'sidebar' + IssueSidebarBasicEntity + when 'sidebar_extras' + IssueSidebarExtrasEntity when 'board' IssueBoardEntity else diff --git a/app/serializers/issue_sidebar_basic_entity.rb b/app/serializers/issue_sidebar_basic_entity.rb new file mode 100644 index 00000000000..723875809ec --- /dev/null +++ b/app/serializers/issue_sidebar_basic_entity.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +class IssueSidebarBasicEntity < IssuableSidebarBasicEntity + expose :due_date + expose :confidential +end diff --git a/app/serializers/issue_sidebar_entity.rb b/app/serializers/issue_sidebar_entity.rb deleted file mode 100644 index d50e19ad046..00000000000 --- a/app/serializers/issue_sidebar_entity.rb +++ /dev/null @@ -1,12 +0,0 @@ -# frozen_string_literal: true - -class IssueSidebarEntity < IssuableSidebarEntity - with_options if: { include_basic: true } do - expose :due_date - expose :confidential - end - - with_options if: { include_extras: true } do - expose :assignees, using: API::Entities::UserBasic - end -end diff --git a/app/serializers/issue_sidebar_extras_entity.rb b/app/serializers/issue_sidebar_extras_entity.rb new file mode 100644 index 00000000000..7b6e860140b --- /dev/null +++ b/app/serializers/issue_sidebar_extras_entity.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +class IssueSidebarExtrasEntity < IssuableSidebarExtrasEntity + expose :assignees, using: API::Entities::UserBasic +end diff --git a/app/serializers/merge_request_serializer.rb b/app/serializers/merge_request_serializer.rb index 4d236273d49..4cf84336aa4 100644 --- a/app/serializers/merge_request_serializer.rb +++ b/app/serializers/merge_request_serializer.rb @@ -7,10 +7,10 @@ class MergeRequestSerializer < BaseSerializer def represent(merge_request, opts = {}) entity = case opts[:serializer] - when 'sidebar', 'sidebar_extras' - opts[:include_basic] = (opts[:serializer] == 'sidebar') - opts[:include_extras] = (opts[:serializer] == 'sidebar_extras') - MergeRequestSidebarEntity + when 'sidebar' + MergeRequestSidebarBasicEntity + when 'sidebar_extras' + IssuableSidebarExtrasEntity when 'basic' MergeRequestBasicEntity else diff --git a/app/serializers/merge_request_sidebar_basic_entity.rb b/app/serializers/merge_request_sidebar_basic_entity.rb new file mode 100644 index 00000000000..0ae7298a7c1 --- /dev/null +++ b/app/serializers/merge_request_sidebar_basic_entity.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class MergeRequestSidebarBasicEntity < IssuableSidebarBasicEntity + expose :assignee, if: lambda { |issuable| issuable.assignee } do + expose :assignee, merge: true, using: API::Entities::UserBasic + + expose :can_merge do |issuable| + issuable.can_be_merged_by?(issuable.assignee) + end + end +end diff --git a/app/serializers/merge_request_sidebar_entity.rb b/app/serializers/merge_request_sidebar_entity.rb deleted file mode 100644 index a861f2ea73b..00000000000 --- a/app/serializers/merge_request_sidebar_entity.rb +++ /dev/null @@ -1,11 +0,0 @@ -# frozen_string_literal: true - -class MergeRequestSidebarEntity < IssuableSidebarEntity - with_options if: { include_basic: true } do - expose :assignee, using: API::Entities::UserBasic - - expose :can_merge do |issuable| - issuable.can_be_merged_by?(issuable.assignee) if issuable.assignee - end - end -end diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index c04f37bc119..8c2fe2625c7 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -88,6 +88,4 @@ %section.issuable-discussion = render 'projects/issues/discussion' --# `assignees` is required for populating selected assignee values in the select box - This is should be removed when sidebar is converted to Vue. = render 'shared/issuable/sidebar', issuable_sidebar: @issuable_sidebar, assignees: @issue.assignees diff --git a/app/views/projects/merge_requests/conflicts/show.html.haml b/app/views/projects/merge_requests/conflicts/show.html.haml index fb176441485..09aeb81671a 100644 --- a/app/views/projects/merge_requests/conflicts/show.html.haml +++ b/app/views/projects/merge_requests/conflicts/show.html.haml @@ -6,12 +6,8 @@ .merge-request-details.issuable-details = render "projects/merge_requests/mr_box" --# `assignees` is required for populating selected assignee values in the select box and rendering the assignee link - This is should be removed when sidebar is converted to Vue. = render 'shared/issuable/sidebar', issuable_sidebar: @issuable_sidebar, assignees: @merge_request.assignees -= render 'shared/issuable/sidebar', issuable: @merge_request, issuable_sidebar: @issuable_sidebar - #conflicts{ "v-cloak" => "true", data: { conflicts_path: conflicts_project_merge_request_path(@merge_request.project, @merge_request, format: :json), resolve_conflicts_path: resolve_conflicts_project_merge_request_path(@merge_request.project, @merge_request) } } .loading{ "v-if" => "isLoading" } diff --git a/app/views/projects/merge_requests/show.html.haml b/app/views/projects/merge_requests/show.html.haml index 0271dee353b..d6f340d0ee2 100644 --- a/app/views/projects/merge_requests/show.html.haml +++ b/app/views/projects/merge_requests/show.html.haml @@ -86,8 +86,6 @@ .mr-loading-status = spinner --# `assignees` is required for populating selected assignee values in the select box and rendering the assignee link - This is should be removed when sidebar is converted to Vue. = render 'shared/issuable/sidebar', issuable_sidebar: @issuable_sidebar, assignees: @merge_request.assignees - if @merge_request.can_be_reverted?(current_user) diff --git a/app/views/shared/issuable/_sidebar.html.haml b/app/views/shared/issuable/_sidebar.html.haml index 07014dc99dd..0520eda37a4 100644 --- a/app/views/shared/issuable/_sidebar.html.haml +++ b/app/views/shared/issuable/_sidebar.html.haml @@ -1,6 +1,9 @@ +-# `assignees` is being passed in for populating selected assignee values in the select box and rendering the assignee link + This should be removed when this sidebar is converted to Vue since assignee data is also available in the `issuable_sidebar` hash + - issuable_type = issuable_sidebar[:type] -- signed_in = issuable_sidebar[:signed_in] -- can_edit_issuable = issuable_sidebar[:can_edit] +- signed_in = !!issuable_sidebar.dig(:current_user, :id) +- can_edit_issuable = issuable_sidebar.dig(:current_user, :can_edit) %aside.right-sidebar.js-right-sidebar.js-issuable-sidebar{ data: { signed: { in: signed_in } }, class: sidebar_gutter_collapsed_class, 'aria-live' => 'polite' } .issuable-sidebar @@ -119,7 +122,7 @@ = icon('chevron-down', 'aria-hidden': 'true') .dropdown-menu.dropdown-select.dropdown-menu-paging.dropdown-menu-labels.dropdown-menu-selectable = render partial: "shared/issuable/label_page_default" - - if issuable_sidebar[:can_admin_label] + - if issuable_sidebar.dig(:current_user, :can_admin_label) = render partial: "shared/issuable/label_page_create" = render_if_exists 'shared/issuable/sidebar_weight', issuable_sidebar: issuable_sidebar @@ -149,7 +152,7 @@ = project_ref = clipboard_button(text: project_ref, title: _('Copy reference to clipboard'), placement: "left", boundary: 'viewport') - - if issuable_sidebar[:can_move] + - if issuable_sidebar.dig(:current_user, :can_move) .block.js-sidebar-move-issue-block .sidebar-collapsed-icon{ data: { toggle: 'tooltip', placement: 'left', container: 'body', boundary: 'viewport' }, title: _('Move issue') } = custom_icon('icon_arrow_right') diff --git a/app/views/shared/issuable/_sidebar_assignees.html.haml b/app/views/shared/issuable/_sidebar_assignees.html.haml index 99dc58b582f..c5cce1823f0 100644 --- a/app/views/shared/issuable/_sidebar_assignees.html.haml +++ b/app/views/shared/issuable/_sidebar_assignees.html.haml @@ -1,6 +1,6 @@ - issuable_type = issuable_sidebar[:type] -- signed_in = issuable_sidebar[:signed_in] -- can_edit_issuable = issuable_sidebar[:can_edit] +- signed_in = !!issuable_sidebar.dig(:current_user, :id) +- can_edit_issuable = issuable_sidebar.dig(:current_user, :can_edit) - if issuable_type == "issue" #js-vue-sidebar-assignees{ data: { field: "#{issuable_type}[assignee_ids]", signed_in: signed_in } } @@ -25,7 +25,7 @@ .value.hide-collapsed - if issuable_sidebar[:assignee] = link_to_member(@project, assignee, size: 32, extra_class: 'bold') do - - if issuable_sidebar[:can_merge] + - if issuable_sidebar[:assignee][:can_merge] %span.float-right.cannot-be-merged{ data: { toggle: 'tooltip', placement: 'left' }, title: _('Not allowed to merge') } = icon('exclamation-triangle', 'aria-hidden': 'true') %span.username diff --git a/app/views/shared/issuable/_sidebar_todo.html.haml b/app/views/shared/issuable/_sidebar_todo.html.haml index 8411327566b..de4df016cfb 100644 --- a/app/views/shared/issuable/_sidebar_todo.html.haml +++ b/app/views/shared/issuable/_sidebar_todo.html.haml @@ -1,29 +1,15 @@ - is_collapsed = local_assigns.fetch(:is_collapsed, false) -- todo = issuable_sidebar[:todo] || {} +- has_todo = !!issuable_sidebar.dig(:current_user, :todo, :id) -- todo_text = _('Add todo') -- mark_text = _('Mark todo as done') -- todo_icon = sprite_icon('todo-add') -- mark_icon = sprite_icon('todo-done', css_class: 'todo-undone') - -- mark_content = is_collapsed ? mark_icon : mark_text -- todo_content = is_collapsed ? todo_icon : todo_text +- todo_button_data = issuable_todo_button_data(issuable_sidebar, is_collapsed) +- button_title = has_todo ? todo_button_data[:mark_text] : todo_button_data[:todo_text] +- button_icon = has_todo ? todo_button_data[:mark_icon] : todo_button_data[:todo_icon] %button.issuable-todo-btn.js-issuable-todo{ type: 'button', class: (is_collapsed ? 'btn-blank sidebar-collapsed-icon dont-change-state has-tooltip' : 'btn btn-default issuable-header-btn float-right'), - title: (todo[:id] ? mark_text : todo_text), - 'aria-label' => (todo[:id] ? mark_text : todo_text), - data: { todo_text: todo_text, - mark_text: mark_text, - todo_icon: is_collapsed ? todo_icon : nil, - mark_icon: is_collapsed ? mark_icon : nil, - issuable_id: issuable_sidebar[:id], - issuable_type: issuable_sidebar[:type], - create_path: issuable_sidebar[:create_todo_path], - delete_path: todo[:delete_path] } } + title: button_title, + 'aria-label' => button_title, + data: todo_button_data } %span.issuable-todo-inner.js-issuable-todo-inner< - - if todo[:id] - = mark_content - - else - = todo_content + = is_collapsed ? button_icon : button_title = icon('spin spinner', 'aria-hidden': 'true') diff --git a/app/views/shared/milestones/_sidebar.html.haml b/app/views/shared/milestones/_sidebar.html.haml index becd1c4884e..b24075c7849 100644 --- a/app/views/shared/milestones/_sidebar.html.haml +++ b/app/views/shared/milestones/_sidebar.html.haml @@ -65,7 +65,7 @@ %span.bold= milestone.due_date.to_s(:medium) - else %span.no-value No due date - - remaining_days = remaining_days_in_words(milestone) + - remaining_days = remaining_days_in_words(milestone.due_date, milestone.start_date) - if remaining_days.present? = surround '(', ')' do %span.remaining-days= remaining_days diff --git a/spec/fixtures/api/schemas/entities/issuable_sidebar_todo.json b/spec/fixtures/api/schemas/entities/issuable_sidebar_todo.json index 264f0a5f0db..b77e60ece12 100644 --- a/spec/fixtures/api/schemas/entities/issuable_sidebar_todo.json +++ b/spec/fixtures/api/schemas/entities/issuable_sidebar_todo.json @@ -1,5 +1,5 @@ { - "type": "object", + "type": ["object", "null"], "properties" : { "id": { "type": "integer" }, "delete_path": { "type": "string" } diff --git a/spec/fixtures/api/schemas/entities/issue_sidebar.json b/spec/fixtures/api/schemas/entities/issue_sidebar.json index ed08df27e0b..93adb493d1b 100644 --- a/spec/fixtures/api/schemas/entities/issue_sidebar.json +++ b/spec/fixtures/api/schemas/entities/issue_sidebar.json @@ -10,15 +10,16 @@ "confidential": { "type": "boolean" }, "reference": { "type": "string" }, "current_user": { - "oneOf": [ - { "type": "null" }, - { "$ref": "user.json" } - ] - }, - "todo": { - "oneOf": [ - { "type": "null" }, - { "$ref": "issuable_sidebar_todo.json" } + "allOf": [ + { "$ref": "../public_api/v4/user/basic.json" }, + { "type": "object", + "properties" : { + "todo": { "$ref": "issuable_sidebar_todo.json" }, + "can_edit": { "type": "boolean" }, + "can_move": { "type": "boolean" }, + "can_admin_label": { "type": "boolean" } + } + } ] }, "milestone": { @@ -31,10 +32,6 @@ "type": "array", "items": { "$ref": "label.json" } }, - "signed_in": { "type": "boolean" }, - "can_edit": { "type": "boolean" }, - "can_move": { "type": "boolean" }, - "can_admin_label": { "type": "boolean" }, "issuable_json_path": { "type": "string" }, "namespace_path": { "type": "string" }, "project_path": { "type": "string" }, diff --git a/spec/fixtures/api/schemas/entities/merge_request_sidebar.json b/spec/fixtures/api/schemas/entities/merge_request_sidebar.json index 3172c2788b9..7e9e048a9fd 100644 --- a/spec/fixtures/api/schemas/entities/merge_request_sidebar.json +++ b/spec/fixtures/api/schemas/entities/merge_request_sidebar.json @@ -8,15 +8,16 @@ "discussion_locked": { "type": ["boolean", "null"] }, "reference": { "type": "string" }, "current_user": { - "oneOf": [ - { "type": "null" }, - { "$ref": "user.json" } - ] - }, - "todo": { - "oneOf": [ - { "type": "null" }, - { "$ref": "issuable_sidebar_todo.json" } + "allOf": [ + { "$ref": "../public_api/v4/user/basic.json" }, + { "type": "object", + "properties" : { + "todo": { "$ref": "issuable_sidebar_todo.json" }, + "can_edit": { "type": "boolean" }, + "can_move": { "type": "boolean" }, + "can_admin_label": { "type": "boolean" } + } + } ] }, "milestone": { @@ -30,13 +31,15 @@ "items": { "$ref": "label.json" } }, "assignee": { - "$ref": "../public_api/v4/user/basic.json" + "allOf": [ + { "$ref": "../public_api/v4/user/basic.json" }, + { "type": "object", + "properties" : { + "can_merge": { "type": "boolean" } + } + } + ] }, - "signed_in": { "type": "boolean" }, - "can_edit": { "type": "boolean" }, - "can_move": { "type": "boolean" }, - "can_admin_label": { "type": "boolean" }, - "can_merge": { "type": ["boolean", "null"] }, "issuable_json_path": { "type": "string" }, "namespace_path": { "type": "string" }, "project_path": { "type": "string" }, diff --git a/spec/helpers/issuables_helper_spec.rb b/spec/helpers/issuables_helper_spec.rb index 3efac10ac9f..81231cca085 100644 --- a/spec/helpers/issuables_helper_spec.rb +++ b/spec/helpers/issuables_helper_spec.rb @@ -43,8 +43,8 @@ describe IssuablesHelper do end describe '#issuable_labels_tooltip' do - let (:label_entity) { LabelEntity.represent(label) } - let (:label2_entity) { LabelEntity.represent(label2) } + let(:label_entity) { LabelEntity.represent(label).as_json } + let(:label2_entity) { LabelEntity.represent(label2).as_json } it 'returns label text with no labels' do expect(issuable_labels_tooltip([])).to eq("Labels") diff --git a/spec/serializers/entity_date_helper_spec.rb b/spec/serializers/entity_date_helper_spec.rb index df7f33847c9..ae0f917415c 100644 --- a/spec/serializers/entity_date_helper_spec.rb +++ b/spec/serializers/entity_date_helper_spec.rb @@ -50,7 +50,7 @@ describe EntityDateHelper do end context 'when less than 31 days remaining' do - let(:milestone_remaining) { date_helper_class.remaining_days_in_words(build_stubbed(:milestone, due_date: 12.days.from_now.utc)) } + let(:milestone_remaining) { date_helper_class.remaining_days_in_words(12.days.from_now.utc.to_date) } it 'returns days remaining' do expect(milestone_remaining).to eq("<strong>12</strong> days remaining") @@ -58,7 +58,7 @@ describe EntityDateHelper do end context 'when less than 1 year and more than 30 days remaining' do - let(:milestone_remaining) { date_helper_class.remaining_days_in_words(build_stubbed(:milestone, due_date: 2.months.from_now.utc)) } + let(:milestone_remaining) { date_helper_class.remaining_days_in_words(2.months.from_now.utc.to_date) } it 'returns months remaining' do expect(milestone_remaining).to eq("<strong>2</strong> months remaining") @@ -66,7 +66,7 @@ describe EntityDateHelper do end context 'when more than 1 year remaining' do - let(:milestone_remaining) { date_helper_class.remaining_days_in_words(build_stubbed(:milestone, due_date: (1.year.from_now + 2.days).utc)) } + let(:milestone_remaining) { date_helper_class.remaining_days_in_words((1.year.from_now + 2.days).utc.to_date) } it 'returns years remaining' do expect(milestone_remaining).to eq("<strong>1</strong> year remaining") @@ -74,7 +74,7 @@ describe EntityDateHelper do end context 'when milestone is expired' do - let(:milestone_remaining) { date_helper_class.remaining_days_in_words(build_stubbed(:milestone, due_date: 2.days.ago.utc)) } + let(:milestone_remaining) { date_helper_class.remaining_days_in_words(2.days.ago.utc.to_date) } it 'returns "Past due"' do expect(milestone_remaining).to eq("<strong>Past due</strong>") @@ -82,7 +82,7 @@ describe EntityDateHelper do end context 'when milestone has start_date in the future' do - let(:milestone_remaining) { date_helper_class.remaining_days_in_words(build_stubbed(:milestone, start_date: 2.days.from_now.utc)) } + let(:milestone_remaining) { date_helper_class.remaining_days_in_words(nil, 2.days.from_now.utc.to_date) } it 'returns "Upcoming"' do expect(milestone_remaining).to eq("<strong>Upcoming</strong>") @@ -90,37 +90,11 @@ describe EntityDateHelper do end context 'when milestone has start_date in the past' do - let(:milestone_remaining) { date_helper_class.remaining_days_in_words(build_stubbed(:milestone, start_date: 2.days.ago.utc)) } + let(:milestone_remaining) { date_helper_class.remaining_days_in_words(nil, 2.days.ago.utc.to_date) } it 'returns days elapsed' do expect(milestone_remaining).to eq("<strong>2</strong> days elapsed") end end - - context 'with Hash as param' do - context 'when due_date is in the past' do - it 'returns "Past due"' do - expect(date_helper_class.remaining_days_in_words(due_date: 2.days.ago.to_date)).to eq("<strong>Past due</strong>") - end - end - - context 'when due_date is in the future' do - it 'returns days remaining' do - expect(date_helper_class.remaining_days_in_words(due_date: 12.days.from_now.to_date)).to eq("<strong>12</strong> days remaining") - end - end - - context 'when start_date is in the future' do - it 'returns "Upcoming"' do - expect(date_helper_class.remaining_days_in_words(start_date: 2.days.from_now.to_date)).to eq("<strong>Upcoming</strong>") - end - end - - context 'when start_date is in the past' do - it 'returns days elapsed' do - expect(date_helper_class.remaining_days_in_words(start_date: 2.days.ago.to_date)).to eq("<strong>2</strong> days elapsed") - end - end - end end end |