Commit 6873e735 authored by Sean McGivern's avatar Sean McGivern

Merge branch 'ee-12910-personal-snippets-notes-show' into 'master'

Port of 12910-personal-snippets-notes-show into EE

See merge request !1768
parents c7075a1a 959379de
Pipeline #7993230 passed with stages
in 225 minutes and 29 seconds
module NotesActions
include RendersNotes
extend ActiveSupport::Concern
included do
before_action :authorize_admin_note!, only: [:update, :destroy]
end
def index
current_fetched_at = Time.now.to_i
notes_json = { notes: [], last_fetched_at: current_fetched_at }
@notes = notes_finder.execute.inc_relations_for_view
@notes = prepare_notes_for_rendering(@notes)
@notes.each do |note|
next if note.cross_reference_not_visible_for?(current_user)
notes_json[:notes] << note_json(note)
end
render json: notes_json
end
def create
create_params = note_params.merge(
merge_request_diff_head_sha: params[:merge_request_diff_head_sha],
in_reply_to_discussion_id: params[:in_reply_to_discussion_id]
)
@note = Notes::CreateService.new(project, current_user, create_params).execute
if @note.is_a?(Note)
Banzai::NoteRenderer.render([@note], @project, current_user)
end
respond_to do |format|
format.json { render json: note_json(@note) }
format.html { redirect_back_or_default }
end
end
def update
@note = Notes::UpdateService.new(project, current_user, note_params).execute(note)
if @note.is_a?(Note)
Banzai::NoteRenderer.render([@note], @project, current_user)
end
respond_to do |format|
format.json { render json: note_json(@note) }
format.html { redirect_back_or_default }
end
end
def destroy
if note.editable?
Notes::DestroyService.new(project, current_user).execute(note)
end
respond_to do |format|
format.js { head :ok }
end
end
private
def note_json(note)
attrs = {
commands_changes: note.commands_changes
}
if note.persisted?
attrs.merge!(
valid: true,
id: note.id,
discussion_id: note.discussion_id(noteable),
html: note_html(note),
note: note.note
)
discussion = note.to_discussion(noteable)
unless discussion.individual_note?
attrs.merge!(
discussion_resolvable: discussion.resolvable?,
diff_discussion_html: diff_discussion_html(discussion),
discussion_html: discussion_html(discussion)
)
end
else
attrs.merge!(
valid: false,
errors: note.errors
)
end
attrs
end
def authorize_admin_note!
return access_denied! unless can?(current_user, :admin_note, note)
end
def note_params
params.require(:note).permit(
:project_id,
:noteable_type,
:noteable_id,
:commit_id,
:noteable,
:type,
:note,
:attachment,
# LegacyDiffNote
:line_code,
# DiffNote
:position
)
end
def noteable
@noteable ||= notes_finder.target
end
def last_fetched_at
request.headers['X-Last-Fetched-At']
end
def notes_finder
@notes_finder ||= NotesFinder.new(project, current_user, finder_params)
end
end
...@@ -10,6 +10,8 @@ module RendersNotes ...@@ -10,6 +10,8 @@ module RendersNotes
private private
def preload_max_access_for_authors(notes, project) def preload_max_access_for_authors(notes, project)
return nil unless project
user_ids = notes.map(&:author_id) user_ids = notes.map(&:author_id)
project.team.max_member_access_for_user_ids(user_ids) project.team.max_member_access_for_user_ids(user_ids)
end end
......
...@@ -22,7 +22,8 @@ module ToggleAwardEmoji ...@@ -22,7 +22,8 @@ module ToggleAwardEmoji
def to_todoable(awardable) def to_todoable(awardable)
case awardable case awardable
when Note when Note
awardable.noteable # we don't create todos for personal snippet comments for now
awardable.for_personal_snippet? ? nil : awardable.noteable
when MergeRequest, Issue when MergeRequest, Issue
awardable awardable
when Snippet when Snippet
......
class Projects::NotesController < Projects::ApplicationController class Projects::NotesController < Projects::ApplicationController
include RendersNotes include NotesActions
include ToggleAwardEmoji include ToggleAwardEmoji
# Authorize
before_action :authorize_read_note! before_action :authorize_read_note!
before_action :authorize_create_note!, only: [:create] before_action :authorize_create_note!, only: [:create]
before_action :authorize_admin_note!, only: [:update, :destroy]
before_action :authorize_resolve_note!, only: [:resolve, :unresolve] before_action :authorize_resolve_note!, only: [:resolve, :unresolve]
def index #
current_fetched_at = Time.now.to_i # This is a fix to make spinach feature tests passing:
# Controller actions are returned from AbstractController::Base and methods of parent classes are
notes_json = { notes: [], last_fetched_at: current_fetched_at } # excluded in order to return only specific controller related methods.
# That is ok for the app (no :create method in ancestors)
@notes = notes_finder.execute.inc_relations_for_view # but fails for tests because there is a :create method on FactoryGirl (one of the ancestors)
@notes = prepare_notes_for_rendering(@notes) #
# see https://github.com/rails/rails/blob/v4.2.7/actionpack/lib/abstract_controller/base.rb#L78
@notes.each do |note| #
next if note.cross_reference_not_visible_for?(current_user)
notes_json[:notes] << note_json(note)
end
render json: notes_json
end
def create def create
create_params = note_params.merge( super
merge_request_diff_head_sha: params[:merge_request_diff_head_sha],
in_reply_to_discussion_id: params[:in_reply_to_discussion_id]
)
@note = Notes::CreateService.new(project, current_user, create_params).execute
if @note.is_a?(Note)
Banzai::NoteRenderer.render([@note], @project, current_user)
end
respond_to do |format|
format.json { render json: note_json(@note) }
format.html { redirect_back_or_default }
end
end
def update
@note = Notes::UpdateService.new(project, current_user, note_params).execute(note)
if @note.is_a?(Note)
Banzai::NoteRenderer.render([@note], @project, current_user)
end
respond_to do |format|
format.json { render json: note_json(@note) }
format.html { redirect_back_or_default }
end
end
def destroy
if note.editable?
Notes::DestroyService.new(project, current_user).execute(note)
end
respond_to do |format|
format.js { head :ok }
end
end end
def delete_attachment def delete_attachment
...@@ -110,7 +64,7 @@ class Projects::NotesController < Projects::ApplicationController ...@@ -110,7 +64,7 @@ class Projects::NotesController < Projects::ApplicationController
def note_html(note) def note_html(note)
render_to_string( render_to_string(
"projects/notes/_note", "shared/notes/_note",
layout: false, layout: false,
formats: [:html], formats: [:html],
locals: { note: note } locals: { note: note }
...@@ -152,76 +106,11 @@ class Projects::NotesController < Projects::ApplicationController ...@@ -152,76 +106,11 @@ class Projects::NotesController < Projects::ApplicationController
) )
end end
def note_json(note) def finder_params
attrs = { params.merge(last_fetched_at: last_fetched_at)
commands_changes: note.commands_changes
}
if note.persisted?
attrs.merge!(
valid: true,
id: note.id,
discussion_id: note.discussion_id(noteable),
html: note_html(note),
note: note.note
)
discussion = note.to_discussion(noteable)
unless discussion.individual_note?
attrs.merge!(
discussion_resolvable: discussion.resolvable?,
diff_discussion_html: diff_discussion_html(discussion),
discussion_html: discussion_html(discussion)
)
end
else
attrs.merge!(
valid: false,
errors: note.errors
)
end
attrs
end
def authorize_admin_note!
return access_denied! unless can?(current_user, :admin_note, note)
end end
def authorize_resolve_note! def authorize_resolve_note!
return access_denied! unless can?(current_user, :resolve_note, note) return access_denied! unless can?(current_user, :resolve_note, note)
end end
def note_params
params.require(:note).permit(
:project_id,
:noteable_type,
:noteable_id,
:commit_id,
:noteable,
:type,
:note,
:attachment,
# LegacyDiffNote
:line_code,
# DiffNote
:position
)
end
def notes_finder
@notes_finder ||= NotesFinder.new(project, current_user, params.merge(last_fetched_at: last_fetched_at))
end
def noteable
@noteable ||= notes_finder.target
end
def last_fetched_at
request.headers['X-Last-Fetched-At']
end
end end
class Snippets::NotesController < ApplicationController
include NotesActions
include ToggleAwardEmoji
skip_before_action :authenticate_user!, only: [:index]
before_action :snippet
before_action :authorize_read_snippet!, only: [:show, :index, :create]
private
def note
@note ||= snippet.notes.find(params[:id])
end
alias_method :awardable, :note
def note_html(note)
render_to_string(
"shared/notes/_note",
layout: false,
formats: [:html],
locals: { note: note }
)
end
def project
nil
end
def snippet
PersonalSnippet.find_by(id: params[:snippet_id])
end
def note_params
super.merge(noteable_id: params[:snippet_id])
end
def finder_params
params.merge(last_fetched_at: last_fetched_at, target_id: snippet.id, target_type: 'personal_snippet')
end
def authorize_read_snippet!
return render_404 unless can?(current_user, :read_personal_snippet, snippet)
end
end
class SnippetsController < ApplicationController class SnippetsController < ApplicationController
include RendersNotes
include ToggleAwardEmoji include ToggleAwardEmoji
include SpammableActions include SpammableActions
include SnippetsActions include SnippetsActions
...@@ -60,6 +61,10 @@ class SnippetsController < ApplicationController ...@@ -60,6 +61,10 @@ class SnippetsController < ApplicationController
end end
def show def show
@noteable = @snippet
@discussions = @snippet.discussions
@notes = prepare_notes_for_rendering(@discussions.flat_map(&:notes))
end end
def destroy def destroy
......
...@@ -68,6 +68,8 @@ class NotesFinder ...@@ -68,6 +68,8 @@ class NotesFinder
MergeRequestsFinder.new(@current_user, project_id: @project.id).execute MergeRequestsFinder.new(@current_user, project_id: @project.id).execute
when "snippet", "project_snippet" when "snippet", "project_snippet"
SnippetsFinder.new.execute(@current_user, filter: :by_project, project: @project) SnippetsFinder.new.execute(@current_user, filter: :by_project, project: @project)
when "personal_snippet"
PersonalSnippet.all
else else
raise 'invalid target_type' raise 'invalid target_type'
end end
......
module AwardEmojiHelper module AwardEmojiHelper
def toggle_award_url(awardable) def toggle_award_url(awardable)
return url_for([:toggle_award_emoji, awardable]) unless @project return url_for([:toggle_award_emoji, awardable]) unless @project || awardable.is_a?(Note)
if awardable.is_a?(Note) if awardable.is_a?(Note)
# We render a list of notes very frequently and calling the specific method is a lot faster than the generic one (4.5x) # We render a list of notes very frequently and calling the specific method is a lot faster than the generic one (4.5x)
toggle_award_emoji_namespace_project_note_url(@project.namespace, @project, awardable.id) if awardable.for_personal_snippet?
toggle_award_emoji_snippet_note_path(awardable.noteable, awardable)
else
toggle_award_emoji_namespace_project_note_path(@project.namespace, @project, awardable.id)
end
else else
url_for([:toggle_award_emoji, @project.namespace.becomes(Namespace), @project, awardable]) url_for([:toggle_award_emoji, @project.namespace.becomes(Namespace), @project, awardable])
end end
......
...@@ -299,7 +299,7 @@ class TodoService ...@@ -299,7 +299,7 @@ class TodoService
def attributes_for_target(target) def attributes_for_target(target)
attributes = { attributes = {
project_id: target.project.id, project_id: target&.project&.id,
target_id: target.id, target_id: target.id,
target_type: target.class.name, target_type: target.class.name,
commit_id: nil commit_id: nil
......
.discussion-notes .discussion-notes
%ul.notes{ data: { discussion_id: discussion.id } } %ul.notes{ data: { discussion_id: discussion.id } }
= render partial: "projects/notes/note", collection: discussion.notes, as: :note = render partial: "shared/notes/note", collection: discussion.notes, as: :note
- if current_user - if current_user
.discussion-reply-holder .discussion-reply-holder
......
- access = note_max_access_for_user(note)
- if access
%span.note-role= access
- if note.resolvable?
- can_resolve = can?(current_user, :resolve_note, note)
%resolve-btn{ "project-path" => project_path(note.project),
"discussion-id" => note.discussion_id(@noteable),
":note-id" => note.id,
":resolved" => note.resolved?,
":can-resolve" => can_resolve,
":author-name" => "'#{j(note.author.name)}'",
"author-avatar" => note.author.avatar_url,
":note-truncated" => "'#{j(truncate(note.note, length: 17))}'",
":resolved-by" => "'#{j(note.resolved_by.try(:name))}'",
"v-show" => "#{can_resolve || note.resolved?}",
"inline-template" => true,
"ref" => "note_#{note.id}" }
%button.note-action-button.line-resolve-btn{ type: "button",
class: ("is-disabled" unless can_resolve),
":class" => "{ 'is-active': isResolved }",
":aria-label" => "buttonText",
"@click" => "resolve",
":title" => "buttonText",
":ref" => "'button'" }
= icon('spin spinner', 'v-show' => 'loading', class: 'loading', 'aria-hidden' => 'true', 'aria-label' => 'Loading')
%div{ 'v-show' => '!loading' }= render 'shared/icons/icon_status_success.svg'
- if current_user
- if note.emoji_awardable?
- user_authored = note.user_authored?(current_user)
= link_to '#', title: 'Award Emoji', class: "note-action-button note-emoji-button js-add-award js-note-emoji #{'js-user-authored' if user_authored}", data: { position: 'right' } do
= icon('spinner spin')
%span{ class: 'link-highlight award-control-icon-neutral' }= custom_icon('emoji_slightly_smiling_face')
%span{ class: 'link-highlight award-control-icon-positive' }= custom_icon('emoji_smiley')
%span{ class: 'link-highlight award-control-icon-super-positive' }= custom_icon('emoji_smile')
- if note_editable
= link_to '#', title: 'Edit comment', class: 'note-action-button js-note-edit' do
= icon('pencil', class: 'link-highlight')
= link_to namespace_project_note_path(note.project.namespace, note.project, note), title: 'Remove comment', method: :delete, data: { confirm: 'Are you sure you want to remove this comment?' }, remote: true, class: 'note-action-button js-note-delete danger' do
= icon('trash-o', class: 'danger-highlight')
.original-note-content.hidden{ data: { post_url: namespace_project_note_path(@project.namespace, @project, note), target_id: note.noteable.id, target_type: note.noteable.class.name.underscore } }
#{note.note}
%textarea.hidden.js-task-list-field.original-task-list{ data: {update_url: namespace_project_note_path(@project.namespace, @project, note) } }= note.note
%ul#notes-list.notes.main-notes-list.timeline %ul#notes-list.notes.main-notes-list.timeline
= render "projects/notes/notes" = render "shared/notes/notes"
= render 'projects/notes/edit_form' = render 'projects/notes/edit_form'
......
...@@ -29,59 +29,21 @@ ...@@ -29,59 +29,21 @@
= time_ago_with_tooltip(note.created_at, placement: 'bottom', html_class: 'note-created-ago') = time_ago_with_tooltip(note.created_at, placement: 'bottom', html_class: 'note-created-ago')
- unless note.system? - unless note.system?
.note-actions .note-actions
- access = note_max_access_for_user(note) - if note.for_personal_snippet?
- if access = render 'snippets/notes/actions', note: note, note_editable: note_editable
%span.note-role= access - else
= render 'projects/notes/actions', note: note, note_editable: note_editable
- if note.resolvable?
- can_resolve = can?(current_user, :resolve_note, note)
%resolve-btn{ "project-path" => project_path(note.project),
"discussion-id" => note.discussion_id(@noteable),
":note-id" => note.id,
":resolved" => note.resolved?,
":can-resolve" => can_resolve,
":author-name" => "'#{j(note.author.name)}'",
"author-avatar" => note.author.avatar_url,
":note-truncated" => "'#{j(truncate(note.note, length: 17))}'",
":resolved-by" => "'#{j(note.resolved_by.try(:name))}'",
"v-show" => "#{can_resolve || note.resolved?}",
"inline-template" => true,
"ref" => "note_#{note.id}" }
%button.note-action-button.line-resolve-btn{ type: "button",
class: ("is-disabled" unless can_resolve),
":class" => "{ 'is-active': isResolved }",
":aria-label" => "buttonText",
"@click" => "resolve",
":title" => "buttonText",
":ref" => "'button'" }
= icon("spin spinner", "v-show" => "loading", class: 'loading')
%div{ 'v-show' => '!loading' }= render "shared/icons/icon_status_success.svg"
- if current_user
- if note.emoji_awardable?
- user_authored = note.user_authored?(current_user)
= link_to '#', title: 'Award Emoji', class: "note-action-button note-emoji-button js-add-award js-note-emoji #{'js-user-authored' if user_authored}", data: { position: 'right' } do
= icon('spinner spin')
%span{ class: "link-highlight award-control-icon-neutral" }= custom_icon('emoji_slightly_smiling_face')
%span{ class: "link-highlight award-control-icon-positive" }= custom_icon('emoji_smiley')
%span{ class: "link-highlight award-control-icon-super-positive" }= custom_icon('emoji_smile')
- if note_editable
= link_to '#', title: 'Edit comment', class: 'note-action-button js-note-edit' do
= icon('pencil', class: 'link-highlight')
= link_to namespace_project_note_path(note.project.namespace, note.project, note), title: 'Remove comment', method: :delete, data: { confirm: 'Are you sure you want to remove this comment?' }, remote: true, class: 'note-action-button js-note-delete danger' do
= icon('trash-o', class: 'danger-highlight')
.note-body{ class: note_editable ? 'js-task-list-container' : '' } .note-body{ class: note_editable ? 'js-task-list-container' : '' }
.note-text.md .note-text.md
= preserve do = preserve do
= note.redacted_note_html = note.redacted_note_html
= edited_time_ago_with_tooltip(note, placement: 'bottom', html_class: 'note_edited_ago', include_author: true) = edited_time_ago_with_tooltip(note, placement: 'bottom', html_class: 'note_edited_ago', include_author: true)
- if note_editable - if note_editable
.original-note-content.hidden{ data: { post_url: namespace_project_note_path(@project.namespace, @project, note), target_id: note.noteable.id, target_type: note.noteable.class.name.underscore } } - if note.for_personal_snippet?
#{note.note} = render 'snippets/notes/edit', note: note
%textarea.hidden.js-task-list-field.original-task-list{ data: {update_url: namespace_project_note_path(@project.namespace, @project, note) } }= note.note - else
= render 'projects/notes/edit', note: note
.note-awards .note-awards
= render 'award_emoji/awards_block', awardable: note, inline: false = render 'award_emoji/awards_block', awardable: note, inline: false
- if note.system - if note.system
......
- if defined?(@discussions) - if defined?(@discussions)
- @discussions.each do |discussion| - @discussions.each do |discussion|
- if discussion.individual_note? - if discussion.individual_note?
= render partial: "projects/notes/note", collection: discussion.notes, as: :note = render partial: "shared/notes/note", collection: discussion.notes, as: :note
- else - else
= render 'discussions/discussion', discussion: discussion = render 'discussions/discussion', discussion: discussion
- else - else
= render partial: "projects/notes/note", collection: @notes, as: :note = render partial: "shared/notes/note", collection: @notes, as: :note
- if current_user
- if note.emoji_awardable?
- user_authored = note.user_authored?(current_user)
= link_to '#', title: 'Award Emoji', class: "note-action-button note-emoji-button js-add-award js-note-emoji #{'js-user-authored' if user_authored}", data: { position: 'right' } do
= icon('spinner spin')
%span{ class: 'link-highlight award-control-icon-neutral' }= custom_icon('emoji_slightly_smiling_face')
%span{ class: 'link-highlight award-control-icon-positive' }= custom_icon('emoji_smiley')
%span{ class: 'link-highlight award-control-icon-super-positive' }= custom_icon('emoji_smile')
- if note_editable
= link_to '#', title: 'Edit comment', class: 'note-action-button js-note-edit' do
= icon('pencil', class: 'link-highlight')
= link_to snippet_note_path(note.noteable, note), title: 'Remove comment', method: :delete, data: { confirm: 'Are you sure you want to remove this comment?' }, remote: true, class: 'note-action-button js-note-delete danger' do
= icon('trash-o', class: 'danger-highlight')
%ul#notes-list.notes.main-notes-list.timeline
= render "projects/notes/notes"
...@@ -7,3 +7,6 @@ ...@@ -7,3 +7,6 @@
.row-content-block.top-block.content-component-block .row-content-block.top-block.content-component-block
= render 'award_emoji/awards_block', awardable: @snippet, inline: true = render 'award_emoji/awards_block', awardable: @snippet, inline: true
%ul#notes-list.notes.main-notes-list.timeline
#notes= render 'shared/notes/notes'
---
title: Display comments for personal snippets
merge_request:
author:
...@@ -5,6 +5,14 @@ resources :snippets, concerns: :awardable do ...@@ -5,6 +5,14 @@ resources :snippets, concerns: :awardable do
post :mark_as_spam post :mark_as_spam
post :preview_markdown post :preview_markdown
end end
scope module: :snippets do
resources :notes, only: [:index, :create, :destroy, :update], concerns: :awardable, constraints: { id: /\d+/ } do
member do
delete :delete_attachment
end
end
end
end end
get '/s/:username', to: redirect('/u/%{username}/snippets'), get '/s/:username', to: redirect('/u/%{username}/snippets'),
......
...@@ -167,6 +167,47 @@ describe Projects::NotesController do ...@@ -167,6 +167,47 @@ describe Projects::NotesController do
end end
end end
describe 'DELETE destroy' do
let(:request_params) do
{